js-interop: Dart exceptions and errors thrown through js-interop are not un-jsified.

[kealjones-wk]

Setup:

• Dart SDK Version: 2.2.0
• OS: macOS 10.14.2
• Browser: Chrome 73.0.3683.86
• pkg/js version: "0.6.1+1"

Does it happen in Dartium or when compiled to JavaScript?: Both
Describe the issue you're seeing:
When a dart exception is thrown and is caught by javascript if the error is then passed back into a dart callback the error comes through as a JsObject in dart2js and as a NativeJavaScriptObject in DDC. Is this an issue or is this expected behavior?

Workarounds:
We have found ways around this, that i wanted ask about and see if they are safe ways to go about re-dartifying thrown exceptions or if these solutions were potentially problematic:
In these examples _JoesException is a dart class that extends from Exception And just adds some additional fields.

in dart2js:

_JoesException joesError = js_util.getProperty(error, 'dartException');

in ddc:

_JoesException joesError = _getSymbolProperty(error, 'Symbol(_thrownValue)');

The js side function accessed in dart using package:js/js.dart in example two is:

function _getSymbolProperty(obj, key){
  var sym = Object.getOwnPropertySymbols(obj).find(function(s) {
    return String(s) === key;
  });
  return obj[sym];
}

Failing code:
It is not really failing, this is where the issue was observed. We are seeing it in componentDidCatch and getDerivedStateFromError.
PR: Workiva/react-dart#175
You can test using this file: https://github.com/cleandart/react-dart/blob/785f74ca68c8c4ecf3c051459d6357d29813f636/example/test/react_test_components.dart
Run pub run build_runner serve example [-r] and navigate to http://localhost:8080/test/react_test.html
In _Component2ErrorTestComponent you can update the componentDidCatch method to use the info argument or getDerivedStateFromError method to use the error argument.

Any insight or better more correct solutions are totally welcome! Also if you need more detail about anything let me know I am happy to share.

[aaronlademann-wf]

@kevmoo @robbecker-wf @johnpryan

[kealjones-wk]

Just hit this issue again for https://github.com/Workiva/react_testing_library/

We were using the window.onError from dart:html to catch errors from event handlers.
Test Case: onClick: (_) => throw StateError('EXPLOSION')

final errorStream = window.onError.cast<ErrorEvent>().listen((event) {
  errors.add(event.error);
});

event.error is typed as Object, fair enough considering this is a catchall for unhandled errors, including non-dart errors, but even trying to do is type checks on known dart errors fail and it ultimately is always a NativeJavaScriptObject.

We worked around this by rethrowing event.error from JS, Dart for whatever reason will catch this throw from JS and dartify that error...

js:

function throwErrorFromJS(error) {
  throw error;
}

dart:

@JS()
external void throwErrorFromJS(error);

then just call the interop throwErrorFromJS and boom correctly typed Dart error!
if (errors.isNotEmpty) throwErrorFromJS(errors.first);

[kevmoo]

@srujzs - is this on your radar?

[srujzs]

Nope, but now it is. :) The difference in type you see between the compilers is just a difference in the way the two compilers reify unknown JS objects. It might be worth unifying the runtime type so it's less confusing, but that's tangential.

It's interesting that the exception is caught when it's directly thrown from JS but not in the dart:html case. I'll have to repro the different examples locally and report back.

[KealJones]

@srujzs I hope this helps, I wrote a dartpad based example that demonstrates getting the error in dart then throwing in js to dartify:
Gist: https://gist.github.com/kealjones-wk/0e955521d53848aef580ebc837642b47
Dartpad: https://dartpad.dev/?id=0e955521d53848aef580ebc837642b47

[srujzs]

I think the behavior you see is a consequence of the way we handle exceptions in the web compilers. We wrap values that are thrown and then they are unwrapped when caught in a catch block. In the Dart world, this is fine, but it presents issues with interop. Since we never catch the error, the error never gets unwrapped, which is what you've noticed.

It's also the same reason why throwing it from JS and catching it in Dart ends up unwrapping the error. The compilers don't try to unwrap something that's already unwrapped, so even if you called throwFromJS('hello world') and catch it in Dart, it'd work as expected. The struggle though is that if we throw in Dart, we don't know whether or not it's caught in Dart or JS, so we don't know concretely whether it should be wrapped or not.

Fixing this might be complicated, but I suppose there are two options:

1. Ironically enough, rethrowing through JS and catching it in Dart is a solution. We could make this simpler by adding similar functionality to a JS library (or have it just unwrap if possible directly).
2. A potentially better solution might be to make some changes to the RTI to handle the wrapped errors. is and as would handle it such that the underlying error also gets checked, with the as check doing the unwrapping. Right now, the wrapped errors are JavaScriptObjects so that'll need to be changed. This would at least allow you to treat the error as the expected Dart value, even though it requires a cast (although maybe we can modify the error streams in dart:html to do these casts beforehand). I'm not sure what complications might arise from this however. For example, are there instances where we don't want wrapped errors to be treated as the underlying error?

@sigmundch, any thoughts on this?