Dart2js throw/catch bug

[nex3]

The following code handles nested throws and catches incorrectly in dart2js:

Future<void> main() async {
  try {
    try {
      await Future(() => throw FormatException("oh no"));
    } catch (error) {
      print("catch 1: $error");
      try {
        throw "aw beans";
      } on String catch (error) {
        print("catch 2: $error");
      }
    }
  } catch (error) {
    print("catch 3: $error");
  }
}

The Dart VM correctly prints:

catch 1: FormatException: oh no
catch 2: aw beans

When compiled with dart2js and run in Node.js or a browser, this incorrectly prints:

catch 1: FormatException: oh no
catch 3: FormatException: oh no

[biggs0125]

Thanks for filing this and attaching a simple reproduction!

The issue here is a scoping issue in the way we're handling a non-async catch block within an async catch block. More specifically it's causing references to the inner catch variable to be overwritten by the references to the outer one. Therefore when we try to do the String type test, it's being checked against the FormatException rather than the "aw beans" string. The type test fails and we fall through to the outer try/catch instead.

I have a fix in progress. But if you need a temporary workaround the bug can be avoided by making the inner try/catch async:

Future<void> main() async {
  try {
    try {
      await Future(() => throw FormatException("oh no"));
    } catch (error) {
      print("catch 1: $error");
      try {
        await Future.value(); // Add a no-op await to make this async.
        throw "aw beans";
      } on String catch (error) {
        print("catch 2: $error");
      }
    }
  } catch (error) {
    print("catch 3: $error");
  }
}

[nex3]

This also happens if null or undefined is thrown—that's forbidden in Dart, but valid in JS, and so probably shouldn't crash the app.

[lrhn]

That's actually an interesting case.
A null value thrown through JavaScript should be uncatchable by Dart code, since all Dart catch clauses have an implicit on Object if they have no explicit on type.
That check is probably not implemented, assuming that you cannot throw null anyway, but it needs to be performed to avoid binding null to a non-nullable catch variable.

[nex3]

If that ends up throwing a TypeError, how can Dart code handle arbitrary errors thrown from JavaScript? Would it be possible to specify that the null gets caught as something different (maybe the TypeError itself)?

[lrhn]

We generally don't specify anything about values which are not Dart values, or about things that cannot happen within the language, it's up to the individual backends to make any value appear to be a Dart value (subtype of Object or null), and manage what happens if something that wouldn't be sound in Dart happens anyway

I think it is reasonable for web compilers to treat any JS object which is not a Dart object, as being instances of JSObject, and let any thrown null value be catchable by a catch with no on type, by converting it to another object.
Maybe the old NullThrownError could be reintroduced for that purpose, or a type error of some sort.
(Or just make it uncatchable, if there is going to be a == null check in every catch anyway, to convert the value to the correct error, we can make it do anything in that branch. It's not like throwing null is that useful.)

[nex3]

Another option, if you want to avoid == null for every catch, would be to allow on Null catch or on Object? catch at the language level even though there's no way to throw null in pure Dart.

[lrhn]

There are lots of places in the SDK, and the async API, where we assume that a caught value is not null, so we'd have to do something if we actually catch null.

We could allow on Null catch (e) or on Object? catch (e) and make e nullable there, while keeping the default of on Object if you don't write anything. Then a magically thrown null will be uncaught by } catch (e) { ... }, but not completely uncatchable.

But it probably means that everybody will start to defensively write on Object? and then start arguing that Completer.completeError should accept null as argument. I don't want to go in that direction.
(If we had allowed throwing null from the start, I would also have had no problems with allowing it, but we're dug in to the "thrown objects are Object" now, and changing that will be breaking. And for no good reason on native, where you cannot throw null.)

I'd prefer that on the web, we either just don't catch null at all, or if we catch it, we convert it to an internale _NullThrownError or type error before exposing it to Dart code.

[nex3]

I'd prefer that on the web, we either just don't catch null at all, or if we catch it, we convert it to an internale _NullThrownError or type error before exposing it to Dart code.

The latter would be acceptable. The former would make it extremely hard to work with JS code that involves callbacks.