Reland "Make throwing Zone.uncaughtError handlers propagate the error to their parent zone."

This reverts commit f0a8b63402.

Reason for revert: The Flutter SDK issues have been resolved - see https://github.com/flutter/flutter/issues/80969 and https://github.com/flutter/flutter/pull/81014

Original change's description:
> Revert "Make throwing Zone.uncaughtError handlers propagate the error to their parent zone."
>
> This reverts commit 88a351f3d2.
>
> This broke the Dart SDK -> Flutter Engine roller. Flutter issue at https://github.com/flutter/flutter/issues/80969
>
> Change-Id: Idaf255a730c7b6054e6cd929b6770dbe66860151
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196561
> Reviewed-by: Zach Anderson <zra@google.com>
> Commit-Queue: Zach Anderson <zra@google.com>

# Not skipping CQ checks because this is a reland.

Change-Id: Icd98c550c63160f35cc5da40af7ca6bf2cbf180e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196621
Reviewed-by: Dan Field <dnfield@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Dan Field <dnfield@google.com>

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ### Core libraries
 
+#### `dart:async`
+
+* The uncaught error handlers of `Zone`s are now run in the parent zone
+  of the zone where they were declared. This prevents a throwing handler
+  from causing an infinite loop by repeatedly triggering itself.
+
 #### `dart:core`
 
 *   The native `DateTime` class now better handles local time around

sdk/lib/async/zone.dart

@@ -17,6 +17,15 @@ typedef R ZoneBinaryCallback<R, T1, T2>(T1 arg1, T2 arg2);
 ///
 /// The [error] and [stackTrace] are the error and stack trace that
 /// was uncaught in [zone].
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
+///
+/// If the uncaught error handler throws, the error will be passed
+/// to `parent.handleUncaughtError`. If the thrown object is [error],
+/// the throw is considered a re-throw and the original [stackTrace]
+/// is retained. This allows an asynchronous error to leave the error zone.
 typedef HandleUncaughtErrorHandler = void Function(Zone self,
     ZoneDelegate parent, Zone zone, Object error, StackTrace stackTrace);
 
@@ -34,6 +43,10 @@ typedef HandleUncaughtErrorHandler = void Function(Zone self,
 /// to call [f] in the current zone, [zone].
 /// A custom handler can do things before, after or instead of
 /// calling [f].
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef RunHandler = R Function<R>(
     Zone self, ZoneDelegate parent, Zone zone, R Function() f);
 
@@ -51,6 +64,10 @@ typedef RunHandler = R Function<R>(
 /// to call [f] with argument [arg] in the current zone, [zone].
 /// A custom handler can do things before, after or instead of
 /// calling [f].
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef RunUnaryHandler = R Function<R, T>(
     Zone self, ZoneDelegate parent, Zone zone, R Function(T arg) f, T arg);
 
@@ -68,6 +85,10 @@ typedef RunUnaryHandler = R Function<R, T>(
 /// to call [f] with arguments [arg1] and [arg2] in the current zone, [zone].
 /// A custom handler can do things before, after or instead of
 /// calling [f].
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef RunBinaryHandler = R Function<R, T1, T2>(Zone self, ZoneDelegate parent,
     Zone zone, R Function(T1 arg1, T2 arg2) f, T1 arg1, T2 arg2);
 
@@ -85,6 +106,10 @@ typedef RunBinaryHandler = R Function<R, T1, T2>(Zone self, ZoneDelegate parent,
 /// or another function replacing [f],
 /// typically by wrapping [f] in a function
 /// which does something extra before and after invoking [f]
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef RegisterCallbackHandler = ZoneCallback<R> Function<R>(
     Zone self, ZoneDelegate parent, Zone zone, R Function() f);
 
@@ -102,6 +127,10 @@ typedef RegisterCallbackHandler = ZoneCallback<R> Function<R>(
 /// or another function replacing [f],
 /// typically by wrapping [f] in a function
 /// which does something extra before and after invoking [f]
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef RegisterUnaryCallbackHandler = ZoneUnaryCallback<R, T> Function<R, T>(
     Zone self, ZoneDelegate parent, Zone zone, R Function(T arg) f);
 
@@ -137,6 +166,12 @@ typedef RegisterBinaryCallbackHandler
 /// to replace the original error and stack trace,
 /// or an [AsyncError] containing a replacement error and stack trace
 /// which will be used to replace the originals.
+///
+/// The error callback handler must not throw.
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef AsyncError? ErrorCallbackHandler(Zone self, ZoneDelegate parent,
     Zone zone, Object error, StackTrace? stackTrace);
 
@@ -155,6 +190,10 @@ typedef AsyncError? ErrorCallbackHandler(Zone self, ZoneDelegate parent,
 /// and then call `parent.scheduleMicrotask(zone, replacement)`.
 /// or it can implement its own microtask scheduling queue, which typically
 /// still depends on `parent.scheduleMicrotask` to as a way to get started.
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef void ScheduleMicrotaskHandler(
     Zone self, ZoneDelegate parent, Zone zone, void f());
 
@@ -177,6 +216,10 @@ typedef void ScheduleMicrotaskHandler(
 ///
 /// The function should return a [Timer] object which can be used
 /// to inspect and control the scheduled timer callback.
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef Timer CreateTimerHandler(
     Zone self, ZoneDelegate parent, Zone zone, Duration duration, void f());
 
@@ -199,6 +242,10 @@ typedef Timer CreateTimerHandler(
 ///
 /// The function should return a [Timer] object which can be used
 /// to inspect and control the scheduled timer callbacks.
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef Timer CreatePeriodicTimerHandler(Zone self, ZoneDelegate parent,
     Zone zone, Duration period, void f(Timer timer));
 
@@ -214,6 +261,10 @@ typedef Timer CreatePeriodicTimerHandler(Zone self, ZoneDelegate parent,
 ///
 /// The custom handler can intercept print operations and
 /// redirect them to other targets than the console.
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef void PrintHandler(
     Zone self, ZoneDelegate parent, Zone zone, String line);
 
@@ -235,6 +286,10 @@ typedef void PrintHandler(
 /// values before calling `parent.fork(zone, specification, zoneValues)`,
 /// but it has to call the [parent]'s [ZoneDelegate.fork] in order
 /// to create a valid [Zone] object.
+///
+/// The function must only access zone-related functionality through
+/// [self], [parent] or [zone].
+/// It should not depend on the current zone ([Zone.current]).
 typedef Zone ForkHandler(Zone self, ZoneDelegate parent, Zone zone,
     ZoneSpecification? specification, Map<Object?, Object?>? zoneValues);
 
@@ -915,10 +970,7 @@ class _ZoneDelegate implements ZoneDelegate {
   _ZoneDelegate(this._delegationTarget);
 
   void handleUncaughtError(Zone zone, Object error, StackTrace stackTrace) {
-    var implementation = _delegationTarget._handleUncaughtError;
-    _Zone implZone = implementation.zone;
-    HandleUncaughtErrorHandler handler = implementation.function;
-    return handler(implZone, implZone._parentDelegate, zone, error, stackTrace);
+    _delegationTarget._processUncaughtError(zone, error, stackTrace);
   }
 
   R run<R>(Zone zone, R f()) {
@@ -1040,6 +1092,28 @@ abstract class _Zone implements Zone {
     return identical(this, otherZone) ||
         identical(errorZone, otherZone.errorZone);
   }
+
+  void _processUncaughtError(Zone zone, Object error, StackTrace stackTrace) {
+    var implementation = _handleUncaughtError;
+    _Zone implZone = implementation.zone;
+    if (identical(implZone, _rootZone)) {
+      _rootHandleError(error, stackTrace);
+      return;
+    }
+    HandleUncaughtErrorHandler handler = implementation.function;
+    ZoneDelegate parentDelegate = implZone._parentDelegate;
+    _Zone parentZone = implZone.parent!; // Not null for non-root zones.
+    _Zone currentZone = Zone._current;
+    try {
+      Zone._current = parentZone;
+      handler(implZone, parentDelegate, zone, error, stackTrace);
+      Zone._current = currentZone;
+    } catch (e, s) {
+      Zone._current = currentZone;
+      parentZone._processUncaughtError(
+          implZone, e, identical(error, e) ? stackTrace : s);
+    }
+  }
 }
 
 class _CustomZone extends _Zone {
@@ -1235,11 +1309,7 @@ class _CustomZone extends _Zone {
   // Methods that can be customized by the zone specification.
 
   void handleUncaughtError(Object error, StackTrace stackTrace) {
-    var implementation = this._handleUncaughtError;
-    ZoneDelegate parentDelegate = implementation.zone._parentDelegate;
-    HandleUncaughtErrorHandler handler = implementation.function;
-    return handler(
-        implementation.zone, parentDelegate, this, error, stackTrace);
+    _processUncaughtError(this, error, stackTrace);
   }
 
   Zone fork(
@@ -1335,6 +1405,10 @@ class _CustomZone extends _Zone {
 
 void _rootHandleUncaughtError(Zone? self, ZoneDelegate? parent, Zone zone,
     Object error, StackTrace stackTrace) {
+  _rootHandleError(error, stackTrace);
+}
+
+void _rootHandleError(Object error, StackTrace stackTrace) {
   _schedulePriorityAsyncCallback(() {
     _rethrow(error, stackTrace);
   });

tests/lib/async/uncaught_error_handler_throws_test.dart

@@ -0,0 +1,76 @@
+// Copyright (c) 2021, 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:expect/expect.dart';
+import 'package:async_helper/async_helper.dart';
+import 'dart:async';
+
+void main() async {
+  asyncStart();
+  await testThrowSame();
+  await testThrowOther();
+  asyncEnd();
+}
+
+Future<void> testThrowSame() async {
+  asyncStart();
+  var object1 = Object();
+  var stack1 = StackTrace.current;
+  var outerZone = Zone.current;
+  var firstZone = Zone.current.fork(specification: onError((error, stack) {
+    // Uncaught error handlers run in the parent zone.
+    Expect.identical(outerZone, Zone.current);
+    Expect.identical(object1, error);
+    Expect.identical(stack1, stack); // Get same stack trace.
+    asyncEnd();
+  }));
+  firstZone.run(() async {
+    Expect.identical(firstZone, Zone.current);
+    var secondZone = Zone.current.fork(specification: onError((error, stack) {
+      // Uncaught error handlers run in the parent zone.
+      Expect.identical(firstZone, Zone.current);
+      Expect.identical(object1, error);
+      Expect.identical(stack1, stack);
+      throw error; // Throw same object
+    }));
+    secondZone.run(() async {
+      Expect.identical(secondZone, Zone.current);
+      Future.error(object1, stack1); // Unhandled async error.
+      await Future(() {});
+    });
+  });
+}
+
+Future<void> testThrowOther() async {
+  asyncStart();
+  var object1 = Object();
+  var object2 = Object();
+  var stack1 = StackTrace.current;
+  var outerZone = Zone.current;
+  var firstZone = Zone.current.fork(specification: onError((error, stack) {
+    Expect.identical(outerZone, Zone.current);
+    Expect.identical(object2, error);
+    Expect.notIdentical(stack1, stack); // Get different stack trace.
+    asyncEnd();
+  }));
+  firstZone.run(() async {
+    Expect.identical(firstZone, Zone.current);
+    var secondZone = Zone.current.fork(specification: onError((error, stack) {
+      Expect.identical(firstZone, Zone.current);
+      Expect.identical(object1, error);
+      Expect.identical(stack1, stack);
+      throw object2; // Throw different object
+    }));
+    secondZone.run(() async {
+      Expect.identical(secondZone, Zone.current);
+      Future.error(object1, stack1); // Unhandled async error.
+      await Future(() {});
+    });
+  });
+}
+
+ZoneSpecification onError(void Function(Object, StackTrace) handler) {
+  return ZoneSpecification(
+      handleUncaughtError: (s, p, z, e, st) => handler(e, st));
+}

tests/lib_2/async/slow_consumer2_test.dart

@@ -88,7 +88,7 @@ class DataProvider {
       listSize -= sentCount - targetCount;
       sentCount = targetCount;
     }
-    controller.add(new List(listSize));
+    controller.add(new List<int>(listSize));
     int ms = listSize * 1000 ~/ bytesPerSecond;
     Duration duration = new Duration(milliseconds: ms);
     if (!controller.isPaused) new Timer(duration, send);

tests/lib_2/async/uncaught_error_handler_throws_test.dart

@@ -0,0 +1,76 @@
+// Copyright (c) 2021, 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:expect/expect.dart';
+import 'package:async_helper/async_helper.dart';
+import 'dart:async';
+
+void main() async {
+  asyncStart();
+  await testThrowSame();
+  await testThrowOther();
+  asyncEnd();
+}
+
+Future<void> testThrowSame() async {
+  asyncStart();
+  var object1 = Object();
+  var stack1 = StackTrace.current;
+  var outerZone = Zone.current;
+  var firstZone = Zone.current.fork(specification: onError((error, stack) {
+    // Uncaught error handlers run in the parent zone.
+    Expect.identical(outerZone, Zone.current);
+    Expect.identical(object1, error);
+    Expect.identical(stack1, stack); // Get same stack trace.
+    asyncEnd();
+  }));
+  firstZone.run(() async {
+    Expect.identical(firstZone, Zone.current);
+    var secondZone = Zone.current.fork(specification: onError((error, stack) {
+      // Uncaught error handlers run in the parent zone.
+      Expect.identical(firstZone, Zone.current);
+      Expect.identical(object1, error);
+      Expect.identical(stack1, stack);
+      throw error; // Throw same object
+    }));
+    secondZone.run(() async {
+      Expect.identical(secondZone, Zone.current);
+      Future.error(object1, stack1); // Unhandled async error.
+      await Future(() {});
+    });
+  });
+}
+
+Future<void> testThrowOther() async {
+  asyncStart();
+  var object1 = Object();
+  var object2 = Object();
+  var stack1 = StackTrace.current;
+  var outerZone = Zone.current;
+  var firstZone = Zone.current.fork(specification: onError((error, stack) {
+    Expect.identical(outerZone, Zone.current);
+    Expect.identical(object2, error);
+    Expect.notIdentical(stack1, stack); // Get different stack trace.
+    asyncEnd();
+  }));
+  firstZone.run(() async {
+    Expect.identical(firstZone, Zone.current);
+    var secondZone = Zone.current.fork(specification: onError((error, stack) {
+      Expect.identical(firstZone, Zone.current);
+      Expect.identical(object1, error);
+      Expect.identical(stack1, stack);
+      throw object2; // Throw different object
+    }));
+    secondZone.run(() async {
+      Expect.identical(secondZone, Zone.current);
+      Future.error(object1, stack1); // Unhandled async error.
+      await Future(() {});
+    });
+  });
+}
+
+ZoneSpecification onError(void Function(Object, StackTrace) handler) {
+  return ZoneSpecification(
+      handleUncaughtError: (s, p, z, e, st) => handler(e, st));
+}