Make all platforms return null from Error.stackTrace until thrown.

The web platforms used to invent a spurious stack trace when reading
`Error.stackTrace` before the object was thrown.
They now return `null` instead, if there is no underlying JS error object,
matching specified behavior.

Fixed bugs in async error throwing in dart2wasm:
* `throw` in an async function did not set the stack trace on an error.
  Now calls `Error._throw` instead of just a direct Wasm "throw".
* `async*` functions did not capture the stack trace of a throw
  that ended the function body, which means it called
  `StreamController.addError` with only one argument.
  That then resused the stack trace from an `Error` throw instead
  of the correct stack trace.

Added tests.

Change-Id: I1d9fa8d9e18076a7fe28254b60b950866cd550a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354021
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>

pkg/dart2wasm/lib/async.dart

@@ -1307,7 +1307,7 @@ class AsyncCodeGenerator extends CodeGenerator {
     // try-finally. Would that be more efficient?
     b.local_get(exceptionLocal);
     b.local_get(stackTraceLocal);
-    b.throw_(translator.exceptionTag);
+    call(translator.errorThrow.reference);
 
     b.unreachable();
     return expectedType;

pkg/dart2wasm/lib/kernel_nodes.dart

@@ -244,6 +244,8 @@ mixin KernelNodes {
   late final Class errorClass = index.getClass("dart:core", "Error");
   late final Field errorClassStackTraceField =
       index.getField("dart:core", "Error", "_stackTrace");
+  late final Procedure errorThrow =
+      index.getProcedure("dart:core", "Error", "_throw");
   late final Procedure errorThrowWithCurrentStackTrace =
       index.getProcedure("dart:core", "Error", "_throwWithCurrentStackTrace");
 

pkg/dart2wasm/lib/transformers.dart

@@ -427,6 +427,9 @@ class _WasmTransformer extends Transformer {
     // Try-catch-finally around the body to call `controller.addError` and
     // `controller.close`.
     final exceptionVar = VariableDeclaration(null, isSynthesized: true);
+    final stackTraceVar = VariableDeclaration(null,
+        isSynthesized: true,
+        type: coreTypes.stackTraceRawType(Nullability.nonNullable));
     final Procedure controllerAddErrorProc = coreTypes.index
         .getProcedure('dart:async', 'StreamController', 'addError');
     final FunctionType controllerAddErrorType =
@@ -442,11 +445,12 @@ class _WasmTransformer extends Transformer {
       [
         Catch(
           exceptionVar,
+          stackTrace: stackTraceVar,
           ExpressionStatement(InstanceInvocation(
             InstanceAccessKind.Instance,
             VariableGet(controller),
             Name('addError'),
-            Arguments([VariableGet(exceptionVar)]),
+            Arguments([VariableGet(exceptionVar), VariableGet(stackTraceVar)]),
             interfaceTarget: controllerAddErrorProc,
             functionType: controllerAddErrorType,
           )),

pkg/front_end/testcases/class_modifiers/mixin/mixin_class_core_libraries.dart.strong.transformed.expect

@@ -71,6 +71,7 @@ abstract class _B&Object&Error extends core::Object implements core::Error /*isA
   }
   @#C4
   @#C10
+  @#C12
   external static method /* from org-dartlang-sdk:///sdk/lib/_internal/vm/lib/errors_patch.dart */ _throw(core::Object error, core::StackTrace stackTrace) → Never;
 }
 class B extends self::_B&Object&Error {
@@ -90,4 +91,6 @@ constants  {
   #C8 = "vm:external-name"
   #C9 = "Error_throwWithStackTrace"
   #C10 = core::pragma {name:#C8, options:#C9}
+  #C11 = "wasm:entry-point"
+  #C12 = core::pragma {name:#C11, options:#C2}
 }

pkg/front_end/testcases/class_modifiers/mixin/mixin_class_core_libraries.dart.weak.transformed.expect

@@ -71,6 +71,7 @@ abstract class _B&Object&Error extends core::Object implements core::Error /*isA
   }
   @#C4
   @#C10
+  @#C12
   external static method /* from org-dartlang-sdk:///sdk/lib/_internal/vm/lib/errors_patch.dart */ _throw(core::Object error, core::StackTrace stackTrace) → Never;
 }
 class B extends self::_B&Object&Error {
@@ -90,4 +91,6 @@ constants  {
   #C8 = "vm:external-name"
   #C9 = "Error_throwWithStackTrace"
   #C10 = core::pragma {name:#C8, options:#C9}
+  #C11 = "wasm:entry-point"
+  #C12 = core::pragma {name:#C11, options:#C2}
 }

pkg/front_end/testcases/class_modifiers/mixin/mixin_class_core_libraries_legacy.dart.strong.transformed.expect

@@ -63,6 +63,7 @@ abstract class _B&Object&Error extends core::Object implements core::Error /*isA
   }
   @#C4
   @#C10
+  @#C12
   external static method /* from org-dartlang-sdk:///sdk/lib/_internal/vm/lib/errors_patch.dart */ _throw(core::Object error, core::StackTrace stackTrace) → Never;
 }
 class B extends self::_B&Object&Error {
@@ -82,4 +83,6 @@ constants  {
   #C8 = "vm:external-name"
   #C9 = "Error_throwWithStackTrace"
   #C10 = core::pragma {name:#C8, options:#C9}
+  #C11 = "wasm:entry-point"
+  #C12 = core::pragma {name:#C11, options:#C2}
 }

pkg/front_end/testcases/class_modifiers/mixin/mixin_class_core_libraries_legacy.dart.weak.transformed.expect

@@ -63,6 +63,7 @@ abstract class _B&Object&Error extends core::Object implements core::Error /*isA
   }
   @#C4
   @#C10
+  @#C12
   external static method /* from org-dartlang-sdk:///sdk/lib/_internal/vm/lib/errors_patch.dart */ _throw(core::Object error, core::StackTrace stackTrace) → Never;
 }
 class B extends self::_B&Object&Error {
@@ -82,4 +83,6 @@ constants  {
   #C8 = "vm:external-name"
   #C9 = "Error_throwWithStackTrace"
   #C10 = core::pragma {name:#C8, options:#C9}
+  #C11 = "wasm:entry-point"
+  #C12 = core::pragma {name:#C11, options:#C2}
 }

pkg/front_end/testcases/dart2wasm/yield.dart.strong.transformed.expect

@@ -81,7 +81,7 @@ static method asyncMethod(asy::Stream<core::int> stream) → asy::Stream<core::i
           }
         }
         on dynamic catch(dynamic #t7, core::StackTrace #t8) {
-          #controller.{asy::StreamController::addError}(#t7){(core::Object, [core::StackTrace?]) → void};
+          #controller.{asy::StreamController::addError}(#t7, #t8){(core::Object, [core::StackTrace?]) → void};
           #t7;
           #t8;
         }

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart

@@ -219,8 +219,10 @@ StackTrace stackTrace(Object? error) {
   return JS('', '#[#] = #', error, _stackTrace, _StackTrace(error));
 }
 
-StackTrace stackTraceForError(Error error) {
-  return stackTrace(JS('', '#[#]', error, _jsError));
+StackTrace? stackTraceForError(Error error) {
+  var jsError = JS('', '#[#]', error, _jsError);
+  if (jsError == null) return null;
+  return stackTrace(jsError);
 }
 
 /// Implements `rethrow` of [error], allowing rethrow in an expression context.

sdk/lib/_internal/js_runtime/lib/js_helper.dart

@@ -1081,8 +1081,10 @@ class Primitives {
     }
   }
 
-  static StackTrace extractStackTrace(Error error) {
-    return getTraceFromException(JS('', r'#.$thrownJsError', error));
+  static StackTrace? extractStackTrace(Error error) {
+    var jsError = JS('', r'#.$thrownJsError', error);
+    if (jsError == null) return null;
+    return getTraceFromException(jsError);
   }
 }
 

sdk/lib/core/errors.dart

@@ -120,6 +120,7 @@ class Error {
     _throw(error, stackTrace);
   }
 
+  @pragma("wasm:entry-point")
   external static Never _throw(Object error, StackTrace stackTrace);
 }
 

tests/corelib/error_stacktrace_test.dart

@@ -0,0 +1,265 @@
+// Copyright (c) 2024, 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.
+
+// Tests that the `Error.stackTrace` is set when thrown, not before,
+// and that it contains the same stack trace text as the stack trace
+// captured by `catch` the first time the error is thrown,
+// and that it doesn't change if thrown again.
+// (Derived from `runtime/tests/vm/dart/error_stacktrace_test.dart`.)
+
+import "package:expect/expect.dart";
+import "package:async_helper/async_helper.dart";
+
+void main() async {
+  testSync();
+  testSyncStar(); // A `throw` in a `sync*` function.
+
+  asyncStart();
+  await testAsync();
+  await testAsyncStar();
+  asyncEnd();
+}
+
+void testSync() {
+  Never throwError(Error error) => throw error;
+  Never throwWithStack(Error error, StackTrace stack) =>
+      Error.throwWithStackTrace(error, stack);
+  dynamic throwNSM(dynamic c) => c * 4;
+
+  _testSync("sync", throwError, throwWithStack, throwNSM);
+}
+
+void testSyncStar() {
+  Iterable<Never> throwErrorStream(Error error) sync* {
+    yield throw error;
+  }
+
+  Iterable<Never> throwWithStackStream(Error error, StackTrace stack) sync* {
+    yield Error.throwWithStackTrace(error, stack);
+  }
+
+  Iterable<dynamic> throwNSMStream(dynamic c) sync* {
+    yield c * 4;
+  }
+
+  Never throwError(Error error) => throwErrorStream(error).first;
+  Never throwWithStack(Error error, StackTrace stack) =>
+      throwWithStackStream(error, stack).first;
+  dynamic throwNSM(dynamic c) => throwNSMStream(c).first;
+
+  _testSync("sync*", throwError, throwWithStack, throwNSM);
+}
+
+Future<void> testAsync() async {
+  Future<Never> throwError(Error error) async => throw error;
+  Future<Never> throwErrorWithStack(Error error, StackTrace stack) async =>
+      Error.throwWithStackTrace(error, stack);
+  Future<dynamic> throwNSM(dynamic c) async => c * 4;
+
+  return _testAsync("async", throwError, throwErrorWithStack, throwNSM);
+}
+
+Future<void> testAsyncStar() async {
+  Stream<Never> throwErrorStream(Error error) async* {
+    yield throw error;
+  }
+
+  Future<Never> throwError(Error error) => throwErrorStream(error).first;
+
+  Stream<Never> throwErrorWithStackStream(
+      Error error, StackTrace stack) async* {
+    yield Error.throwWithStackTrace(error, stack);
+  }
+
+  Future<Never> throwErrorWithStack(Error error, StackTrace stack) =>
+      throwErrorWithStackStream(error, stack).first;
+
+  Stream<dynamic> throwNSMStream(dynamic c) async* {
+    yield c * 4;
+  }
+
+  Future<dynamic> throwNSM(dynamic c) => throwNSMStream(c).first;
+
+  return _testAsync("async*", throwError, throwErrorWithStack, throwNSM);
+}
+
+void _testSync(
+    String functionKind,
+    Never Function(Error) throwError,
+    Never Function(Error, StackTrace) throwWithStack,
+    dynamic Function(dynamic) throwNSM) {
+  // Checks that an error first thrown with [firstStack] as [Error.stackTrace],
+  // will keep that stack trace if thrown again asynchronously.
+  void testErrorSet(String throwKind, Error error, StackTrace firstStack) {
+    var desc = "$functionKind $throwKind";
+    // Was thrown with [stackTrace] as stack trace.
+    Expect.isNotNull(error.stackTrace, "$desc throw - did not set .stackTrace");
+    Expect.stringEquals(firstStack.toString(), error.stackTrace.toString(),
+        "$desc, caught stack/set stack - not same");
+    // Throw same error again, using `throw`, with different stack.
+    try {
+      throwError(error);
+    } on Error catch (e, s) {
+      var redesc = "$functionKind throw again";
+      Expect.identical(error, e, "$redesc - not same error");
+      Expect.notEquals(firstStack.toString(), s.toString(),
+          "$redesc, set stack/new stack - not different");
+      // Did not overwrite existing `error.stackTrace`.
+      Expect.equals(firstStack.toString(), e.stackTrace.toString(),
+          "$redesc - changed .stackTrace");
+    }
+
+    // Throw same error again using `Error.throwWithStackTrace`.
+    var stack2 = StackTrace.fromString("stack test string 2");
+    try {
+      throwWithStack(error, stack2);
+    } on Error catch (e, s) {
+      var redesc = "$functionKind E.tWST again";
+      Expect.identical(error, e, "$redesc - not same error");
+      Expect.equals(stack2.toString(), s.toString(),
+          "$redesc, thrown stack/caught stack - not same");
+      Expect.notEquals(firstStack.toString(), s.toString(),
+          "$redesc, first stack/new stack - not different");
+      // Did not overwrite existing `error.stackTrace`.
+      Expect.equals(firstStack.toString(), e.stackTrace.toString(),
+          "$redesc - changed .stackTrace");
+    }
+  }
+
+  {
+    // System thrown error.
+    try {
+      throwNSM(NoMult());
+      Expect.fail("Did not throw");
+    } on NoSuchMethodError catch (e, s) {
+      testErrorSet("throwNSM", e, s);
+    }
+  }
+
+  {
+    // User thrown error, explicit `throw`.
+    var error = StateError("error test string");
+    Expect.isNull(error.stackTrace);
+    try {
+      throwError(error);
+    } on Error catch (e, s) {
+      Expect.identical(error, e,
+          "$functionKind throw: thrown error/caught error - not same");
+      testErrorSet("throw", e, s);
+    }
+  }
+
+  {
+    // Thrown using `Error.throwWithStackTrace`.
+    var error = StateError("error test string");
+    Expect.isNull(error.stackTrace);
+    var stack = StackTrace.fromString("stack test string");
+    try {
+      throwWithStack(error, stack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e,
+          "$functionKind E.tWST: thrown error/caught error - not same");
+      Expect.stringEquals(stack.toString(), s.toString(),
+          "$functionKind E.tWST: thrown stack/caught stack - not same");
+      testErrorSet("E.tWST", e, s);
+    }
+  }
+}
+
+Future<void> _testAsync(
+    String functionKind,
+    Future<Never> Function(Error) throwError,
+    Future<Never> Function(Error, StackTrace) throwWithStack,
+    Future<dynamic> Function(dynamic) throwNSM) async {
+  // Checks that an error first thrown with [firstStack] as [Error.stackTrace],
+  // will keep that stack trace if thrown again asynchronously.
+  Future<void> testErrorSet(
+      String throwKind, Error error, StackTrace firstStack) async {
+    var desc = "$functionKind $throwKind";
+    // Was thrown with [stackTrace] as stack trace.
+    Expect.isNotNull(error.stackTrace, "$desc throw - did not set .stackTrace");
+    Expect.stringEquals(firstStack.toString(), error.stackTrace.toString(),
+        "$desc, caught stack/set stack - not same");
+    // Throw same error again, using `throw`, with different stack.
+    try {
+      await throwError(error);
+    } on Error catch (e, s) {
+      var redesc = "$functionKind throw again";
+      Expect.identical(error, e, "$redesc - not same error");
+      if (functionKind != "async*") {
+        // An async* throw happens asynchronously, so its stack trace
+        // can be just the same short stack from the event loop every time.
+        Expect.notEquals(firstStack.toString(), s.toString(),
+            "$redesc, set stack/new stack - not different");
+      }
+      // Did not overwrite existing `error.stackTrace`.
+      Expect.equals(firstStack.toString(), e.stackTrace.toString(),
+          "$redesc - changed .stackTrace");
+    }
+
+    // Throw same error again using `Error.throwWithStackTrace`.
+    var stack2 = StackTrace.fromString("stack test string 2");
+    try {
+      await throwWithStack(error, stack2);
+    } on Error catch (e, s) {
+      var redesc = "$functionKind E.tWST again";
+      Expect.identical(error, e, "$redesc - not same error");
+      Expect.equals(stack2.toString(), s.toString(),
+          "$redesc, thrown stack/caught stack - not same");
+      if (functionKind != "async*") {
+        Expect.notEquals(firstStack.toString(), s.toString(),
+            "$redesc, first stack/new stack - not different");
+      }
+      // Did not overwrite existing `error.stackTrace`.
+      Expect.equals(firstStack.toString(), e.stackTrace.toString(),
+          "$redesc - changed .stackTrace");
+    }
+  }
+
+  asyncStart();
+
+  {
+    // System thrown error.
+    try {
+      await throwNSM(NoMult());
+      Expect.fail("Did not throw");
+    } on NoSuchMethodError catch (e, s) {
+      await testErrorSet("throwNSM", e, s);
+    }
+  }
+
+  {
+    // User thrown error, explicit `throw`.
+    var error = StateError("error test string");
+    Expect.isNull(error.stackTrace);
+    try {
+      await throwError(error);
+    } on Error catch (e, s) {
+      Expect.identical(error, e,
+          "$functionKind throw: thrown error/caught error - not same");
+      await testErrorSet("throw", e, s);
+    }
+  }
+
+  {
+    // Thrown using `Error.throwWithStackTrace`.
+    var error = StateError("error test string");
+    Expect.isNull(error.stackTrace);
+    var stack = StackTrace.fromString("stack test string");
+    try {
+      await throwWithStack(error, stack);
+    } on Error catch (e, s) {
+      Expect.identical(error, e,
+          "$functionKind E.tWST: thrown error/caught error - not same");
+      Expect.stringEquals(stack.toString(), s.toString(),
+          "$functionKind E.tWST: thrown stack/caught stack - not same");
+      await testErrorSet("E.tWST", e, s);
+    }
+  }
+  asyncEnd();
+}
+
+// Has no `operator *`, forcing a dynamic `NoSuchMethodError`
+// when used in `c * 4`.
+class NoMult {}