Use onError over catchError
`Future.catchError` is more error prone in null safe dart than `onError`
because return type of the `Function` callback is not statically
enforced and `null` was a common default value which no longer works.
Migrate all calls from `catchError` to `onError` to demonstrate best
practices. The other usages in this package are mostly on
`Future<dynamic>` and mostly look safe, though I can't rule out a type
error in unusual situations.
There was at least one mistake made in the migration of this library -
the code to ignore errors when completing a `CancelableOperation` as an
error after it was already canceled would instead cause a type error to
surface. Fix this and add a test.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d43d717..0041245 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 2.5.1-dev
+
+* Fix an unhandled async error when completing a `CancelableCompleter` with a
+ `Future.error` after it was already canceled.
+
## 2.5.0
* Stable release for null safety.
diff --git a/lib/src/cancelable_operation.dart b/lib/src/cancelable_operation.dart
index c321078..b1cac55 100644
--- a/lib/src/cancelable_operation.dart
+++ b/lib/src/cancelable_operation.dart
@@ -189,7 +189,7 @@
final future = value as Future<T>;
if (_isCanceled) {
// Make sure errors from [value] aren't top-leveled.
- future.catchError((_) {});
+ future..then<void>((_) {}).onError((_, __) {});
return;
}
diff --git a/lib/src/future_group.dart b/lib/src/future_group.dart
index 3a6291f..2968b0b 100644
--- a/lib/src/future_group.dart
+++ b/lib/src/future_group.dart
@@ -77,7 +77,7 @@
if (!_closed) return null;
if (onIdleController != null) onIdleController.close();
_completer.complete(_values.whereType<T>().toList());
- }).catchError((Object error, StackTrace stackTrace) {
+ }).onError<Object>((error, stackTrace) {
if (_completer.isCompleted) return null;
_completer.completeError(error, stackTrace);
});
diff --git a/lib/src/null_stream_sink.dart b/lib/src/null_stream_sink.dart
index 34b100b..b61b2bc 100644
--- a/lib/src/null_stream_sink.dart
+++ b/lib/src/null_stream_sink.dart
@@ -53,7 +53,7 @@
// Don't top-level the error. This gives the user a change to call
// [close] or [done], and matches the behavior of a remote endpoint
// experiencing an error.
- ..catchError((_) {});
+ ..onError((_, __) {});
@override
void add(T data) {
diff --git a/lib/src/stream_sink_completer.dart b/lib/src/stream_sink_completer.dart
index 10e549d..c5a098c 100644
--- a/lib/src/stream_sink_completer.dart
+++ b/lib/src/stream_sink_completer.dart
@@ -166,7 +166,7 @@
sink
.addStream(_controller!.stream)
.whenComplete(sink.close)
- .catchError((_) {});
+ .onError((_, __) {});
}
// If the user has already asked when the sink is done, connect the sink's
diff --git a/lib/src/stream_sink_transformer/handler_transformer.dart b/lib/src/stream_sink_transformer/handler_transformer.dart
index d6f188e..5f9e0b4 100644
--- a/lib/src/stream_sink_transformer/handler_transformer.dart
+++ b/lib/src/stream_sink_transformer/handler_transformer.dart
@@ -101,7 +101,7 @@
_SafeCloseSink(StreamSink<T> inner) : super(inner);
@override
- Future close() => super.close().catchError((_) {});
+ Future close() => super.close().onError((_, __) {});
}
/// A function to pass as a [StreamTransformer]'s `handleDone` callback.
diff --git a/lib/src/stream_sink_transformer/stream_transformer_wrapper.dart b/lib/src/stream_sink_transformer/stream_transformer_wrapper.dart
index 2afcbff..373a650 100644
--- a/lib/src/stream_sink_transformer/stream_transformer_wrapper.dart
+++ b/lib/src/stream_sink_transformer/stream_transformer_wrapper.dart
@@ -40,7 +40,7 @@
// Ignore any errors that come from this call to [_inner.close]. The
// user can access them through [done] or the value returned from
// [this.close], and we don't want them to get top-leveled.
- _inner.close().catchError((_) {});
+ _inner.close().onError((_, __) {});
});
}
diff --git a/pubspec.yaml b/pubspec.yaml
index 3c8dd95..47c653f 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: async
-version: 2.5.0
+version: 2.5.1-dev
description: Utility functions and classes related to the 'dart:async' library.
homepage: https://www.github.com/dart-lang/async
diff --git a/test/cancelable_operation_test.dart b/test/cancelable_operation_test.dart
index 0e8d8d4..ebb8f49 100644
--- a/test/cancelable_operation_test.dart
+++ b/test/cancelable_operation_test.dart
@@ -195,6 +195,13 @@
expect(() => completer.complete(1), throwsStateError);
});
+ test('ignores completion errors after canceled', () async {
+ var completer = CancelableCompleter<int>();
+ completer.operation.value.whenComplete(expectAsync0(() {}, count: 0));
+ await completer.operation.cancel();
+ completer.complete(Future.error('error'));
+ });
+
test('fires valueOrCancellation with the given value', () {
var completer = CancelableCompleter();
expect(completer.operation.valueOrCancellation(1), completion(equals(1)));
diff --git a/test/result/result_future_test.dart b/test/result/result_future_test.dart
index 7171005..d211dca 100644
--- a/test/result/result_future_test.dart
+++ b/test/result/result_future_test.dart
@@ -35,7 +35,7 @@
// The completer calls its listeners asynchronously. We have to wait
// before we can access the result.
- return future.catchError((_) {}).then((_) {
+ return future.onError((_, __) {}).then((_) {
var error = future.result!.asError!;
expect(error.error, equals('error'));
expect(error.stackTrace, equals(trace));
diff --git a/test/stream_sink_transformer_test.dart b/test/stream_sink_transformer_test.dart
index e5f6baa..20dda61 100644
--- a/test/stream_sink_transformer_test.dart
+++ b/test/stream_sink_transformer_test.dart
@@ -80,9 +80,9 @@
// The futures shouldn't complete until the inner sink's close future
// completes.
var doneResult = ResultFuture(sink.done);
- doneResult.catchError((_) {});
+ doneResult.onError((_, __) {});
var closeResult = ResultFuture(sink.close());
- closeResult.catchError((_) {});
+ closeResult.onError((_, __) {});
await flushMicrotasks();
expect(doneResult.isComplete, isFalse);
expect(closeResult.isComplete, isFalse);
@@ -177,9 +177,9 @@
// The futures shouldn't complete until the inner sink's close future
// completes.
var doneResult = ResultFuture(sink.done);
- doneResult.catchError((_) {});
+ doneResult.onError((_, __) {});
var closeResult = ResultFuture(sink.close());
- closeResult.catchError((_) {});
+ closeResult.onError((_, __) {});
await flushMicrotasks();
expect(doneResult.isComplete, isFalse);
expect(closeResult.isComplete, isFalse);
