Add onError extension method to Future.

Provides better typing than `catchError`.

Change-Id: If0d4487b7c3a499160fb719740a1d65c0545024d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151512
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>

CHANGELOG.md

@@ -20,11 +20,19 @@
 
 ### Core libraries
 
+#### `dart:async`
+
+* Adds extension method `onError` on `Future` to allow better typing
+  of error handling.
+
 #### `dart:collection`
 
 * Added `UnmodifiableSetView` class, which allows users to guarantee that
   methods that could change underlying `Set` instance can not be invoked.
 
+* `LinkedList` made it explicit that elements are compared by identity,
+  and updated `contains` to take advantage of this.
+
 #### `dart:core`
 
 * Added `unmodifiable` constructor to class `Set`, which allows users to create
@@ -41,11 +49,6 @@
   constructors, a name which can be associated with the port and displayed in
   tooling.
 
-#### `dart:collection`
-
-* `LinkedList` made it explicit that elements are compared by identity,
-  and updated `contains` to take advantage of this.
-
 #### `dart:html`
 
 * `EventStreamSubscription.cancel` has been updated to retain its synchronous

pkg/front_end/messages.yaml

@@ -3853,10 +3853,10 @@ CantUseDeferredPrefixAsConstant:
     from the import.
   analyzerCode: CONST_DEFERRED_CLASS
   script: |
-    import "dart:core" deferred as prefix;
+    import "dart:convert" deferred as prefix;
 
     main() {
-      const prefix.Object();
+      const prefix.JsonCodec();
     }
 
 CyclicRedirectingFactoryConstructors:

pkg/front_end/test/extensions/data/ambiguous/lib1.dart

@@ -4,7 +4,11 @@
 
 // @dart = 2.9
 
-/*library: scope=[AmbiguousExtension1,AmbiguousExtension2,UnambiguousExtension1]*/
+/*library: scope=[
+  AmbiguousExtension1,
+  AmbiguousExtension2,
+  UnambiguousExtension1,
+  async.dart.FutureExtensions]*/
 
 /*class: AmbiguousExtension1:
  builder-name=AmbiguousExtension1,

pkg/front_end/test/extensions/data/ambiguous/lib2.dart

@@ -4,7 +4,11 @@
 
 // @dart = 2.9
 
-/*library: scope=[AmbiguousExtension1,AmbiguousExtension2,UnambiguousExtension2]*/
+/*library: scope=[
+  AmbiguousExtension1,
+  AmbiguousExtension2,
+  UnambiguousExtension2,
+  async.dart.FutureExtensions]*/
 
 /*class: AmbiguousExtension1:
  builder-name=AmbiguousExtension1,

pkg/front_end/test/extensions/data/ambiguous/main.dart

@@ -5,13 +5,13 @@
 // @dart = 2.9
 
 /*library: scope=[
- lib1.dart.AmbiguousExtension1,
- lib1.dart.AmbiguousExtension2,
- lib1.dart.UnambiguousExtension1,
- lib2.dart.AmbiguousExtension1,
- lib2.dart.AmbiguousExtension2,
- lib2.dart.UnambiguousExtension2]
-*/
+  async.dart.FutureExtensions,
+  lib1.dart.AmbiguousExtension1,
+  lib1.dart.AmbiguousExtension2,
+  lib1.dart.UnambiguousExtension1,
+  lib2.dart.AmbiguousExtension1,
+  lib2.dart.AmbiguousExtension2,
+  lib2.dart.UnambiguousExtension2]*/
 
 import 'lib1.dart';
 import 'lib2.dart';

pkg/front_end/test/extensions/data/as_show/lib.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[Extension1]*/
+/*library: scope=[
+  Extension1,
+  async.dart.FutureExtensions]*/
 
 /*class: Extension1:
  builder-name=Extension1,

pkg/front_end/test/extensions/data/as_show/main.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[lib.dart.Extension1,origin.dart.Extension2]*/
+/*library: scope=[
+  async.dart.FutureExtensions,
+  lib.dart.Extension1,
+  origin.dart.Extension2]*/
 
 import 'lib.dart' as lib1;
 import 'lib.dart' show Extension1;

pkg/front_end/test/extensions/data/as_show/origin.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[Extension2]*/
+/*library: scope=[
+  Extension2,
+  async.dart.FutureExtensions]*/
 
 /*class: Extension2:
  builder-name=Extension2,

pkg/front_end/test/extensions/data/explicit_this.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2]*/
+/*library: scope=[
+  A2,
+  async.dart.FutureExtensions]*/
 
 class A1 {
   Object field;

pkg/front_end/test/extensions/data/export_twice/lib1.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[E]*/
+/*library: scope=[
+  E,
+  async.dart.FutureExtensions]*/
 
 class A {}
 

pkg/front_end/test/extensions/data/export_twice/lib2.dart

@@ -2,6 +2,8 @@
 // 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.
 
+/*library: scope=[async.dart.FutureExtensions]*/
+
 // @dart = 2.9
 
 export 'lib1.dart' show E;

pkg/front_end/test/extensions/data/export_twice/main.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[lib1.dart.E]*/
+/*library: scope=[
+  async.dart.FutureExtensions,
+  lib1.dart.E]*/
 
 import 'lib1.dart';
 import 'lib2.dart';

pkg/front_end/test/extensions/data/export_unnamed/lib.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[NamedExtension,_extension#0]*/
+/*library: scope=[
+  NamedExtension,
+  _extension#0,
+  async.dart.FutureExtensions]*/
 
 /*class: _extension#0:
  builder-name=_extension#0,

pkg/front_end/test/extensions/data/export_unnamed/main.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[lib.dart.NamedExtension]*/
+/*library: scope=[
+  async.dart.FutureExtensions,
+  lib.dart.NamedExtension]*/
 
 import 'lib.dart';
 

pkg/front_end/test/extensions/data/extension_on_type_variable.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[GeneralGeneric]*/
+/*library: scope=[
+  GeneralGeneric,
+  async.dart.FutureExtensions]*/
 
 /*class: GeneralGeneric:
  builder-name=GeneralGeneric,

pkg/front_end/test/extensions/data/implicit_this.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2]*/
+/*library: scope=[
+  A2,
+  async.dart.FutureExtensions]*/
 
 class A1 {
   Object field;

pkg/front_end/test/extensions/data/instance_members.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2,B2]*/
+/*library: scope=[
+  A2,
+  B2,
+  async.dart.FutureExtensions]*/
 
 class A1 {}
 

pkg/front_end/test/extensions/data/named_declarations.dart

@@ -4,7 +4,12 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2,B2,B3,B4]*/
+/*library: scope=[
+  A2,
+  B2,
+  B3,
+  B4,
+  async.dart.FutureExtensions]*/
 
 class A1 {}
 

pkg/front_end/test/extensions/data/other_kinds.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2]*/
+/*library: scope=[
+  A2,
+  async.dart.FutureExtensions]*/
 
 class A1 {
   int _instanceField;

pkg/front_end/test/extensions/data/part/main.dart

@@ -4,7 +4,8 @@
 
 /*library: scope=[
   Extension,
-  _extension#0]*/
+  _extension#0,
+  async.dart.FutureExtensions]*/
 
 part 'part.dart';
 

pkg/front_end/test/extensions/data/patching/main.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[origin.dart.Extension,origin.dart.GenericExtension]*/
+/*library: scope=[
+  async.dart.FutureExtensions,
+  origin.dart.Extension,
+  origin.dart.GenericExtension]*/
 
 // ignore: uri_does_not_exist
 import 'dart:test';

pkg/front_end/test/extensions/data/patching/origin.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[Extension,GenericExtension]*/
+/*library: scope=[
+  Extension,
+  GenericExtension,
+  async.dart.FutureExtensions]*/
 
 /*class: Extension:
  builder-name=Extension,

pkg/front_end/test/extensions/data/prefix/lib1.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[HiddenExtension1,ShownExtension1]*/
+/*library: scope=[
+  HiddenExtension1,
+  ShownExtension1,
+  async.dart.FutureExtensions]*/
 
 /*class: ShownExtension1:
  builder-name=ShownExtension1,

pkg/front_end/test/extensions/data/prefix/lib2.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[HiddenExtension2,ShownExtension2]*/
+/*library: scope=[
+  HiddenExtension2,
+  ShownExtension2,
+  async.dart.FutureExtensions]*/
 
 /*class: HiddenExtension2:
  builder-name=HiddenExtension2,

pkg/front_end/test/extensions/data/prefix/lib3.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[ShownExtension3]*/
+/*library: scope=[
+  ShownExtension3,
+  async.dart.FutureExtensions]*/
 
 /*class: ShownExtension3:
  builder-name=ShownExtension3,

pkg/front_end/test/extensions/data/prefix/main.dart

@@ -5,10 +5,10 @@
 // @dart = 2.9
 
 /*library: scope=[
- lib1.dart.ShownExtension1,
- lib2.dart.ShownExtension2,
- lib3.dart.ShownExtension3]
-*/
+  async.dart.FutureExtensions,
+  lib1.dart.ShownExtension1,
+  lib2.dart.ShownExtension2,
+  lib3.dart.ShownExtension3]*/
 
 import 'lib1.dart' as lib1 show ShownExtension1;
 import 'lib2.dart' as lib2 hide HiddenExtension2;

pkg/front_end/test/extensions/data/reexport/lib.dart

@@ -2,6 +2,8 @@
 // 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.
 
+/*library: scope=[async.dart.FutureExtensions]*/
+
 // @dart = 2.9
 
 export 'lib1.dart';

pkg/front_end/test/extensions/data/reexport/lib1.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[ClashingExtension,UniqueExtension1]*/
+/*library: scope=[
+  ClashingExtension,
+  UniqueExtension1,
+  async.dart.FutureExtensions]*/
 
 /*class: ClashingExtension:
  builder-name=ClashingExtension,

pkg/front_end/test/extensions/data/reexport/lib2.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[ClashingExtension,UniqueExtension2]*/
+/*library: scope=[
+  ClashingExtension,
+  UniqueExtension2,
+  async.dart.FutureExtensions]*/
 
 /*class: ClashingExtension:
  builder-name=ClashingExtension,

pkg/front_end/test/extensions/data/reexport/main.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[lib1.dart.UniqueExtension1,lib2.dart.UniqueExtension2]*/
+/*library: scope=[
+  async.dart.FutureExtensions,
+  lib1.dart.UniqueExtension1,
+  lib2.dart.UniqueExtension2]*/
 
 import 'lib.dart';
 

pkg/front_end/test/extensions/data/show_hide/lib1.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[HiddenExtension1,ShownExtension1]*/
+/*library: scope=[
+  HiddenExtension1,
+  ShownExtension1,
+  async.dart.FutureExtensions]*/
 
 /*class: ShownExtension1:
  builder-name=ShownExtension1,

pkg/front_end/test/extensions/data/show_hide/lib2.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[HiddenExtension2,ShownExtension2]*/
+/*library: scope=[
+  HiddenExtension2,
+  ShownExtension2,
+  async.dart.FutureExtensions]*/
 
 /*class: HiddenExtension2:
  builder-name=HiddenExtension2,

pkg/front_end/test/extensions/data/show_hide/main.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[lib1.dart.ShownExtension1,lib2.dart.ShownExtension2]*/
+/*library: scope=[
+  async.dart.FutureExtensions,
+  lib1.dart.ShownExtension1,
+  lib2.dart.ShownExtension2]*/
 
 import 'lib1.dart' show ShownExtension1;
 import 'lib2.dart' hide HiddenExtension2;

pkg/front_end/test/extensions/data/static_members.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2,B2]*/
+/*library: scope=[
+  A2,
+  B2,
+  async.dart.FutureExtensions]*/
 
 class A1 {}
 

pkg/front_end/test/extensions/data/super.dart

@@ -4,7 +4,9 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2]*/
+/*library: scope=[
+  A2,
+  async.dart.FutureExtensions]*/
 
 class A1 {
   method1() {}

pkg/front_end/test/extensions/data/type_variables.dart

@@ -4,7 +4,11 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2,A3,A4]*/
+/*library: scope=[
+  A2,
+  A3,
+  A4,
+  async.dart.FutureExtensions]*/
 
 class A1<T> {}
 

pkg/front_end/test/extensions/data/unnamed_declarations.dart

@@ -4,7 +4,13 @@
 
 // @dart = 2.9
 
-/*library: scope=[_extension#0,_extension#1,_extension#2,_extension#3,_extension#4]*/
+/*library: scope=[
+  _extension#0,
+  _extension#1,
+  _extension#2,
+  _extension#3,
+  _extension#4,
+  async.dart.FutureExtensions]*/
 
 class A1 {}
 

pkg/front_end/test/extensions/data/use_as_type.dart

@@ -4,7 +4,10 @@
 
 // @dart = 2.9
 
-/*library: scope=[A2,B2]*/
+/*library: scope=[
+  A2,
+  B2,
+  async.dart.FutureExtensions]*/
 
 class A1 {}
 

sdk/lib/async/future.dart

@@ -698,6 +698,80 @@ abstract class Future<T> {
   Future<T> timeout(Duration timeLimit, {FutureOr<T> onTimeout()?});
 }
 
+/// Convenience methods on futures.
+///
+/// Adds functionality to futures which makes it easier to
+/// write well-typed asynchronous code.
+@Since("2.12")
+extension FutureExtensions<T> on Future<T> {
+  /// Handles errors on this future.
+  ///
+  /// Catches errors of type [E] that this future complete with.
+  /// If [test] is supplied, only catches errors of type [E]
+  /// where [test] returns `true`.
+  /// If [E] is [Object], then all errors are potentially caught,
+  /// depending only on a supplied [test].toString()
+  ///
+  /// If the error is caught,
+  /// the returned future completes with the result of calling [handleError]
+  /// with the error and stack trace.
+  /// This result must be a value of the same type that this future
+  /// could otherwise complete with.
+  /// For example, if this future cannot complete with `null`,
+  /// then [handleError] also cannot return `null`.
+  /// Example:
+  /// ```dart
+  /// Future<T> retryOperation<T>(Future<T> operation(), T onFailure()) =>
+  ///     operation().onError<RetryException>((e, s) {
+  ///       if (e.canRetry) {
+  ///         return retryOperation(operation, onFailure);
+  ///       }
+  ///       return onFailure();
+  ///     });
+  /// ```
+  ///
+  /// If [handleError] throws, the returned future completes
+  /// with the thrown error and stack trace,
+  /// except that if it throws the *same* error object again,
+  /// then it is considered a "rethrow"
+  /// and the original stack trace is retained.
+  /// This can be used as an alternative to skipping the
+  /// error in [test].
+  /// Example:
+  /// ```dart
+  /// // Unwraps an an exceptions cause, if it has one.
+  /// someFuture.onError<SomeException>((e, _) {
+  ///   throw e.cause ?? e;
+  /// });
+  /// // vs.
+  /// someFuture.onError<SomeException>((e, _) {
+  ///   throw e.cause!;
+  /// }, test: (e) => e.cause != null);
+  /// ```
+  ///
+  /// If the error is not caught, the returned future
+  /// completes with the same result, value or error,
+  /// as this future.
+  ///
+  /// This method is effectively a more precisely typed version
+  /// of [Future.catchError].
+  /// It makes it easy to catch specific error types,
+  /// and requires a correctly typed error handler function,
+  /// rather than just [Function].
+  /// Because of this, the error handlers must accept
+  /// the stack trace argument.
+  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)));
+  }
+}
+
 /// Thrown when a scheduled timeout happens while waiting for an async result.
 class TimeoutException implements Exception {
   /// Description of the cause of the timeout.

sdk/lib/core/core.dart

@@ -168,6 +168,8 @@ import "dart:typed_data" show Uint8List;
 
 @Since("2.1")
 export "dart:async" show Future, Stream;
+@Since("2.12")
+export "dart:async" show FutureExtensions;
 
 part "annotations.dart";
 part "bigint.dart";

tests/language/constants_2018/constant_type_literal_test.dart

@@ -8,7 +8,7 @@ import "dart:core";
 import "dart:core" as core;
 // No reloading support for deferred loading.
 // See https://github.com/dart-lang/sdk/issues/33118.
-import "dart:core" deferred as dcore; //# 01: compile-time error
+import "dart:core" deferred as dcore show int //# 01: compile-time error
 
 // Declares F function type alias, M mixin and C class.
 import "constant_type_literal_types.dart";

tests/language/deferred/import_core_test.dart

@@ -5,7 +5,7 @@
 // Nothing in the language spec explicitly prohibits a deferred import of
 // 'dart:core'.  Make sure it doesn't lead to any strange behavior.
 
-import "dart:core" deferred as core;
+import "dart:core" deferred as core show Object;
 
 main() {
   core.loadLibrary().then((_) => null);

tests/language_2/constants_2018/constant_type_literal_test.dart

@@ -8,7 +8,7 @@ import "dart:core";
 import "dart:core" as core;
 // No reloading support for deferred loading.
 // See https://github.com/dart-lang/sdk/issues/33118.
-import "dart:core" deferred as dcore; //# 01: compile-time error
+import "dart:core" deferred as dcore show int; //# 01: compile-time error
 
 // Declares F function type alias, M mixin and C class.
 import "constant_type_literal_types.dart";

tests/language_2/deferred/import_core_test.dart

@@ -5,7 +5,7 @@
 // Nothing in the language spec explicitly prohibits a deferred import of
 // 'dart:core'.  Make sure it doesn't lead to any strange behavior.
 
-import "dart:core" deferred as core;
+import "dart:core" deferred as core show Object;
 
 main() {
   core.loadLibrary().then((_) => null);

tests/lib/async/catch_errors.dart

@@ -1,3 +1,7 @@
+// Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
 library catch_errors;
 
 import 'dart:async';

tests/lib/async/future_onerror_test.dart

@@ -0,0 +1,115 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// 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 "package:expect/expect.dart";
+
+main() {
+  var stack = StackTrace.fromString("for testing");
+  var error = Object();
+  var stateError = StateError("test");
+
+  asyncStart();
+
+  {
+    // Match any error.
+    asyncStart();
+    var future = Future.error(error, stack);
+    future.onError((e, s) {
+      Expect.identical(error, e);
+      Expect.identical(stack, s);
+      asyncEnd();
+    });
+  }
+
+  {
+    // 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);
+  }
+
+  {
+    // 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();
+}