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

[eyebrowsoffire]

If the catch guard in a try/catch block is general enough to cover JS exceptions, the lowering to wasm needs to include a catch_all block, as that's the only way to catch JS exceptions (they have no exception tag). This works in synchronous functions. For example, in the following code:

import 'dart:js_interop';

@JS('window.navigator.userAgent')
external JSString get userAgent;

void main() {
    String? userAgentString = null;
    try {
        userAgentString = userAgent.toDart;
    } catch (e) {
    }
    print('userAgent: $userAgentString');
}

The try/catch block lowers to:

try $label0
  call $userAgent
  call $JSStringToString|get#toDart
  local.set $var0
  br $label0
catch $tag0
  local.set $var2
  local.set $var1
  block
    local.get $var1
    local.set $var3
    local.get $var2
    local.set $var4
    local.get $var3
    drop
    local.get $var4
    drop
    br $label0
  end
  rethrow $label0
catch_all
  call $StackTrace.current
  local.set $var2
  call $new _JavaScriptError._
  local.set $var1
  block
    local.get $var1
    local.set $var5
    local.get $var2
    local.set $var6
    local.get $var5
    drop
    local.get $var6
    drop
    br $label0
  end
  rethrow $label0
end $label0

However, if this is in an async context instead, the catch_all block is not actually emitted. For example, in the same code, but in with an async main function:

import 'dart:js_interop';

@JS('window.navigator.userAgent')
external JSString get userAgent;

Future<void> main() async {
    String? userAgentString = null;
    try {
        userAgentString = userAgent.toDart;
    } catch (e) {
    }
    print('userAgent: $userAgentString');
}

The try/catch block lowers to:

try
  local.get $var6
  call $userAgent
  call $JSStringToString|get#toDart
  struct.set $<context file:///Users/jacksongardner/Desktop/scratch/assert_text.dart:6:18> $field0
  br $label2
catch $tag0
  local.set $var7
  local.set $var8
  local.get $var0
  local.get $var8
  struct.set $_AsyncSuspendState $field6
  local.get $var0
  local.get $var7
  struct.set $_AsyncSuspendState $field7
  br $label1
end

Notice the missing catch_all block. As a result, JS exceptions escape this try/catch and cause failures.

The specific customer who this is affecting is the flutter firebase packages, which have this try/catch here: https://github.com/firebase/flutterfire/blob/213608cf7d889aeb60084ef5fb74a1e74178d7ba/packages/firebase_core/firebase_core_web/lib/src/firebase_core_web.dart#L254 which normally allows them to continue and properly initialize the firebase app.

[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.