Update `Future.onError` extension method.
Now works better with futures that have been up-cast.
CoreLibraryReviewExempt: Rewrite of existing pure Dart function.
Change-Id: Iefd05b9cfd8ff0cc3e27fc1122a670030e0901ff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283680
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
diff --git a/sdk/lib/async/future.dart b/sdk/lib/async/future.dart
index 65155ef..dc002cc 100644
--- a/sdk/lib/async/future.dart
+++ b/sdk/lib/async/future.dart
@@ -748,8 +748,11 @@
/// and a stack trace for the error.
/// In the case of `onError`,
/// if the exception thrown is `identical` to the error argument to `onError`,
+ /// and it is thrown *synchronously*
/// the throw is considered a rethrow,
/// and the original stack trace is used instead.
+ /// To rethrow with the same stack trace in an asynchronous callback,
+ /// use [Error.throwWithStackTrace].
///
/// If the callback returns a [Future],
/// the future returned by `then` will be completed with
@@ -1054,12 +1057,20 @@
Future<T> onError<E extends Object>(
FutureOr<T> handleError(E error, StackTrace stackTrace),
{bool test(E error)?}) {
- // There are various ways to optimize this to avoid the double is E/as E
- // type check, but for now we are not optimizing the error path.
- return this.catchError(
- (Object error, StackTrace stackTrace) =>
- handleError(error as E, stackTrace),
- test: (Object error) => error is E && (test == null || test(error)));
+ FutureOr<T> onError(Object error, StackTrace stackTrace) {
+ if (error is! E || test != null && !test(error)) {
+ // Counts as rethrow, preserves stack trace.
+ throw error;
+ }
+ return handleError(error, stackTrace);
+ }
+ if (this is _Future<Object?>) {
+ // Internal method working like `catchError`,
+ // but allows specifying a different result future type.
+ return unsafeCast<_Future<T>>(this)._safeOnError<T>(onError);
+ }
+
+ return this.then<T>((T value) => value, onError: onError);
}
/// Completely ignores this future and its result.
@@ -1110,7 +1121,7 @@
/// A way to produce Future objects and to complete them later
/// with a value or error.
///
-/// Most of the time, the simplest way to create a future is to just use
+/// Most of the time, the simples t way to create a future is to just use
/// one of the [Future] constructors to capture the result of a single
/// asynchronous computation:
/// ```dart
diff --git a/sdk/lib/async/future_impl.dart b/sdk/lib/async/future_impl.dart
index f8fb750..812fff4 100644
--- a/sdk/lib/async/future_impl.dart
+++ b/sdk/lib/async/future_impl.dart
@@ -349,6 +349,23 @@
return result;
}
+ // Used by extension method `onError` to up-cast the result to `R`.
+ //
+ // Not public because we cannot statically ensure that [R] is a supertype
+ // of [T].
+ //
+ // Avoids needing to allocate an extra value handler to do the up cast,
+ // which is needed if using `.then`.
+ Future<R> _safeOnError<R>(FutureOr<R> Function(Object, StackTrace) onError) {
+ assert(this is _Future<R>); // Is up-cast.
+ _Future<R> result = _Future<R>();
+ if (!identical(result._zone, _rootZone)) {
+ onError = result._zone.registerBinaryCallback(onError);
+ }
+ _addListener(_FutureListener<T, R>.catchError(result, onError, null));
+ return result;
+ }
+
Future<T> whenComplete(dynamic action()) {
_Future<T> result = new _Future<T>();
if (!identical(result._zone, _rootZone)) {
diff --git a/tests/lib/async/future_onerror_test.dart b/tests/lib/async/future_onerror_test.dart
index ea5ffab..307f76e 100644
--- a/tests/lib/async/future_onerror_test.dart
+++ b/tests/lib/async/future_onerror_test.dart
@@ -2,114 +2,213 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-import 'package:async_helper/async_helper.dart';
+import "dart:async";
+
+import "package:async_helper/async_helper.dart";
import "package:expect/expect.dart";
main() {
+ // Values used.
+ var error = StateError("test");
var stack = StackTrace.fromString("for testing");
- var error = Object();
- var stateError = StateError("test");
+ var expectedError = AsyncError(error, stack);
+
+ var otherError = ArgumentError("Other error");
+ var expectedOtherError = AsyncError(otherError, StackTrace.empty);
+
+ var errorFuture = Future<int?>.error(error, stack)..ignore();
+ var nonNativeErrorFuture = NonNativeFuture<int?>._(errorFuture);
asyncStart();
- {
- // Match any error.
+ /// Perform `onError` on [future] with arguments, and expect [result].
+ ///
+ /// If [result] is [AsyncError], expecte an error result with
+ /// that error and stack trace.
+ /// If [result.stackTrace] is [StackTrace.empty],
+ /// check that the stack trace is *not* the original [stack].
+ ///
+ /// Otherwise expect the future to complete with value equal to [result].
+ void test<E extends Object, T>(
+ String testName,
+ Future<T> future,
+ FutureOr<T> Function(E, StackTrace) onError, {
+ bool Function(E)? test,
+ Object? result,
+ }) {
asyncStart();
- var future = Future.error(error, stack);
- future.onError((e, s) {
- Expect.identical(error, e);
- Expect.identical(stack, s);
- asyncEnd();
- });
+ var resultFuture = future.onError(onError, test: test);
+
+ if (result is AsyncError) {
+ resultFuture.then((value) {
+ Expect.fail("$testName: Did not throw, value: $value");
+ }, onError: (Object error, StackTrace stackTrace) {
+ Expect.identical(result.error, error, testName);
+ if (result.stackTrace != StackTrace.empty) {
+ Expect.identical(result.stackTrace, stackTrace, testName);
+ } else {
+ Expect.notEquals(stack, stackTrace, testName);
+ }
+ asyncEnd();
+ });
+ } else {
+ resultFuture.then((value) {
+ Expect.equals(result, value);
+ asyncEnd();
+ }, onError: (error, stackTrace) {
+ Expect.fail("$testName: Threw $error");
+ });
+ }
}
- {
- // With matching test.
- asyncStart();
- var future = Future.error(error, stack);
- future.onError((e, s) {
- Expect.identical(error, e);
- Expect.identical(stack, s);
- asyncEnd();
- }, test: (o) => true);
+ // Go through all the valid permutations of the following options:
+ //
+ // * Native future or non-native future.
+ // * The error type parameter matches the error or not.
+ // * If type matches, test function is provided or not.
+ // * If test function provided, it returns either true or false.
+ // * If type and test both succeeds,
+ // * Error handler is synchronous or asynchronous
+ // * Error handler returns a value, throws new error,
+ // or rethrows same error.
+ // * The future is up-cast to a supertype or not when `onError` is called.
+ // (Since `onError` is an extension method, it affects the method.)
+ //
+ // The expected result is computed for each combination,
+ // and checked against the behavior:
+ // * If type or test fails to match,
+ // the original error and stack is the error result
+ // * If type matches and test doesn't reject,
+ // the error handler is invoked.
+ // * If it returns a value, that becomes the result.
+ // * If upcast, a return value of the supertype is accepted.
+ // * If it throws a new error, that is the error result.
+ // * If it throws the original error *synchronously*,
+ // the original stack trace is retained.
+ // * If it throws the original error *asynchronously*,
+ // the original stack trace is not expected to be retained.
+ for (var native in [true, false]) {
+ var nativity = native ? "native" : "non-native";
+ var future = native ? errorFuture : nonNativeErrorFuture;
+ for (var upcast in [true, false]) {
+ var upcastText = upcast ? " as Future<num?>" : "";
+ // "fails" means not to match type parameter.
+ // Rest means match type parameter, a bool is what `test` returns,
+ // `null` means no test function.
+ for (var testKind in ["fails", true, false, null]) {
+ var matches = testKind == true || testKind == null;
+ // If `matches` is false, the error handler should not be invoked.
+ // Don't bother with creating sync/async versions of it then,
+ // just one which fails if called.
+ for (var async in [if (matches) true, false]) {
+ var asyncText = async ? "async" : "sync";
+ // Action of the handler body. If test doesn't match, it will fail.
+ for (var action
+ in matches ? ["return", "throw", "rethrow"] : ["fail"]) {
+ // Called to provide the necessary types as type parameters.
+ void doTest<E extends Object, T>() {
+ String testName =
+ "$nativity Future<int?>.error(StateError(..))$upcastText ";
+
+ bool Function(E)? testFunction;
+ switch (testKind) {
+ case "fails":
+ testFunction = (E value) {
+ Expect.fail("$testName: Matched error");
+ throw "unreachable"; // Expect.fail throws.
+ };
+ break;
+ case true:
+ case false:
+ var testResult = testKind == true;
+ testFunction = (E value) => testResult;
+ testName +=
+ "matches type $E ${testResult ? "and test" : "but not test"}";
+ break;
+ default:
+ testName += "matches type with no test";
+ }
+ Object? expectation = expectedError;
+ FutureOr<T> Function(E, StackTrace) onError;
+ switch (action) {
+ case "return":
+ expectation = upcast ? 3.14 : 42;
+ testName += " and returns $expectation";
+ onError = (E e, StackTrace s) {
+ return expectation as T;
+ };
+ break;
+ case "throw":
+ expectation = expectedOtherError;
+ onError = (E e, StackTrace s) {
+ throw otherError;
+ };
+ testName += " and throws new error";
+ break;
+ case "rethrow":
+ onError = (E e, StackTrace s) {
+ throw e;
+ };
+ testName += " and rethrows";
+ // Asynchronous callbacks rethrowing the error
+ // doesn't count as a rethrow, only synchronous throws.
+ if (async) expectation = AsyncError(error, StackTrace.empty);
+ break;
+ case "fail":
+ onError = (E e, StackTrace s) {
+ Expect.fail("$testName: Matched error");
+ throw "unreachable"; // Expect.fail throws.
+ };
+ testName += " and rethrows original error";
+ break;
+ default:
+ throw "unreachable";
+ }
+ if (async) {
+ testName += " asynchronously";
+ var originalOnError = onError;
+ onError = (E e, StackTrace s) async => originalOnError(e, s);
+ }
+ test<E, T>(testName, future as Future<T>, onError,
+ test: testFunction, result: expectation);
+ }
+
+ // Find the types to use.
+ if (testKind == "fails") {
+ if (upcast) {
+ doTest<Exception, num?>();
+ } else {
+ doTest<Exception, int?>();
+ }
+ } else {
+ if (upcast) {
+ doTest<StateError, num?>();
+ } else {
+ doTest<StateError, int?>();
+ }
+ }
+ }
+ }
+ }
+ }
}
- {
- // With failing test.
- asyncStart();
- var future = Future.error(error, stack);
- var onErrorFuture = future.onError((e, s) {
- Expect.fail('unreachable');
- }, test: (o) => false);
- onErrorFuture.catchError((e, s) {
- Expect.identical(error, e);
- Expect.identical(stack, s);
- asyncEnd();
- });
- }
-
- {
- // With matching type.
- asyncStart();
- var future = Future.error(stateError, stack);
- future.onError<StateError>((e, s) {
- Expect.identical(stateError, e);
- Expect.identical(stack, s);
- asyncEnd();
- });
- }
-
- {
- // With non-matching type.
- asyncStart();
- var future = Future.error(stateError, stack);
- var onErrorFuture = future.onError<ArgumentError>((e, s) {
- Expect.fail('unreachable');
- });
- onErrorFuture.catchError((e, s) {
- Expect.identical(stateError, e);
- Expect.identical(stack, s);
- asyncEnd();
- });
- }
-
- {
- // With non-matching type and matching test.
- asyncStart();
- var future = Future.error(stateError, stack);
- var onErrorFuture = future.onError<ArgumentError>((e, s) {
- Expect.fail('unreachable');
- }, test: (ArgumentError e) => true);
- onErrorFuture.catchError((e, s) {
- Expect.identical(stateError, e);
- Expect.identical(stack, s);
- asyncEnd();
- });
- }
-
- {
- // With matching type and matching test.
- asyncStart();
- var future = Future.error(stateError, stack);
- future.onError<StateError>((e, s) {
- Expect.identical(stateError, e);
- Expect.identical(stack, s);
- asyncEnd();
- }, test: (StateError e) => true);
- }
-
- {
- // With matching type and non-matching test.
- asyncStart();
- var future = Future.error(stateError, stack);
- var onErrorFuture = future.onError<StateError>((e, s) {
- Expect.fail('unreachable');
- }, test: (StateError e) => false);
- onErrorFuture.catchError((e, s) {
- Expect.identical(stateError, e);
- Expect.identical(stack, s);
- asyncEnd();
- });
- }
asyncEnd();
}
+
+class NonNativeFuture<T> implements Future<T> {
+ Future<T> _original;
+ NonNativeFuture.value(T value) : _original = Future<T>.value(value);
+ NonNativeFuture.error(Object error, [StackTrace? stack])
+ : _original = Future<T>.error(error, stack)..ignore();
+ NonNativeFuture._(this._original);
+ Future<R> then<R>(FutureOr<R> Function(T) handleValue, {Function? onError}) =>
+ NonNativeFuture<R>._(_original.then(handleValue, onError: onError));
+ Future<T> catchError(Function handleError, {bool Function(Object)? test}) =>
+ NonNativeFuture<T>._(_original.catchError(handleError, test: test));
+ Future<T> whenComplete(FutureOr<void> Function() onDone) =>
+ NonNativeFuture<T>._(_original.whenComplete(onDone));
+ Future<T> timeout(Duration timeLimit, {FutureOr<T> Function()? onTimeout}) =>
+ NonNativeFuture<T>._(_original.timeout(timeLimit, onTimeout: onTimeout));
+ Stream<T> asStream() => _original.asStream();
+}
