Add Error.throwWithStackTrace function which throws an error and stack.

Allows throwing an error with an existing stack trace, instead of always
creating a new stack traces.

Fixes #30741

Bug: https://github.com/dart-lang/sdk/issues/30741
TEST=corelib/error_throw_with_stacktrace_test.dart
Change-Id: Iea77b4a8eb5e0cab4cd0d75432d033256c9f79fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219522
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>

CHANGELOG.md

@@ -309,6 +309,9 @@ them, you must set the lower bound on the SDK constraint for your package to
   ready to be removed.
   Code catching the class should move to catching `Error` instead
   (or, for integers, check first for whether it's dividing by zero).
+- Add `Error.throwWithStackTrace` which can `throw` an
+  error with an existing stack trace, instead of creating
+  a new stack trace.
 
 #### `dart:io`
 

runtime/lib/errors.cc

@@ -220,10 +220,10 @@ DEFINE_NATIVE_ENTRY(AbstractClassInstantiationError_throwNew, 0, 2) {
 }
 
 // Rethrow an error with a stacktrace.
-DEFINE_NATIVE_ENTRY(Async_rethrow, 0, 2) {
+DEFINE_NATIVE_ENTRY(Error_throwWithStackTrace, 0, 2) {
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, error, arguments->NativeArgAt(0));
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, stacktrace, arguments->NativeArgAt(1));
-  Exceptions::ReThrow(thread, error, stacktrace);
+  Exceptions::ThrowWithStackTrace(thread, error, stacktrace);
   return Object::null();
 }
 

runtime/vm/bootstrap_natives.h

@@ -160,7 +160,7 @@ namespace dart {
   V(DateTime_localTimeZoneAdjustmentInSeconds, 0)                              \
   V(AssertionError_throwNew, 3)                                                \
   V(AssertionError_throwNewSource, 4)                                          \
-  V(Async_rethrow, 2)                                                          \
+  V(Error_throwWithStackTrace, 2)                                              \
   V(StackTrace_current, 0)                                                     \
   V(TypeError_throwNew, 4)                                                     \
   V(FallThroughError_throwNew, 1)                                              \

runtime/vm/exceptions.cc

@@ -696,8 +696,9 @@ static void ThrowExceptionHelper(Thread* thread,
   if (exception.IsNull()) {
     exception ^=
         Exceptions::Create(Exceptions::kNullThrown, Object::empty_array());
-  } else if (exception.ptr() == object_store->out_of_memory() ||
-             exception.ptr() == object_store->stack_overflow()) {
+  } else if (existing_stacktrace.IsNull() &&
+             (exception.ptr() == object_store->out_of_memory() ||
+              exception.ptr() == object_store->stack_overflow())) {
     use_preallocated_stacktrace = true;
   }
   // Find the exception handler and determine if the handler needs a
@@ -729,11 +730,17 @@ static void ThrowExceptionHelper(Thread* thread,
     }
   } else {
     if (!existing_stacktrace.IsNull()) {
-      // If we have an existing stack trace then this better be a rethrow. The
-      // reverse is not necessarily true (e.g. Dart_PropagateError can cause
-      // a rethrow being called without an existing stacktrace.)
-      ASSERT(is_rethrow);
       stacktrace = existing_stacktrace.ptr();
+      // If this is not a rethrow, it's a "throw with stacktrace".
+      // Set an Error object's stackTrace field if needed.
+      if (!is_rethrow) {
+        const Field& stacktrace_field =
+            Field::Handle(zone, LookupStackTraceField(exception));
+        if (!stacktrace_field.IsNull() &&
+            (exception.GetField(stacktrace_field) == Object::null())) {
+          exception.SetField(stacktrace_field, stacktrace);
+        }
+      }
     } else {
       // Get stacktrace field of class Error to determine whether we have a
       // subclass of Error which carries around its stack trace.
@@ -917,6 +924,13 @@ void Exceptions::ReThrow(Thread* thread,
   ThrowExceptionHelper(thread, exception, stacktrace, true);
 }
 
+void Exceptions::ThrowWithStackTrace(Thread* thread,
+                                     const Instance& exception,
+                                     const Instance& stacktrace) {
+  // Null object is a valid exception object.
+  ThrowExceptionHelper(thread, exception, stacktrace, false);
+}
+
 void Exceptions::PropagateError(const Error& error) {
   ASSERT(!error.IsNull());
   Thread* thread = Thread::Current();

runtime/vm/exceptions.h

@@ -32,6 +32,9 @@ class Exceptions : AllStatic {
   DART_NORETURN static void ReThrow(Thread* thread,
                                     const Instance& exception,
                                     const Instance& stacktrace);
+  DART_NORETURN static void ThrowWithStackTrace(Thread* thread,
+                                                const Instance& exception,
+                                                const Instance& stacktrace);
   DART_NORETURN static void PropagateError(const Error& error);
 
   // Propagate an error to the entry frame, skipping over Dart frames.

sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart

@@ -195,11 +195,6 @@ class Timer {
   }
 }
 
-@patch
-void _rethrow(Object error, StackTrace stackTrace) {
-  JS('', 'throw #', dart.createErrorWithStack(error, stackTrace));
-}
-
 /// Used by the compiler to implement `async*` functions.
 ///
 /// This is inspired by _AsyncStarStreamController in dart-lang/sdk's

sdk/lib/_internal/js_dev_runtime/patch/core_patch.dart

@@ -282,6 +282,12 @@ class Error {
 
   @patch
   StackTrace? get stackTrace => dart.stackTraceForError(this);
+
+  @patch
+  static Never _throw(Object error, StackTrace stackTrace) {
+    JS("", "throw #", dart.createErrorWithStack(error, stackTrace));
+    throw "unreachable";
+  }
 }
 
 @patch
@@ -592,9 +598,7 @@ class String {
   }
 
   static String _stringFromJSArray(
-      /*=JSArray<int>*/ list,
-      int start,
-      int? endOrNull) {
+      /*=JSArray<int>*/ list, int start, int? endOrNull) {
     int len = list.length;
     int end = RangeError.checkValidRange(start, endOrNull, len);
     if (start > 0 || end < len) {

sdk/lib/_internal/js_runtime/lib/async_patch.dart

@@ -704,10 +704,3 @@ class _SyncStarIterable<T> extends IterableBase<T> {
   Iterator<T> get iterator =>
       new _SyncStarIterator<T>(JS('', '#()', _outerHelper));
 }
-
-@patch
-void _rethrow(Object error, StackTrace stackTrace) {
-  error = wrapException(error);
-  JS('void', '#.stack = #', error, stackTrace.toString());
-  JS('void', 'throw #', error);
-}

sdk/lib/_internal/js_runtime/lib/core_patch.dart

@@ -22,7 +22,8 @@ import 'dart:_js_helper'
         Primitives,
         quoteStringForRegExp,
         getTraceFromException,
-        RuntimeError;
+        RuntimeError,
+        wrapException;
 
 import 'dart:_foreign_helper' show JS;
 import 'dart:_native_typed_data' show NativeUint8List;
@@ -187,6 +188,14 @@ class Error {
 
   @patch
   StackTrace? get stackTrace => Primitives.extractStackTrace(this);
+
+  @patch
+  static Never _throw(Object error, StackTrace stackTrace) {
+    error = wrapException(error);
+    JS('void', '#.stack = #', error, stackTrace.toString());
+    JS('', 'throw #', error);
+    throw "unreachable";
+  }
 }
 
 @patch

sdk/lib/_internal/vm/lib/async_patch.dart

@@ -244,10 +244,6 @@ class _AsyncStarStreamController<T> {
   }
 }
 
-@patch
-@pragma("vm:external-name", "Async_rethrow")
-external void _rethrow(Object error, StackTrace stackTrace);
-
 @patch
 class _StreamImpl<T> {
   /// The closure implementing the async-generator body that is creating events

sdk/lib/_internal/vm/lib/errors_patch.dart

@@ -21,6 +21,10 @@ class Error {
 
   @pragma("vm:entry-point")
   StackTrace? _stackTrace;
+
+  @patch
+  @pragma("vm:external-name", "Error_throwWithStackTrace")
+  external static Never _throw(Object error, StackTrace stackTrace);
 }
 
 class _AssertionError extends Error implements AssertionError {

sdk/lib/async/zone.dart

@@ -1410,12 +1410,10 @@ void _rootHandleUncaughtError(Zone? self, ZoneDelegate? parent, Zone zone,
 
 void _rootHandleError(Object error, StackTrace stackTrace) {
   _schedulePriorityAsyncCallback(() {
-    _rethrow(error, stackTrace);
+    Error.throwWithStackTrace(error, stackTrace);
   });
 }
 
-external void _rethrow(Object error, StackTrace stackTrace);
-
 R _rootRun<R>(Zone? self, ZoneDelegate? parent, Zone zone, R f()) {
   if (identical(Zone._current, zone)) return f();
 

sdk/lib/core/errors.dart

@@ -92,6 +92,23 @@ class Error {
   /// trace filled in the first time they are thrown by a `throw`
   /// expression.
   external StackTrace? get stackTrace;
+
+  /// Throws [error] with associated stack trace [stackTrace].
+  ///
+  /// If [error] extends [Error] and has not yet been thrown,
+  /// its [stackTrace] is set as well, just as if it was thrown by a `throw`.
+  /// The actual stack trace captured along with the [error],
+  /// or set on [error] if it is an [Error],
+  /// may not be the [stackTrace] object itself,
+  /// but will be a stack trace with the same content.
+  @Since("2.15")
+  static Never throwWithStackTrace(Object error, StackTrace stackTrace) {
+    checkNotNullable(error, "error");
+    checkNotNullable(stackTrace, "stackTrace");
+    _throw(error, stackTrace);
+  }
+
+  external static Never _throw(Object error, StackTrace stackTrace);
 }
 
 /// Error thrown by the runtime system when an assert statement fails.

tests/corelib/error_throw_with_stacktrace_test.dart

@@ -0,0 +1,210 @@
+// 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 "dart:async";
+import "package:expect/expect.dart";
+import "package:async_helper/async_helper.dart";
+
+// Test of Error.throwWithStackTrace.
+
+main() {
+  // Ensure `systemStack` is different from any other stack tracek.
+  var systemStack = (() => StackTrace.current)();
+
+  // Test that an error can be thrown with a system stack trace..
+  {
+    var error = ArgumentError("e1");
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+      Expect.fail("Didn't throw: e1.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e1.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e1.3");
+      Expect.isNotNull(error.stackTrace, "e1.4");
+      Expect.equals("$systemStack", "${error.stackTrace}", "e1.5");
+    }
+  }
+
+  // Test that an error can be thrown with a user-created stack trace..
+  {
+    var stringStack = StackTrace.fromString("Nonce");
+    var error = ArgumentError("e2");
+    try {
+      Error.throwWithStackTrace(error, stringStack);
+      Expect.fail("Didn't throw: e2.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e2.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$stringStack", "$s", "e2.3");
+      Expect.isNotNull(error.stackTrace, "e2.4");
+      Expect.equals("$stringStack", "${error.stackTrace}", "e2.5");
+    }
+  }
+
+  // Test that a non-error object can be thrown too.
+  {
+    var exception = FormatException("e3");
+    try {
+      Error.throwWithStackTrace(exception, systemStack);
+      Expect.fail("Didn't throw: e3.1");
+    } on Exception catch (e, s) {
+      Expect.identical(exception, e, "e3.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e3.3");
+    }
+  }
+
+  // Test that an [Error] not extending {Error} can be thrown,
+  // but doesn't (and cannot) set the stack trace.
+  {
+    var error = CustomError("e4");
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+      Expect.fail("Didn't throw: e4.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e4.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e4.3");
+      Expect.isNull(error.stackTrace, "e4.4");
+    }
+  }
+
+  // Test that an already set stack trace isn't changed.
+  {
+    var error = ArgumentError("e5");
+    StackTrace? originalStack;
+    try {
+      throw error;
+    } on Error catch (e) {
+      originalStack = e.stackTrace;
+    }
+    Expect.isNotNull(originalStack);
+    Expect.notIdentical(originalStack, systemStack);
+    Expect.notEquals("$originalStack", "");
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+      Expect.fail("Didn't throw: e5.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e5.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e5.3");
+      // Expect the already-set stack trace to stay.
+      Expect.isNotNull(error.stackTrace, "e5.4");
+      Expect.equals("$originalStack", "${error.stackTrace}", "e5.5");
+    }
+  }
+
+  // Works with OutOfMemoryError.
+  {
+    var error = const OutOfMemoryError();
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e);
+      Expect.equals("$systemStack", "$s");
+    }
+  }
+
+  // Works with StackOverflowError.
+  {
+    var error = const StackOverflowError();
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e);
+      Expect.equals("$systemStack", "$s");
+    }
+  }
+
+  // Also for live, captured, StackOverflowError.
+  {
+    Object error;
+    Never foo() => foo() + 1;
+    try {
+      foo(); // Force stack overflow.
+    } catch (e, s) {
+      error = e;
+    }
+    // Some platforms might use another error than StackOverflowError.
+    // Should work with whichever object gets here.
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e);
+      Expect.equals("$systemStack", "$s");
+    }
+  }
+
+  asyncTest(() async {
+    var theFuture = Future.value(null);
+
+    // Test that throwing inside an asynchronous context can be caught.
+    {
+      var error = ArgumentError("e6");
+      try {
+        await theFuture;
+        Error.throwWithStackTrace(error, systemStack);
+        Expect.fail("Didn't throw: e6.1");
+        await theFuture;
+      } on Error catch (e, s) {
+        Expect.identical(error, e, "e6.2");
+        // No not expect *identical* stack trace objects.
+        Expect.equals("$systemStack", "$s", "e6.3");
+        Expect.isNotNull(error.stackTrace, "e6.4");
+        Expect.equals("$systemStack", "${error.stackTrace}", "e6.5");
+      }
+    }
+
+    // Test that throwing in asynchronous context can be locally uncaught.
+    {
+      asyncStart();
+      var error = ArgumentError("e7");
+      var future = () async {
+        await theFuture;
+        Error.throwWithStackTrace(error, systemStack);
+        Expect.fail("Didn't throw: e7.1");
+        await theFuture;
+        return null; // Force future type to Future<dynamic>
+      }();
+      future.catchError((e, s) {
+        Expect.identical(error, e, "e7.2");
+        // No not expect *identical* stack trace objects.
+        Expect.equals("$systemStack", "$s", "e7.3");
+        Expect.isNotNull(error.stackTrace, "e7.4");
+        Expect.equals("$systemStack", "${error.stackTrace}", "e7.5");
+        asyncEnd();
+      });
+    }
+
+    // Test throwing an uncaught async error caught by the Zone.
+    {
+      asyncStart();
+      var error = ArgumentError("e8");
+      await runZonedGuarded(() {
+        // Make an uncaught asynchronous error.
+        (() async {
+          await theFuture;
+          Error.throwWithStackTrace(error, systemStack);
+          Expect.fail("Didn't throw: e8.1");
+          await theFuture;
+        }());
+      }, (e, s) {
+        Expect.identical(error, e, "e8.2");
+        // No not expect *identical* stack trace objects.
+        Expect.equals("$systemStack", "$s", "e8.3");
+        Expect.isNotNull(error.stackTrace, "e8.4");
+        Expect.equals("$systemStack", "${error.stackTrace}", "e8.5");
+        asyncEnd();
+      });
+    }
+  });
+}
+
+class CustomError implements Error {
+  final String message;
+  CustomError(this.message);
+  StackTrace? get stackTrace => null;
+  String toString() => "CustomError: $message";
+}

tests/corelib_2/error_throw_with_stacktrace_test.dart

@@ -0,0 +1,211 @@
+// 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.
+
+// @dart = 2.9
+
+import "dart:async";
+import "package:expect/expect.dart";
+import "package:async_helper/async_helper.dart";
+
+// Test of Error.throwWithStackTrace.
+main() {
+  // Ensure `systemStack` is different from any other stack tracek.
+  var systemStack = (() => StackTrace.current)();
+
+  // Test that an error can be thrown with a system stack trace..
+  {
+    var error = ArgumentError("e1");
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+      Expect.fail("Didn't throw: e1.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e1.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e1.3");
+      Expect.isNotNull(error.stackTrace, "e1.4");
+      Expect.equals("$systemStack", "${error.stackTrace}", "e1.5");
+    }
+  }
+
+  // Test that an error can be thrown with a user-created stack trace..
+  {
+    var stringStack = StackTrace.fromString("Nonce");
+    var error = ArgumentError("e2");
+    try {
+      Error.throwWithStackTrace(error, stringStack);
+      Expect.fail("Didn't throw: e2.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e2.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$stringStack", "$s", "e2.3");
+      Expect.isNotNull(error.stackTrace, "e2.4");
+      Expect.equals("$stringStack", "${error.stackTrace}", "e2.5");
+    }
+  }
+
+  // Test that a non-error object can be thrown too.
+  {
+    var exception = FormatException("e3");
+    try {
+      Error.throwWithStackTrace(exception, systemStack);
+      Expect.fail("Didn't throw: e3.1");
+    } on Exception catch (e, s) {
+      Expect.identical(exception, e, "e3.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e3.3");
+    }
+  }
+
+  // Test that an [Error] not extending {Error} can be thrown,
+  // but doesn't (and cannot) set the stack trace.
+  {
+    var error = CustomError("e4");
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+      Expect.fail("Didn't throw: e4.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e4.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e4.3");
+      Expect.isNull(error.stackTrace, "e4.4");
+    }
+  }
+
+  // Test that an already set stack trace isn't changed.
+  {
+    var error = ArgumentError("e5");
+    StackTrace originalStack;
+    try {
+      throw error;
+    } on Error catch (e) {
+      originalStack = e.stackTrace;
+    }
+    Expect.isNotNull(originalStack);
+    Expect.notIdentical(originalStack, systemStack);
+    Expect.notEquals("$originalStack", "");
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+      Expect.fail("Didn't throw: e5.1");
+    } on Error catch (e, s) {
+      Expect.identical(error, e, "e5.2");
+      // No not expect *identical* stack trace objects.
+      Expect.equals("$systemStack", "$s", "e5.3");
+      // Expect the already-set stack trace to stay.
+      Expect.isNotNull(error.stackTrace, "e5.4");
+      Expect.equals("$originalStack", "${error.stackTrace}", "e5.5");
+    }
+  }
+
+  // Works with OutOfMemoryError.
+  {
+    var error = const OutOfMemoryError();
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e);
+      Expect.equals("$systemStack", "$s");
+    }
+  }
+
+  // Works with StackOverflowError.
+  {
+    var error = const StackOverflowError();
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e);
+      Expect.equals("$systemStack", "$s");
+    }
+  }
+
+  // Also for live, captured, StackOverflowError.
+  {
+    Object error;
+    int foo() => foo() + 1;
+    try {
+      foo(); // Force stack overflow.
+    } catch (e, s) {
+      error = e;
+    }
+    // Some platforms might use another error than StackOverflowError.
+    // Should work with whichever object gets here.
+    try {
+      Error.throwWithStackTrace(error, systemStack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e);
+      Expect.equals("$systemStack", "$s");
+    }
+  }
+
+  asyncTest(() async {
+    var theFuture = Future.value(null);
+
+    // Test that throwing inside an asynchronous context can be caught.
+    {
+      var error = ArgumentError("e6");
+      try {
+        await theFuture;
+        Error.throwWithStackTrace(error, systemStack);
+        Expect.fail("Didn't throw: e6.1");
+        await theFuture;
+      } on Error catch (e, s) {
+        Expect.identical(error, e, "e6.2");
+        // No not expect *identical* stack trace objects.
+        Expect.equals("$systemStack", "$s", "e6.3");
+        Expect.isNotNull(error.stackTrace, "e6.4");
+        Expect.equals("$systemStack", "${error.stackTrace}", "e6.5");
+      }
+    }
+
+    // Test that throwing in asynchronous context can be locally uncaught.
+    {
+      asyncStart();
+      var error = ArgumentError("e7");
+      var future = () async {
+        await theFuture;
+        Error.throwWithStackTrace(error, systemStack);
+        Expect.fail("Didn't throw: e7.1");
+        await theFuture;
+        return null; // Force future type to Future<dynamic>
+      }();
+      future.catchError((e, s) {
+        Expect.identical(error, e, "e7.2");
+        // No not expect *identical* stack trace objects.
+        Expect.equals("$systemStack", "$s", "e7.3");
+        Expect.isNotNull(error.stackTrace, "e7.4");
+        Expect.equals("$systemStack", "${error.stackTrace}", "e7.5");
+        asyncEnd();
+      });
+    }
+
+    // Test throwing an uncaught async error caught by the Zone.
+    {
+      asyncStart();
+      var error = ArgumentError("e8");
+      await runZonedGuarded(() {
+        // Make an uncaught asynchronous error.
+        (() async {
+          await theFuture;
+          Error.throwWithStackTrace(error, systemStack);
+          Expect.fail("Didn't throw: e8.1");
+          await theFuture;
+        }());
+      }, (e, s) {
+        Expect.identical(error, e, "e8.2");
+        // No not expect *identical* stack trace objects.
+        Expect.equals("$systemStack", "$s", "e8.3");
+        Expect.isNotNull(error.stackTrace, "e8.4");
+        Expect.equals("$systemStack", "${error.stackTrace}", "e8.5");
+        asyncEnd();
+      });
+    }
+  });
+}
+
+class CustomError implements Error {
+  final String message;
+  CustomError(this.message);
+  StackTrace get stackTrace => null;
+  String toString() => "CustomError: $message";
+}