From a79aac71d2410238f45b7d1fa0c489d30abca9a6 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 11 Jan 2022 12:09:42 +0000 Subject: [PATCH 1/8] Ensure ready future completes (with error) in the case the watcher fails and closes See #115. --- lib/src/directory_watcher/mac_os.dart | 4 ++++ lib/src/directory_watcher/windows.dart | 4 ++++ lib/src/resubscribable.dart | 6 ++++-- test/ready/shared.dart | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/src/directory_watcher/mac_os.dart b/lib/src/directory_watcher/mac_os.dart index 2046ce0..9b69f95 100644 --- a/lib/src/directory_watcher/mac_os.dart +++ b/lib/src/directory_watcher/mac_os.dart @@ -396,6 +396,10 @@ class _MacOSDirectoryWatcher /// Emit an error, then close the watcher. void _emitError(Object error, StackTrace stackTrace) { + // Guarantee that ready always completes. + if (!_readyCompleter.isCompleted) { + _readyCompleter.completeError(error); + } _eventsController.addError(error, stackTrace); close(); } diff --git a/lib/src/directory_watcher/windows.dart b/lib/src/directory_watcher/windows.dart index 09b4b36..923db80 100644 --- a/lib/src/directory_watcher/windows.dart +++ b/lib/src/directory_watcher/windows.dart @@ -427,6 +427,10 @@ class _WindowsDirectoryWatcher /// Emit an error, then close the watcher. void _emitError(Object error, StackTrace stackTrace) { + // Guarantee that ready always completes. + if (!_readyCompleter.isCompleted) { + _readyCompleter.completeError(error); + } _eventsController.addError(error, stackTrace); close(); } diff --git a/lib/src/resubscribable.dart b/lib/src/resubscribable.dart index 91f5090..294aba6 100644 --- a/lib/src/resubscribable.dart +++ b/lib/src/resubscribable.dart @@ -53,8 +53,10 @@ abstract class ResubscribableWatcher implements Watcher { // It's important that we complete the value of [_readyCompleter] at // the time [onListen] is called, as opposed to the value when // [watcher.ready] fires. A new completer may be created by that time. - await watcher.ready; - _readyCompleter.complete(); + await watcher.ready.then( + _readyCompleter.complete, + onError: _readyCompleter.completeError, + ); }, onCancel: () { // Cancel the subscription before closing the watcher so that the diff --git a/test/ready/shared.dart b/test/ready/shared.dart index b6b3cce..a97fbee 100644 --- a/test/ready/shared.dart +++ b/test/ready/shared.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:io'; import 'package:test/test.dart'; @@ -61,4 +62,18 @@ void sharedTests() { // Should be back to not ready. expect(watcher.ready, doesNotComplete); }); + + test('completes with error if directory does not exist', () async { + var watcher = createWatcher(path: 'does/not/exist'); + + // Subscribe to the events (else ready will never fire). + // ignore: unused_local_variable + var subscription = watcher.events.listen((event) {}, onError: (error) {}); + + // Expected ready completes with an error. + await expectLater(watcher.ready, throwsA(isA())); + + // Now unsubscribe. + await subscription.cancel(); + }); } From e1b5ee8be23f6459f883416f1f2015e10dce884d Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 11 Jan 2022 12:34:41 +0000 Subject: [PATCH 2/8] Update Linux watcher to handle completing ready with errors too --- lib/src/directory_watcher/linux.dart | 32 +++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/src/directory_watcher/linux.dart b/lib/src/directory_watcher/linux.dart index 1bf5efd..458ae42 100644 --- a/lib/src/directory_watcher/linux.dart +++ b/lib/src/directory_watcher/linux.dart @@ -90,12 +90,10 @@ class _LinuxDirectoryWatcher } else { _files.add(entity.path); } - }, onError: (Object error, StackTrace stackTrace) { - _eventsController.addError(error, stackTrace); - close(); - }, onDone: () { - _readyCompleter.complete(); - }, cancelOnError: true); + }, + onError: _emitError, + onDone: _readyCompleter.complete, + cancelOnError: true); } @override @@ -195,15 +193,15 @@ class _LinuxDirectoryWatcher // contents, if (files.contains(path) && _files.contains(path)) continue; for (var file in _files.remove(path)) { - _emit(ChangeType.REMOVE, file); + _emitEvent(ChangeType.REMOVE, file); } } for (var file in files) { if (_files.contains(file)) { - _emit(ChangeType.MODIFY, file); + _emitEvent(ChangeType.MODIFY, file); } else { - _emit(ChangeType.ADD, file); + _emitEvent(ChangeType.ADD, file); _files.add(file); } } @@ -221,7 +219,7 @@ class _LinuxDirectoryWatcher _watchSubdir(entity.path); } else { _files.add(entity.path); - _emit(ChangeType.ADD, entity.path); + _emitEvent(ChangeType.ADD, entity.path); } }, onError: (Object error, StackTrace stackTrace) { // Ignore an exception caused by the dir not existing. It's fine if it @@ -242,7 +240,7 @@ class _LinuxDirectoryWatcher // caused by a MOVE, we need to manually emit events. if (isReady) { for (var file in _files.paths) { - _emit(ChangeType.REMOVE, file); + _emitEvent(ChangeType.REMOVE, file); } } @@ -251,12 +249,22 @@ class _LinuxDirectoryWatcher /// Emits a [WatchEvent] with [type] and [path] if this watcher is in a state /// to emit events. - void _emit(ChangeType type, String path) { + void _emitEvent(ChangeType type, String path) { if (!isReady) return; if (_eventsController.isClosed) return; _eventsController.add(WatchEvent(type, path)); } + /// Emit an error, then close the watcher. + void _emitError(Object error, StackTrace stackTrace) { + // Guarantee that ready always completes. + if (!_readyCompleter.isCompleted) { + _readyCompleter.completeError(error); + } + _eventsController.addError(error, stackTrace); + close(); + } + /// Like [Stream.listen], but automatically adds the subscription to /// [_subscriptions] so that it can be canceled when [close] is called. void _listen(Stream stream, void Function(T) onData, From 99571227b883a0c1f6ed985732ce7fef54bd88a3 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 11 Jan 2022 16:07:47 +0000 Subject: [PATCH 3/8] Always check ready hasn't completed --- lib/src/directory_watcher/linux.dart | 27 ++++++++++++++++---------- lib/src/directory_watcher/mac_os.dart | 13 ++++++++++--- lib/src/directory_watcher/windows.dart | 4 +++- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/src/directory_watcher/linux.dart b/lib/src/directory_watcher/linux.dart index 458ae42..47c0c65 100644 --- a/lib/src/directory_watcher/linux.dart +++ b/lib/src/directory_watcher/linux.dart @@ -84,16 +84,23 @@ class _LinuxDirectoryWatcher var innerStream = _nativeEvents.stream.batchEvents(); _listen(innerStream, _onBatch, onError: _eventsController.addError); - _listen(Directory(path).list(recursive: true), (FileSystemEntity entity) { - if (entity is Directory) { - _watchSubdir(entity.path); - } else { - _files.add(entity.path); - } - }, - onError: _emitError, - onDone: _readyCompleter.complete, - cancelOnError: true); + _listen( + Directory(path).list(recursive: true), + (FileSystemEntity entity) { + if (entity is Directory) { + _watchSubdir(entity.path); + } else { + _files.add(entity.path); + } + }, + onError: _emitError, + onDone: () { + if (!_readyCompleter.isCompleted) { + _readyCompleter.complete(); + } + }, + cancelOnError: true, + ); } @override diff --git a/lib/src/directory_watcher/mac_os.dart b/lib/src/directory_watcher/mac_os.dart index 9b69f95..2d0457d 100644 --- a/lib/src/directory_watcher/mac_os.dart +++ b/lib/src/directory_watcher/mac_os.dart @@ -87,8 +87,11 @@ class _MacOSDirectoryWatcher // // If we do receive a batch of events, [_onBatch] will ensure that these // futures don't fire and that the directory is re-listed. - Future.wait([_listDir(), _waitForBogusEvents()]) - .then((_) => _readyCompleter.complete()); + Future.wait([_listDir(), _waitForBogusEvents()]).then((_) { + if (!_readyCompleter.isCompleted) { + _readyCompleter.complete(); + } + }); } @override @@ -115,7 +118,11 @@ class _MacOSDirectoryWatcher // Cancel the timer because bogus events only occur in the first batch, so // we can fire [ready] as soon as we're done listing the directory. _bogusEventTimer.cancel(); - _listDir().then((_) => _readyCompleter.complete()); + _listDir().then((_) { + if (!_readyCompleter.isCompleted) { + _readyCompleter.complete(); + } + }); return; } diff --git a/lib/src/directory_watcher/windows.dart b/lib/src/directory_watcher/windows.dart index 923db80..cf387f5 100644 --- a/lib/src/directory_watcher/windows.dart +++ b/lib/src/directory_watcher/windows.dart @@ -92,7 +92,9 @@ class _WindowsDirectoryWatcher _listDir().then((_) { _startWatch(); _startParentWatcher(); - _readyCompleter.complete(); + if (!_readyCompleter.isCompleted) { + _readyCompleter.complete(); + } }); } From ac6a90eecc7134e0e25c5860faaa13d3fa49a2cb Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 11 Jan 2022 17:37:34 +0000 Subject: [PATCH 4/8] Ensure ready completes for more errors for Linux --- lib/src/directory_watcher/linux.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/directory_watcher/linux.dart b/lib/src/directory_watcher/linux.dart index 47c0c65..83b227d 100644 --- a/lib/src/directory_watcher/linux.dart +++ b/lib/src/directory_watcher/linux.dart @@ -233,8 +233,7 @@ class _LinuxDirectoryWatcher // was added and then quickly removed. if (error is FileSystemException) return; - _eventsController.addError(error, stackTrace); - close(); + _emitError(error, stackTrace); }, cancelOnError: true); } From 05f2ec99aec2a9c81375c0c54b7b35e5beb3cf32 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 11 Jan 2022 17:38:06 +0000 Subject: [PATCH 5/8] Ensure PollingDirectoryWatcher fails for non-existing directories to match others --- lib/src/directory_watcher/polling.dart | 17 ++++++++--------- lib/src/utils.dart | 11 ----------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/src/directory_watcher/polling.dart b/lib/src/directory_watcher/polling.dart index 2a43937..56db7e4 100644 --- a/lib/src/directory_watcher/polling.dart +++ b/lib/src/directory_watcher/polling.dart @@ -9,7 +9,6 @@ import '../async_queue.dart'; import '../directory_watcher.dart'; import '../resubscribable.dart'; import '../stat.dart'; -import '../utils.dart'; import '../watch_event.dart'; /// Periodically polls a directory for changes. @@ -43,11 +42,11 @@ class _PollingDirectoryWatcher final _events = StreamController.broadcast(); @override - bool get isReady => _ready.isCompleted; + bool get isReady => _readyCompleter.isCompleted; @override - Future get ready => _ready.future; - final _ready = Completer(); + Future get ready => _readyCompleter.future; + final _readyCompleter = Completer(); /// The amount of time the watcher pauses between successive polls of the /// directory contents. @@ -119,11 +118,11 @@ class _PollingDirectoryWatcher if (entity is! File) return; _filesToProcess.add(entity.path); }, onError: (Object error, StackTrace stackTrace) { - if (!isDirectoryNotFoundException(error)) { - // It's some unknown error. Pipe it over to the event stream so the - // user can see it. - _events.addError(error, stackTrace); + // Guarantee that ready always completes. + if (!_readyCompleter.isCompleted) { + _readyCompleter.completeError(error); } + _events.addError(error, stackTrace); // When an error occurs, we end the listing normally, which has the // desired effect of marking all files that were in the directory as @@ -177,7 +176,7 @@ class _PollingDirectoryWatcher _lastModifieds.remove(removed); } - if (!isReady) _ready.complete(); + if (!isReady) _readyCompleter.complete(); // Wait and then poll again. await Future.delayed(_pollingDelay); diff --git a/lib/src/utils.dart b/lib/src/utils.dart index ecf4e10..4973029 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -3,19 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; -import 'dart:io'; import 'dart:collection'; -/// Returns `true` if [error] is a [FileSystemException] for a missing -/// directory. -bool isDirectoryNotFoundException(Object error) { - if (error is! FileSystemException) return false; - - // See dartbug.com/12461 and tests/standalone/io/directory_error_test.dart. - var notFoundCode = Platform.operatingSystem == 'windows' ? 3 : 2; - return error.osError?.errorCode == notFoundCode; -} - /// Returns the union of all elements in each set in [sets]. Set unionAll(Iterable> sets) => sets.fold({}, (union, set) => union.union(set)); From 89c96e9d0c309f639db5ccfeef6374edde88dd02 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 12 Jan 2022 10:35:19 +0000 Subject: [PATCH 6/8] Always complete `ready` successfully --- lib/src/directory_watcher/linux.dart | 15 +++++++++++---- lib/src/directory_watcher/mac_os.dart | 8 ++++---- lib/src/directory_watcher/polling.dart | 4 ++-- lib/src/directory_watcher/windows.dart | 6 +++--- lib/src/resubscribable.dart | 6 ++---- lib/watcher.dart | 3 +++ test/ready/shared.dart | 8 +++----- 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lib/src/directory_watcher/linux.dart b/lib/src/directory_watcher/linux.dart index 83b227d..dafaeb1 100644 --- a/lib/src/directory_watcher/linux.dart +++ b/lib/src/directory_watcher/linux.dart @@ -82,7 +82,14 @@ class _LinuxDirectoryWatcher // Batch the inotify changes together so that we can dedup events. var innerStream = _nativeEvents.stream.batchEvents(); - _listen(innerStream, _onBatch, onError: _eventsController.addError); + _listen(innerStream, _onBatch, + onError: (Object error, StackTrace stackTrace) { + // Guarantee that ready always completes. + if (!isReady) { + _readyCompleter.complete(); + } + _eventsController.addError(error, stackTrace); + }); _listen( Directory(path).list(recursive: true), @@ -95,7 +102,7 @@ class _LinuxDirectoryWatcher }, onError: _emitError, onDone: () { - if (!_readyCompleter.isCompleted) { + if (!isReady) { _readyCompleter.complete(); } }, @@ -264,8 +271,8 @@ class _LinuxDirectoryWatcher /// Emit an error, then close the watcher. void _emitError(Object error, StackTrace stackTrace) { // Guarantee that ready always completes. - if (!_readyCompleter.isCompleted) { - _readyCompleter.completeError(error); + if (!isReady) { + _readyCompleter.complete(); } _eventsController.addError(error, stackTrace); close(); diff --git a/lib/src/directory_watcher/mac_os.dart b/lib/src/directory_watcher/mac_os.dart index 2d0457d..12648c8 100644 --- a/lib/src/directory_watcher/mac_os.dart +++ b/lib/src/directory_watcher/mac_os.dart @@ -88,7 +88,7 @@ class _MacOSDirectoryWatcher // If we do receive a batch of events, [_onBatch] will ensure that these // futures don't fire and that the directory is re-listed. Future.wait([_listDir(), _waitForBogusEvents()]).then((_) { - if (!_readyCompleter.isCompleted) { + if (!isReady) { _readyCompleter.complete(); } }); @@ -119,7 +119,7 @@ class _MacOSDirectoryWatcher // we can fire [ready] as soon as we're done listing the directory. _bogusEventTimer.cancel(); _listDir().then((_) { - if (!_readyCompleter.isCompleted) { + if (!isReady) { _readyCompleter.complete(); } }); @@ -404,8 +404,8 @@ class _MacOSDirectoryWatcher /// Emit an error, then close the watcher. void _emitError(Object error, StackTrace stackTrace) { // Guarantee that ready always completes. - if (!_readyCompleter.isCompleted) { - _readyCompleter.completeError(error); + if (!isReady) { + _readyCompleter.complete(); } _eventsController.addError(error, stackTrace); close(); diff --git a/lib/src/directory_watcher/polling.dart b/lib/src/directory_watcher/polling.dart index 56db7e4..e99e26b 100644 --- a/lib/src/directory_watcher/polling.dart +++ b/lib/src/directory_watcher/polling.dart @@ -119,8 +119,8 @@ class _PollingDirectoryWatcher _filesToProcess.add(entity.path); }, onError: (Object error, StackTrace stackTrace) { // Guarantee that ready always completes. - if (!_readyCompleter.isCompleted) { - _readyCompleter.completeError(error); + if (!isReady) { + _readyCompleter.complete(); } _events.addError(error, stackTrace); diff --git a/lib/src/directory_watcher/windows.dart b/lib/src/directory_watcher/windows.dart index cf387f5..710caf5 100644 --- a/lib/src/directory_watcher/windows.dart +++ b/lib/src/directory_watcher/windows.dart @@ -92,7 +92,7 @@ class _WindowsDirectoryWatcher _listDir().then((_) { _startWatch(); _startParentWatcher(); - if (!_readyCompleter.isCompleted) { + if (!isReady) { _readyCompleter.complete(); } }); @@ -430,8 +430,8 @@ class _WindowsDirectoryWatcher /// Emit an error, then close the watcher. void _emitError(Object error, StackTrace stackTrace) { // Guarantee that ready always completes. - if (!_readyCompleter.isCompleted) { - _readyCompleter.completeError(error); + if (!isReady) { + _readyCompleter.complete(); } _eventsController.addError(error, stackTrace); close(); diff --git a/lib/src/resubscribable.dart b/lib/src/resubscribable.dart index 294aba6..91f5090 100644 --- a/lib/src/resubscribable.dart +++ b/lib/src/resubscribable.dart @@ -53,10 +53,8 @@ abstract class ResubscribableWatcher implements Watcher { // It's important that we complete the value of [_readyCompleter] at // the time [onListen] is called, as opposed to the value when // [watcher.ready] fires. A new completer may be created by that time. - await watcher.ready.then( - _readyCompleter.complete, - onError: _readyCompleter.completeError, - ); + await watcher.ready; + _readyCompleter.complete(); }, onCancel: () { // Cancel the subscription before closing the watcher so that the diff --git a/lib/watcher.dart b/lib/watcher.dart index 22e0d6e..12a5369 100644 --- a/lib/watcher.dart +++ b/lib/watcher.dart @@ -42,6 +42,9 @@ abstract class Watcher { /// /// If the watcher is already monitoring, this returns an already complete /// future. + /// + /// This future always completes successfully as errors are provided through + /// the [events] stream. Future get ready; /// Creates a new [DirectoryWatcher] or [FileWatcher] monitoring [path], diff --git a/test/ready/shared.dart b/test/ready/shared.dart index a97fbee..6578c52 100644 --- a/test/ready/shared.dart +++ b/test/ready/shared.dart @@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; -import 'dart:io'; import 'package:test/test.dart'; @@ -63,15 +62,14 @@ void sharedTests() { expect(watcher.ready, doesNotComplete); }); - test('completes with error if directory does not exist', () async { + test('completes even if directory does not exist', () async { var watcher = createWatcher(path: 'does/not/exist'); // Subscribe to the events (else ready will never fire). - // ignore: unused_local_variable var subscription = watcher.events.listen((event) {}, onError: (error) {}); - // Expected ready completes with an error. - await expectLater(watcher.ready, throwsA(isA())); + // Expect ready still completes. + await watcher.ready; // Now unsubscribe. await subscription.cancel(); From 408a74e416d72f374820452bc540f273e78015e9 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 12 Jan 2022 10:41:56 +0000 Subject: [PATCH 7/8] Revert unnecessary changes in PollingDirectoryWatcher --- lib/src/directory_watcher/polling.dart | 7 ++++++- lib/src/utils.dart | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/src/directory_watcher/polling.dart b/lib/src/directory_watcher/polling.dart index e99e26b..3dcec05 100644 --- a/lib/src/directory_watcher/polling.dart +++ b/lib/src/directory_watcher/polling.dart @@ -9,6 +9,7 @@ import '../async_queue.dart'; import '../directory_watcher.dart'; import '../resubscribable.dart'; import '../stat.dart'; +import '../utils.dart'; import '../watch_event.dart'; /// Periodically polls a directory for changes. @@ -122,7 +123,11 @@ class _PollingDirectoryWatcher if (!isReady) { _readyCompleter.complete(); } - _events.addError(error, stackTrace); + if (!isDirectoryNotFoundException(error)) { + // It's some unknown error. Pipe it over to the event stream so the + // user can see it. + _events.addError(error, stackTrace); + } // When an error occurs, we end the listing normally, which has the // desired effect of marking all files that were in the directory as diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 4973029..9e1b708 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -5,6 +5,18 @@ import 'dart:async'; import 'dart:collection'; +import 'dart:io'; + +/// Returns `true` if [error] is a [FileSystemException] for a missing +/// directory. +bool isDirectoryNotFoundException(Object error) { + if (error is! FileSystemException) return false; + + // See dartbug.com/12461 and tests/standalone/io/directory_error_test.dart. + var notFoundCode = Platform.operatingSystem == 'windows' ? 3 : 2; + return error.osError?.errorCode == notFoundCode; +} + /// Returns the union of all elements in each set in [sets]. Set unionAll(Iterable> sets) => sets.fold({}, (union, set) => union.union(set)); From 6e49921df6b82f158726103506667fce7c2da398 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 12 Jan 2022 16:45:06 +0000 Subject: [PATCH 8/8] Add changelog entry --- CHANGELOG.md | 1 + lib/src/utils.dart | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d987225..af54045 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # 1.0.2-dev - Require Dart SDK >= 2.14 +- Ensure `DirectoryWatcher.ready` completes even when errors occur that close the watcher. # 1.0.1 diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 9e1b708..c2e71b3 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -4,7 +4,6 @@ import 'dart:async'; import 'dart:collection'; - import 'dart:io'; /// Returns `true` if [error] is a [FileSystemException] for a missing