[dart2wasm] Try/catch in an async function doesn't emit `catch_all` for JS exceptions about sdk

[eyebrowsoffire]

I think probably the issue is that the current mechanisms for async try/catch of JS exceptions actually do work if the exception is thrown asynchronously, because it looks like in the async context we do catch JS exceptions and probably convert them to a dart exception:

sdk/pkg/dart2wasm/lib/async.dart

Line 793 in 8af6f94

b.catch_all();

So the try/catch lowering assumes that it has already been converted to a dart exception with the $tag0 tag. However, this doesn't actually work when the exception is thrown synchronously, because the try/catch lowering is responsible for directly catching the thrown exception (rather than something transformed by the async error handling context)

[kevmoo]

Related? #55466

[osa1]

We need the try_catch blocks we have in the sync code generator in try-finally and try-catch compilers in async code generator as well.

Test file:

import 'dart:js_interop';

@JS()
external void eval(String code);

@JS()
external void throwFromJS();

void defineFunctions() {
  eval(r'''
    globalThis.throwFromJS = function() {
      throw 'exception from JS';
    };
  ''');
}

Future<void> catchInAsync() async {
  try {
    throwFromJS();
  } catch (e) {
    print("Error caugh in async function");
  } finally {
    print("Finally block in async function");
  }
}

void catchInSync() {
  try {
    throwFromJS();
  } catch (e) {
    print("Error caugh in sync function");
  } finally {
    print("Finally block in sync function");
  }
}

Future<void> main() async {
  await catchInAsync();
  catchInSync();
}

catchInSync works as expected, but catchInAsync crashes as the JS exception isn't caught.

I think probably the issue is that the current mechanisms for async try/catch of JS exceptions actually do work if the exception is thrown asynchronously

This is right, if the JS exception is caught in an await it's handled. Example:

import 'dart:js_interop';

@JS()
external void eval(String code);

@JS()
external void throwFromJS();

void defineFunctions() {
  eval(r'''
    globalThis.throwFromJS = function() {
      throw 'exception from JS';
    };
  ''');
}

Future<void> catchInAsync() async {
  try {
    throwFromJS();
  } catch (e) {
    print("Error caugh in async function");
  } finally {
    print("Finally block in async function");
  }
}

Future<void> main() async {
  try {
    await catchInAsync();
  } catch (e) {
    print("Exception caught in main");
  }
}

Here main catches the JS exception, but catchInAsync doesn't (because of this bug).

The fix should just be a matter of adding catch_all blocks in try-finally and try-catch blocks.

We can actually avoid catch_all blocks when a call is known to not make JS calls because the only reason why we have catch_all is to catch JS exceptions. I think we probably doesn't have this information today. I'll file an issue for this for now. (filed #55470)

[eyebrowsoffire]

One other possibility that has occurred to me is to actually wrap every JS interop entry point with a try/catch in JavaScript that converts any JS exception to a dart exception right at the JS/Wasm boundary. That would theoretically allow us to eliminate the catch_all blocks from all the dart2wasm code completely, since it can assume that any exceptions flowing through the wasm code will already be converted to a dart exception. This also would allow us to get more detailed/complete information about the JS exceptions, since we can actually introspect info about the JS exception (such as stack trace, exception type, message, etc) in a JS catch block that is not accessible with a catch_all instruction in wasm.

On the other hand, exceptions flowing through an export that comes from a @Native annotation might disobey this assumption. Also the performance overhead of wrapping all JS interop calls with a try/catch block is unclear. JIT may be fairly smart at eliminating the overhead for calls that can't actually throw.

[osa1]

Fix in progress in https://dart-review.googlesource.com/c/sdk/+/363000.

[osa1]

This also would allow us to get more detailed/complete information about the JS exceptions, since we can actually introspect info about the JS exception (such as stack trace, exception type, message, etc)

I wonder if we can get this information by using catch_all_ref instead of catch_all, and implementing JS functions to get the stack trace (and other info) from passed exception references.

[eyebrowsoffire]

That is also a possibility, but catch_all_ref is fairly recent and browser support is limited. Whereas catching the JS exception in JS would be supported by all browsers today.