Fix error handling in dart2js async-await

sdk/lib/_internal/compiler/js_lib/js_helper.dart

Index: sdk/lib/_internal/compiler/js_lib/js_helper.dart
diff --git a/sdk/lib/_internal/compiler/js_lib/js_helper.dart b/sdk/lib/_internal/compiler/js_lib/js_helper.dart
index b1705388bd0b3cd0032f832cf4e66c74dea001a6..16801a99d567449cba8f6e2ac08f1db4202b966b 100644
--- a/sdk/lib/_internal/compiler/js_lib/js_helper.dart
+++ b/sdk/lib/_internal/compiler/js_lib/js_helper.dart
@@ -4,6 +4,8 @@
 
 library _js_helper;
 
+import 'dart:_async_await_error_codes' as async_error_codes;
+
 import 'dart:_js_embedded_names' show
     GET_TYPE_FROM_NAME,
     GET_ISOLATE_TAG,
@@ -26,13 +28,13 @@ import 'dart:_isolate_helper' show
     leaveJsAsync;
 
 import 'dart:async' show
-  Future,
-  DeferredLoadException,
-  Completer,
-  StreamController,
-  Stream,
-  StreamSubscription,
-  scheduleMicrotask;
+    Future,
+    DeferredLoadException,
+    Completer,
+    StreamController,
+    Stream,
+    StreamSubscription,
+    scheduleMicrotask;
 
 import 'dart:_foreign_helper' show
     DART_CLOSURE_TO_JS,
@@ -3477,59 +3479,60 @@ void mainHasTooManyParameters() {
 ///
 /// If [object] is not a future it will be wrapped in a `new Future.value`.
 ///
-/// If [helperCallback] is null it indicates a return from the async function,
-/// and we complete the completer with object.
-///
-/// Otherwise [helperCallback] is set up to be called when the future is
-/// successfull and [errorCallback] if it is completed with an error.
+/// If [asyncBody] is [async_error_codes.SUCCESS]/[async_error_codes.ERROR] it
+/// indicates a return or throw from the async function, and
+/// complete/completeError is called on [completer] with [object].
 ///
-/// If helperCallback or errorCallback throws we complete the completer with the
-/// error.
+/// Otherwise [asyncBody] is set up to be called when the future is completed
+/// with a code [async_error_codes.SUCCESS]/[async_error_codes.ERROR] depending
+/// on the success of the future.
 ///
 /// Returns the future of the completer for convenience of the first call.
-dynamic thenHelper(dynamic object,
-                   dynamic /* js function */ helperCallback,
-                   Completer completer,
-                   dynamic /* js function */ errorCallback) {
-  if (helperCallback == null) {
+dynamic asyncHelper(dynamic object,
+                    dynamic /* js function */ asyncBody,
+                    Completer completer) {
+  if (asyncBody == async_error_codes.SUCCESS) {
     completer.complete(object);
     return;
+  } else if (asyncBody == async_error_codes.ERROR) {
+    // The error will be a js-error.

[floitsch, 2015/02/13 15:30:07]

is a

[sigurdm, 2015/02/17 08:43:10]

Done.

+    completer.completeError(unwrapException(object),
+                            getTraceFromException(object));
+    return;
   }
   Future future = object is Future ? object : new Future.value(object);
-  future.then(_wrapJsFunctionForThenHelper(helperCallback, completer),
-              onError: (errorCallback == null)
-                  ? null
-                  : _wrapJsFunctionForThenHelper(errorCallback, completer));
+  future.then(_wrapJsFunctionForAsync(asyncBody, async_error_codes.SUCCESS),
+      onError: _wrapJsFunctionForAsync(asyncBody, async_error_codes.ERROR));
   return completer.future;
 }
 
-Function _wrapJsFunctionForThenHelper(dynamic /* js function */ function,
-                                      Completer completer) {
+Function _wrapJsFunctionForAsync(dynamic /* js function */ function,
+                                 int errorCode) {
   return (result) {
-    try {
-      JS('', '#(#)', function, result);
-    } catch (e, st) {
-      completer.completeError(e, st);
-    }
+    JS('', '#(#, #)', function, errorCode, result);
   };
 }
 
-
 /// Implements the runtime support for async* functions.
 ///
 /// Called by the transformed function for each original return, await, yield,
 /// yield* and before starting the function.
 ///
-/// When the async* function wants to return it calls this function. with
-/// [helperCallback] == null, the streamHelper takes this as signal to close the
-/// stream.
+/// When the async* function wants to return it calls this function with
+/// [asyncBody] == [async_error_codes.SUCCESS], the asyncStarHelper takes this
+/// as signal to close the stream.
+///
+/// When the async* function wants to signal that an uncaught error was thrown,
+/// it calls this function with [asyncBody] == [async_error_codes.ERROR],
+/// the streamHelper takes this as signal to addError [object] to the
+/// [controller] and close it.
 ///
-/// If the async* function wants to do a yield or yield* it calls this function
-/// with [object] being an [IterationMarker]. In this case [errorCallback] has a
-/// special meaning; it is a callback that will run all enclosing finalizers.
+/// If the async* function wants to do a yield or yield*, it calls this function
+/// with [object] being an [IterationMarker].
 ///
 /// In the case of a yield or yield*, if the stream subscription has been
-/// canceled [errorCallback] is scheduled.
+/// canceled, schedules [asyncBody] to be called with
+/// [async_error_codes.STREAM_WAS_CANCELED].
 ///
 /// If [object] is a single-yield [IterationMarker], adds the value of the
 /// [IterationMarker] to the stream. If the stream subscription has been
@@ -3539,30 +3542,32 @@ Function _wrapJsFunctionForThenHelper(dynamic /* js function */ function,
 /// If [object] is a yield-star [IterationMarker], starts listening to the
 /// yielded stream, and adds all events and errors to our own controller (taking
 /// care if the subscription has been paused or canceled) - when the sub-stream
-/// is done, schedules [helperCallback] again.
+/// is done, schedules [asyncBody] again.
 ///
 /// If the async* function wants to do an await it calls this function with
 /// [object] not and [IterationMarker].
 ///
 /// If [object] is not a [Future], it is wrapped in a `Future.value`.
-/// The [helperCallback] is called on successfull completion of the
-/// future.
-///
-/// If [helperCallback] or [errorCallback] throws the error is added to the
-/// stream.
-void streamHelper(dynamic object,
-                     dynamic /* js function */ helperCallback,
-                     AsyncStarStreamController controller,
-                     dynamic /* js function */ errorCallback) {
-  if (helperCallback == null) {
+/// The [asyncBody] is called on completion of the future (see [asyncHelper].
+void asyncStarHelper(dynamic object,
+                     dynamic /* int | js function */ asyncBody,

[floitsch, 2015/02/13 15:30:07]

bodyFunctionOrErrorCode ?

It's long, but not used often anyway.

[sigurdm, 2015/02/17 08:43:10]

Good point. I will use identical.

+                     AsyncStarStreamController controller) {
+  if (asyncBody == async_error_codes.SUCCESS) {

[floitsch, 2015/02/13 15:30:07]

nit.
use identical, or invert the order.
The asyncBody is marked as dynamic (which the inferrer cannot infer). This means
that it currently needs to get an interceptor, ...
If you switch to "identical" or invert the order, it produces much better code.

[sigurdm, 2015/02/17 08:43:10]

Done.

     // This happens on return from the async* function.
     controller.close();
     return;
+  } else if (asyncBody == async_error_codes.ERROR) {

[floitsch, 2015/02/13 15:30:07]

ditto.

[sigurdm, 2015/02/17 08:43:10]

Done.

+    // The error will be a js-error.

[floitsch, 2015/02/13 15:30:07]

is

[sigurdm, 2015/02/17 08:43:10]

Done.

+    controller.addError(unwrapException(object),
+                        getTraceFromException(object));
+    controller.close();
+    return;
   }
 
   if (object is IterationMarker) {
     if (controller.stopRunning) {
-      _wrapJsFunctionForStream(errorCallback, controller)();
+      _wrapJsFunctionForAsync(asyncBody,
+          async_error_codes.STREAM_WAS_CANCELED)(null);
       return;
     }
     if (object.state == IterationMarker.YIELD_SINGLE) {
@@ -3573,7 +3578,8 @@ void streamHelper(dynamic object,
       }
       // TODO(sigurdm): We should not suspend here according to the spec.
       scheduleMicrotask(() {
-          _wrapJsFunctionForStream(helperCallback, controller)(null);
+        _wrapJsFunctionForAsync(asyncBody, async_error_codes.SUCCESS)
+            (null);
       });
       return;
     } else if (object.state == IterationMarker.YIELD_STAR) {
@@ -3584,17 +3590,16 @@ void streamHelper(dynamic object,
       // TODO(sigurdm): The spec is not very clear here. Clarify with Gilad.
       controller.addStream(stream).then((_) {
         controller.isAdding = false;
-        _wrapJsFunctionForStream(helperCallback, controller)(null);
+        _wrapJsFunctionForAsync(asyncBody, async_error_codes.SUCCESS)(null);
       });
       return;
     }
   }
 
   Future future = object is Future ? object : new Future.value(object);
-  future.then(_wrapJsFunctionForStream(helperCallback, controller),
-              onError: errorCallback == null
-                  ? null
-                  : _wrapJsFunctionForStream(errorCallback, controller));
+  future.then(_wrapJsFunctionForAsync(asyncBody, async_error_codes.SUCCESS),
+              onError: _wrapJsFunctionForAsync(asyncBody,
+                                               async_error_codes.ERROR));
 }
 
 Stream streamOfController(AsyncStarStreamController controller) {
@@ -3622,11 +3627,13 @@ class AsyncStarStreamController {
   AsyncStarStreamController(helperCallback) {
     controller = new StreamController(
       onListen: () {
-        scheduleMicrotask(() => JS('', '#(null)', helperCallback));
+        scheduleMicrotask(() {
+          JS('', '#(#, null)',helperCallback, async_error_codes.SUCCESS);

[floitsch, 2015/02/13 15:30:07]

missing space.

[sigurdm, 2015/02/17 08:43:10]

Done.

+        });
       },
       onResume: () {
         if (!isAdding) {
-          streamHelper(null, helperCallback, this, null);
+          asyncStarHelper(null, helperCallback, this);
         }
       }, onCancel: () {
         stopRunning = true;
@@ -3638,18 +3645,6 @@ makeAsyncStarController(helperCallback) {
   return new AsyncStarStreamController(helperCallback);
 }
 
-Function _wrapJsFunctionForStream(dynamic /* js function */ function,
-                                  AsyncStarStreamController controller) {
-  return (result) {
-    try {
-      JS('', '#(#)', function, result);
-    } catch (e, st) {
-      controller.addError(e, st);
-    }
-  };
-}
-
-
 class IterationMarker {
   static const YIELD_SINGLE = 0;
   static const YIELD_STAR = 1;
@@ -3676,7 +3671,7 @@ class IterationMarker {
 }
 
 class SyncStarIterator implements Iterator {
-  final Function _helper;
+  final Function _body;
 
   // If [runningNested] this is the nested iterator, otherwise it is the
   // current value.
@@ -3685,8 +3680,8 @@ class SyncStarIterator implements Iterator {
 
   get current => _runningNested ? _current.current : _current;
 
-  SyncStarIterator(helper)
-      : _helper = ((arg) => JS('', '#(#)', helper, arg));
+  SyncStarIterator(body)
+      : _body = (() => JS('', '#()', body));
 
   bool moveNext() {
     if (_runningNested) {
@@ -3696,7 +3691,7 @@ class SyncStarIterator implements Iterator {
         _runningNested = false;
       }
     }
-    _current = _helper(null);
+    _current = _body();
     if (_current is IterationMarker) {
       if (_current.state == IterationMarker.ITERATION_ENDED) {
         _current = null;
@@ -3720,7 +3715,7 @@ class SyncStarIterable extends IterableBase {
   // This is a function that will return a helper function that does the
   // iteration of the sync*.
   //
-  // Each invocation should give a helper with fresh state.
+  // Each invocation should give a body with fresh state.
   final dynamic /* js function */ _outerHelper;
 
   SyncStarIterable(this._outerHelper);