Fix error handling in dart2js async-await

pkg/compiler/lib/src/js/rewrite_async.dart

Index: pkg/compiler/lib/src/js/rewrite_async.dart
diff --git a/pkg/compiler/lib/src/js/rewrite_async.dart b/pkg/compiler/lib/src/js/rewrite_async.dart
index 2946e90e121354702c3f54d038dc8872a9b622d0..d773e46b5c07c768dd4fe4c242de7b2c28de3c82 100644
--- a/pkg/compiler/lib/src/js/rewrite_async.dart
+++ b/pkg/compiler/lib/src/js/rewrite_async.dart
@@ -4,13 +4,15 @@
 
 library rewrite_async;
 
-// TODO(sigurdm): Throws in catch-handlers are handled wrong.
 // TODO(sigurdm): Avoid using variables in templates. It could blow up memory
 // use.
 
 import "dart:math" show max;
 import 'dart:collection';
 
+import 'package:_internal/compiler/js_lib/shared/async_await_error_codes.dart'
+    as error_codes;
+
 import "js.dart" as js;
 
 import '../util/util.dart';
@@ -37,14 +39,30 @@ class AsyncRewriter extends js.NodeVisitor {
 
   Map<js.Node, int> continueLabels = new Map<js.Node, int>();
   Map<js.Node, int> breakLabels = new Map<js.Node, int>();
-  Map<js.Node, int> finallyLabels = new Map<js.Node, int>();
+
+  /// The label of a finally part.
+  Map<js.Block, int> finallyLabels = new Map<js.Block, int>();
+
+  /// The label of the catch handler of a [js.Try] or a [js.Fun] or [js.Catch].

[floitsch, 2015/02/16 18:26:36]

Not sure I understand the comment.
is it ([js.Try] or a [js.Fun] or [js.Catch]), or
(the catch handler of a [js.Try]), or a [js.Fun] or [js.Catch].?

In any case it's not clear.

[sigurdm, 2015/02/17 10:29:05]

I try to explain more in depth.

+  /// The catch handler of a catch block will redirect to the enclosing handler
+  /// after running all the finally blocks inbetween.
+  Map<js.Node, int> handlerLabels = new Map<js.Node, int>();
+
   int exitLabel;
+  int rethrowLabel;
 
-  // A stack of all enclosing jump targets (including the function for
-  // representing the target of a return, and all enclosing try-blocks that have
-  // finally part, this way ensuring all the finally blocks between a jump and
-  // its target are run before the jump.
-  List<js.Node> targetsAndTries = new List<js.Node>();
+  /// A stack of all enclosing jump targets, pending finally blocks and
+  /// catch handlers.
+  ///
+  /// The jump-targets include the function for

[floitsch, 2015/02/16 18:26:36]

the function, representing the target of a return.

[sigurdm, 2015/02/17 10:29:06]

Done.

+  /// representing the target of a return,
+  ///
+  /// The enclosing catch-handlers include the enclosing function for rethrowing
+  /// an uncaught error.

[floitsch, 2015/02/16 18:26:36]

That sounds like the function is used either for the "return" or for rethrowing
uncaught errors. correct?

[sigurdm, 2015/02/17 10:29:06]

Yes, it is used for both.

+  ///
+  /// This is used to ensure all the finally blocks between a jump and
+  /// its target are run before the jump.
+  List<js.Node> targetsCatchesAndFinallies = new List<js.Node>();

[floitsch, 2015/02/16 18:26:36]

/// A stack of all (surrounding) jump targets.
///
/// Jump targets are:
///
/// * The function, signalling a return or uncaught throw.
/// * Loops.
/// * LabeledStatements (also used for 'continue' when attached to loops).
/// * Try statements, for catch and finally handlers.
///
/// When jumping to a target it is necessary to visit all finallies that
/// are on the way to target (i.e. more nested than the jump target).
List<js.Node> jumpTargetStack = new List<js.Node>();

[sigurdm, 2015/02/17 10:29:06]

This is not very precise. But might be easier to understand.

 
   List<int> continueStack = new List<int>();
   List<int> breakStack = new List<int>();
@@ -55,8 +73,6 @@ class AsyncRewriter extends js.NodeVisitor {
 
   PreTranslationAnalysis analysis;
 
-  List<int> errorHandlerLabels = new List<int>();
-
   final Function safeVariableName;
 
   // All the <x>Name variables are names of Javascript variables used in the
@@ -86,13 +102,17 @@ class AsyncRewriter extends js.NodeVisitor {
   ///   }
   /// }
   ///
-  /// It is a parameter to the [helperName] function, so that [thenHelper] and
-  /// [streamHelper] can call [helperName] with the result of an awaited Future.
+  /// It is a parameter to the [bodyName] function, so that [asyncHelper] and
+  /// [streamHelper] can call [bodyName] with the result of an awaited Future.
   String resultName;
 
+  /// A parameter to the [bodyName] function. Indicating if we are in success
+  /// or error case.
+  String errorCodeName;
+
   /// The name of the inner function that is scheduled to do each await/yield,
   /// and called to do a new iteration for sync*.
-  String helperName;
+  String bodyName;
 
   /// The Completer that will finish an async function.
   ///
@@ -121,9 +141,16 @@ class AsyncRewriter extends js.NodeVisitor {
   /// the last.
   String nextName;
 
+  /// The stack of labels of finally blocks to assign to [nextName] if the
+  /// async* [StreamSubscription] was canceled during a yield.
+  String nextWhenCanceledName;
+
   /// The current returned value (a finally block may overwrite it).
   String returnValueName;
 
+  /// If we are in the process of handling an error, stores the current error.
+  String currentErrorName;
+
   /// The label of the outer loop.
   ///
   /// Used if there are untransformed loops containing break or continues to
@@ -131,7 +158,7 @@ class AsyncRewriter extends js.NodeVisitor {
   String outerLabelName;
 
   /// If javascript `this` is used, it is accessed via this variable, in the
-  /// [helperName] function.
+  /// [bodyName] function.
   String selfName;
 
   // These expressions are hooks for communicating with the runtime.
@@ -141,19 +168,21 @@ class AsyncRewriter extends js.NodeVisitor {
   /// For an await it is called with:
   ///
   /// - The value to await
-  /// - The [helperName]
-  /// - The [completerName]
-  /// - A JavaScript function that is executed if the future completed with
-  ///   an error. That function is responsible for executing the right error
-  ///   handler and/or finally blocks).
+  /// - The body function [bodyName]
+  /// - The completer object [completerName]
   ///
   /// For a return it is called with:
   ///
   /// - The value to complete the completer with.
-  /// - null
+  /// - 0
   /// - The [completerName]

[floitsch, 2015/02/16 18:26:36]

nit: the completer object [completerName]

[sigurdm, 2015/02/17 10:29:05]

Done.

-  /// - null.
-  final js.Expression thenHelper;
+  ///
+  /// For a throw it is called with:
+  ///
+  /// - The value to complete the completer with.
+  /// - 1
+  /// - The [completerName]

[floitsch, 2015/02/16 18:26:36]

ditto.

[sigurdm, 2015/02/17 10:29:06]

Done.

+  final js.Expression asyncHelper;
 
   /// The function called by an async* function to simulate an await, yield or
   /// yield*.
@@ -162,11 +191,8 @@ class AsyncRewriter extends js.NodeVisitor {
   ///
   /// - The value to await/yieldExpression(value to yield)/
   /// yieldStarExpression(stream to yield)
-  /// - The [helperName]
-  /// - The [controllerName]
-  /// - A JavaScript function that is executed if the future completed with
-  ///   an error. That function is responsible for executing the right error
-  ///   handler and/or finally blocks).
+  /// - The body function [bodyName]
+  /// - The controller object [controllerName]
   ///
   /// For a return it is called with:
   ///
@@ -192,7 +218,7 @@ class AsyncRewriter extends js.NodeVisitor {
   final js.Expression streamOfController;
 
   /// Contructor creating the Iterable for a sync* method. Called with
-  /// [helperName].
+  /// [bodyName].
   final js.Expression newIterable;
 
   /// A JS Expression that creates a marker showing that iteration is over.
@@ -235,7 +261,7 @@ class AsyncRewriter extends js.NodeVisitor {
 
   AsyncRewriter(this.diagnosticListener,
                 spannable,
-                {this.thenHelper,
+                {this.asyncHelper,
                  this.streamHelper,
                  this.streamOfController,
                  this.newCompleter,
@@ -263,14 +289,17 @@ class AsyncRewriter extends js.NodeVisitor {
     // To avoid name collisions with existing names, the fresh names are
     // generated after the analysis.
     resultName = freshName("result");
+    errorCodeName = freshName("errorCode");
     completerName = freshName("completer");
     controllerName = freshName("controller");
-    helperName = freshName("helper");
+    bodyName = freshName("body");
     gotoName = freshName("goto");
     handlerName = freshName("handler");
     errorName = freshName("error");
     nextName = freshName("next");
+    nextWhenCanceledName = freshName("nextWhenCanceled");
     returnValueName = freshName("returnValue");
+    currentErrorName = freshName("storedError");

[floitsch, 2015/02/16 18:26:36]

freshName("currentError")

[sigurdm, 2015/02/17 10:29:06]

Done.

     outerLabelName = freshName("outer");
     selfName = freshName("self");
 
@@ -278,9 +307,8 @@ class AsyncRewriter extends js.NodeVisitor {
   }
 
   js.Expression get currentErrorHandler {
-    return errorHandlerLabels.isEmpty
-        ? new js.LiteralNull()
-        : js.number(errorHandlerLabels.last);
+    return js.number(handlerLabels[targetsCatchesAndFinallies.lastWhere(
+        (node) => handlerLabels[node] != null)]);
   }
 
   int allocateTempVar() {
@@ -542,7 +570,7 @@ class AsyncRewriter extends js.NodeVisitor {
   /// through all the enclosing finally blocks). The jump to here is made in
   /// [visitReturn].
   ///
-  /// Returning from an async method calls the [thenHelper] with the result.
+  /// Returning from an async method calls the [asyncHelper] with the result.
   /// (the result might have been stored in [returnValueName] by some finally
   /// block).
   ///
@@ -561,26 +589,35 @@ class AsyncRewriter extends js.NodeVisitor {
         String returnValue =
             analysis.hasExplicitReturns ? returnValueName : "null";
         addStatement(js.js.statement(
-            "return #thenHelper($returnValue, null, $completerName, null)", {
-          "thenHelper": thenHelper
-        }));
+            "return #thenHelper($returnValue, #successCode, "
+                "$completerName, null)", {
+             "thenHelper": asyncHelper,
+             "successCode": js.number(error_codes.SUCCESS)}));
         break;
       case const js.AsyncModifier.syncStar():
         addStatement(new js.Return(new js.Call(endOfIteration, [])));
         break;
       case const js.AsyncModifier.asyncStar():
         addStatement(js.js.statement(
-            "return #streamHelper(null, null, $controllerName, null)", {
-          "streamHelper": streamHelper
-        }));
+            "return #streamHelper(null, #successCode, $controllerName)", {
+          "streamHelper": streamHelper,
+          "successCode": js.number(error_codes.SUCCESS)}));
         break;
       default:
         diagnosticListener.internalError(
             spannable, "Internal error, unexpected asyncmodifier $async");
     }
+    if (isAsync || isAsyncStar) {
+      beginLabel(rethrowLabel);
+      addStatement(js.js.statement(
+          "return #thenHelper($currentErrorName, #errorCode, "
+          "${isAsync ? completerName : controllerName})", {
+            "thenHelper": isAsync ? asyncHelper : streamHelper,
+            "errorCode": js.number(error_codes.ERROR)}));
+    }
   }
 
-  /// The initial call to [thenHelper]/[streamHelper].
+  /// The initial call to [asyncHelper]/[streamHelper].
   ///
   /// There is no value to await/yield, so the first argument is `null` and
   /// also the errorCallback is `null`.
@@ -590,8 +627,8 @@ class AsyncRewriter extends js.NodeVisitor {
   js.Statement generateInitializer() {
     if (isAsync) {
       return js.js.statement(
-          "return #thenHelper(null, $helperName, $completerName, null);", {
-        "thenHelper": thenHelper
+          "return #asyncHelper(null, $bodyName, $completerName, null);", {
+        "asyncHelper": asyncHelper
       });
     } else if (isAsyncStar) {
       return js.js.statement(
@@ -607,17 +644,17 @@ class AsyncRewriter extends js.NodeVisitor {
   /// Rewrites an async/sync*/async* function to a normal Javascript function.
   ///
   /// The control flow is flattened by simulating 'goto' using a switch in a
-  /// loop and a state variable [gotoName] inside a helper-function that can be
-  /// called back by [thenHelper]/[streamHelper]/the [Iterator].
+  /// loop and a state variable [gotoName] inside a nested function [bodyName]
+  /// that can be called back by [asyncHelper]/[asyncStarHelper]/the [Iterator].
   ///
   /// Local variables are hoisted outside the helper.
   ///
   /// Awaits in async/async* are translated to code that remembers the current
   /// location (so the function can resume from where it was) followed by a call
-  /// to the [thenHelper]. The helper sets up the waiting for the awaited value
+  /// to the [asyncHelper]. The helper sets up the waiting for the awaited value
   /// and returns a future which is immediately returned by the translated
   /// await.
-  /// Yields in async* are translated to a call to the [streamHelper]. They,
+  /// Yields in async* are translated to a call to the [asyncStarHelper]. They,
   /// too, need to be prepared to be interrupted in case the stream is paused or
   /// canceled. (Currently we always suspend - this is different from the spec,
   /// see `streamHelper` in `js_helper.dart`).
@@ -635,7 +672,7 @@ class AsyncRewriter extends js.NodeVisitor {
   ///   return bar(p);
   /// }
   ///
-  /// Becomes:
+  /// Becomes (without error handling):
   ///
   /// function(x, y, z) {
   ///   var goto = 0, returnValue, completer = new Completer(), p;
@@ -644,17 +681,17 @@ class AsyncRewriter extends js.NodeVisitor {
   ///       switch (goto) {
   ///         case 0:
   ///           goto = 1 // Remember where to continue when the future succeeds.
-  ///           return thenHelper(foo(), helper, completer, null);
+  ///           return thenHelper(foo(), helper, completer);
   ///         case 1:
   ///           p = result;
   ///           returnValue = bar(p);
   ///           goto = 2;
   ///           break;
   ///         case 2:
-  ///           return thenHelper(returnValue, null, completer, null)
+  ///           return thenHelper(returnValue, null, completer)
   ///       }
   ///     }
-  ///     return thenHelper(null, helper, completer, null);
+  ///     return thenHelper(null, helper, completer);
   ///   }
   /// }
   ///
@@ -689,11 +726,16 @@ class AsyncRewriter extends js.NodeVisitor {
   ///   var goto = 0;
   ///   var returnValue;
   ///   var completer = new Completer();
-  ///   var handler = null;
+  ///   var handler = 8; // Outside try-blocks go to the rethrow label.
   ///   var p;
+  ///   var storedError;
   ///   // The result can be either the result of an awaited future, or an
   ///   // error if the future completed with an error.
-  ///   function helper(result) {
+  ///   function helper(errorCode, result) {
+  ///     if (errorCode == 1) {
+  ///       storedError = result;
+  ///       goto = handler;
+  ///     }
   ///     while (true) {
   ///       try {
   ///         switch (goto) {
@@ -718,7 +760,9 @@ class AsyncRewriter extends js.NodeVisitor {
   ///             goto = 5; // finally handler for outer try.
   ///             break;
   ///           case 4: // catch handler for outer try.
-  ///             e = result;
+  ///             handler = 5; // If the handler throws, do the finally ..
+  ///             next = [8] // ... and rethrow.
+  ///             e = storedError;
   ///             handle(e);
   ///             // Fall through to finally.
   ///           case 5: // finally handler for outer try.
@@ -728,14 +772,16 @@ class AsyncRewriter extends js.NodeVisitor {
   ///             break;
   ///           case 6: // Exiting outer try.
   ///           case 7: // return
-  ///             return thenHelper(returnValue, null, completer, null);
+  ///             return thenHelper(returnValue, 0, completer);
+  ///           case 8: // Rethrow
+  ///             return thenHelper(storedError, 1, completer);
   ///         }
   ///       } catch (error) {
-  ///         result = error;
+  ///         storedError = error;
   ///         goto = handler;
   ///       }
   ///     }
-  ///     return thenHelper(null, helper, completer, null);
+  ///     return thenHelper(null, helper, completer);
   ///   }
   /// }
   ///
@@ -748,10 +794,12 @@ class AsyncRewriter extends js.NodeVisitor {
     // [visitDartYield].
     exitLabel =
         analysis.hasExplicitReturns || isAsyncStar ? newLabel("return") : null;
+    rethrowLabel = newLabel("rethrow");
+    handlerLabels[node] = rethrowLabel;
     js.Statement body = node.body;
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     addExit();
 
     List<js.SwitchClause> clauses = labelledParts.keys.map((label) {
@@ -762,17 +810,13 @@ class AsyncRewriter extends js.NodeVisitor {
     if (hasJumpThoughOuterLabel) {
       helperBody = js.js.statement("$outerLabelName: #", [helperBody]);
     }
-    if (hasTryBlocks) {
-      helperBody = js.js.statement("""
-          try {
-            #body
-          } catch ($errorName){
-            if ($handlerName === null)
-              throw $errorName;
-            $resultName = $errorName;
-            $gotoName = $handlerName;
-          }""", {"body": helperBody});
-    }
+    helperBody = js.js.statement("""
+        try {
+          #body
+        } catch ($errorName){
+          $currentErrorName = $errorName;
+          $gotoName = $handlerName;
+        }""", {"body": helperBody});
     List<js.VariableInitialization> inits = <js.VariableInitialization>[];
 
     js.VariableInitialization makeInit(String name, js.Expression initValue) {
@@ -785,17 +829,19 @@ class AsyncRewriter extends js.NodeVisitor {
       inits.add(makeInit(completerName, new js.New(newCompleter, [])));
     } else if (isAsyncStar) {
       inits.add(makeInit(controllerName,
-          new js.Call(newController, [new js.VariableUse(helperName)])));
-    }
-    if (hasTryBlocks) {
-      inits.add(makeInit(handlerName, new js.LiteralNull()));
+          new js.Call(newController, [new js.VariableUse(bodyName)])));
     }
+    inits.add(makeInit(handlerName, js.number(rethrowLabel)));
+    inits.add(makeInit(currentErrorName, null));
     if (hasJumpThroughFinally || analysis.hasYield) {
       inits.add(makeInit(nextName, null));
     }
     if (analysis.hasExplicitReturns && isAsync) {
       inits.add(makeInit(returnValueName, null));
     }
+    if (isSyncStar) {
+      inits.add(makeInit(resultName, null));
+    }
     if (analysis.hasThis && !isSyncStar) {
       // Sync* functions must remember `this` on the level of the outer
       // function.
@@ -807,6 +853,7 @@ class AsyncRewriter extends js.NodeVisitor {
     inits.addAll(new Iterable.generate(tempVarHighWaterMark,
         (int i) => makeInit(useTempVar(i + 1).name, null)));
     js.VariableDeclarationList varDecl = new js.VariableDeclarationList(inits);
+    // TODO(sigurdm): Explain the difference between these cases.
     if (isSyncStar) {
       return js.js("""
           function (#params) {
@@ -814,7 +861,7 @@ class AsyncRewriter extends js.NodeVisitor {
               var $selfName = this;
             return new #newIterable(function () {
               #varDecl;
-              return function $helperName($resultName) {
+              return function $bodyName() {
                 while (true)
                   #helperBody;
               };
@@ -831,15 +878,33 @@ class AsyncRewriter extends js.NodeVisitor {
     return js.js("""
         function (#params) {
           #varDecl;
-          function $helperName($resultName) {
+          function $bodyName($errorCodeName, $resultName) {
+            if (#hasYield)
+              switch ($errorCodeName) {
+                case #streamWasCanceled:
+                  $nextName = $nextWhenCanceledName;
+                  $gotoName = $nextName.pop();
+                  break;
+                case #errorCode:
+                  $currentErrorName = $resultName;
+                  $gotoName = $handlerName;
+              }
+            else
+              if ($errorCodeName == #errorCode) {
+                  $currentErrorName = $resultName;
+                  $gotoName = $handlerName;
+              }
             while (true)
               #helperBody;
           }
           #init;
         }""", {
       "params": node.params,
-      "helperBody": helperBody,
       "varDecl": varDecl,
+      "streamWasCanceled": js.number(error_codes.STREAM_WAS_CANCELED),
+      "errorCode": js.number(error_codes.ERROR),
+      "hasYield": analysis.hasYield,
+      "helperBody": helperBody,
       "init": generateInitializer()
     });
   }
@@ -888,7 +953,7 @@ class AsyncRewriter extends js.NodeVisitor {
     }
   }
 
-  /// An await is translated to a call to [thenHelper]/[streamHelper].
+  /// An await is translated to a call to [asyncHelper]/[streamHelper].
   ///
   /// See the comments of [visitFun] for an example.
   @override
@@ -897,23 +962,13 @@ class AsyncRewriter extends js.NodeVisitor {
     int afterAwait = newLabel("returning from await.");
     withExpression(node.expression, (js.Expression value) {
       addStatement(setGotoVariable(afterAwait));
-      js.Expression errorCallback = errorHandlerLabels.isEmpty
-          ? new js.LiteralNull()
-          : js.js("""
-            function($errorName) {
-              $gotoName = #currentHandler;
-              $helperName($errorName);
-            }""", {"currentHandler": currentErrorHandler});
-
       addStatement(js.js.statement("""
-          return #thenHelper(#value,
-                             $helperName,
-                             ${isAsync ? completerName : controllerName},
-                             #errorCallback);
+          return #asyncHelper(#value,
+                             $bodyName,

[floitsch, 2015/02/16 18:26:36]

nit: indentation.

[sigurdm, 2015/02/17 10:29:06]

Done.

+                             ${isAsync ? completerName : controllerName});
           """, {
-        "thenHelper": isAsync ? thenHelper : streamHelper,
+        "asyncHelper": isAsync ? asyncHelper : streamHelper,
         "value": value,
-        "errorCallback": errorCallback
       }));
     }, store: false);
     beginLabel(afterAwait);
@@ -1073,9 +1128,8 @@ class AsyncRewriter extends js.NodeVisitor {
     // the jump.
     // The bottom of the stack is the label where the jump goes to.
     List<int> jumpStack = new List<int>();
-    for (js.Node node in targetsAndTries.reversed) {
-      if (node is js.Try) {
-        assert(node.finallyPart != null);
+    for (js.Node node in targetsCatchesAndFinallies.reversed) {
+      if (finallyLabels[node] != null) {
         jumpStack.add(finallyLabels[node]);
       } else if (node == target) {
         jumpStack.add(targetLabel);
@@ -1120,9 +1174,9 @@ class AsyncRewriter extends js.NodeVisitor {
 
     beginLabel(startLabel);
 
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
 
     beginLabel(continueLabel);
     withExpression(node.condition, (js.Expression condition) {
@@ -1191,9 +1245,9 @@ class AsyncRewriter extends js.NodeVisitor {
             new js.Prefix("!", condition), gotoAndBreak(afterLabel)));
       }, store: false);
     }
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     if (node.update != null) {
       beginLabel(continueLabel);
       visitExpressionIgnoreResult(node.update);
@@ -1294,9 +1348,9 @@ class AsyncRewriter extends js.NodeVisitor {
     continueLabels[node] = continueLabel;
 
     beginLabel(continueLabel);
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     beginLabel(breakLabel);
   }
 
@@ -1502,13 +1556,13 @@ class AsyncRewriter extends js.NodeVisitor {
       }
     }
 
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     for (int i = 0; i < labels.length; i++) {
       beginLabel(labels[i]);
       visitStatement(node.cases[i].body);
     }
     beginLabel(after);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
   }
 
   @override
@@ -1523,12 +1577,30 @@ class AsyncRewriter extends js.NodeVisitor {
     }, store: false);
   }
 
-  setErrorHandler() {
+  setErrorHandler([int errorHandler]) {
     addExpressionStatement(new js.Assignment(
-        new js.VariableUse(handlerName), currentErrorHandler));
+        new js.VariableUse(handlerName),
+        errorHandler == null ? currentErrorHandler : js.number(errorHandler)));
   }
 
-  @override
+  List<int> _finalliesUpToAndEnclosingHandler() {
+    List<int> result = new List<int>();
+    for (int i = targetsCatchesAndFinallies.length - 1; i >= 0; i--) {
+      js.Node node = targetsCatchesAndFinallies[i];
+      int handlerLabel = handlerLabels[node];
+      if (handlerLabel != null) {
+        result.add(handlerLabel);
+        break;
+      }
+      int finallyLabel = finallyLabels[node];
+      if (finallyLabel != null) {
+        result.add(finallyLabel);
+      }
+    }
+    return result.reversed.toList();
+  }
+
+  /// See the comments of [visitFun] for more explanation.
   void visitTry(js.Try node) {
     if (!shouldTransform(node)) {
       js.Block body = translateInBlock(node.body);
@@ -1542,31 +1614,48 @@ class AsyncRewriter extends js.NodeVisitor {
       addStatement(new js.Try(body, catchPart, finallyPart));
       return;
     }
+
     hasTryBlocks = true;
-    int handlerLabel = newLabel("catch");
+    int uncaughtLabel = newLabel("uncaught");
+    int handlerLabel = (node.catchPart == null)
+        ? uncaughtLabel
+        : newLabel("catch");
+
     int finallyLabel = newLabel("finally");
     int afterFinallyLabel = newLabel("after finally");
-    errorHandlerLabels.add(handlerLabel);
+    if (node.finallyPart != null) {
+      finallyLabels[node.finallyPart] = finallyLabel;
+      targetsCatchesAndFinallies.add(node.finallyPart);
+    }
+    if (node.catchPart != null) {
+      handlerLabels[node.catchPart] = uncaughtLabel;

[floitsch, 2015/02/16 18:26:36]

This needs a comment.
Could you delay this until line 1658?

[sigurdm, 2015/02/17 10:29:05]

Comment added
Yes, it belongs better there.

+    }
+
+    handlerLabels[node] = handlerLabel;
+    targetsCatchesAndFinallies.add(node);
+
     // Set the error handler here. It must be cleared on every path out;
     // normal and error exit.
     setErrorHandler();
-    if (node.finallyPart != null) {
-      finallyLabels[node] = finallyLabel;
-      targetsAndTries.add(node);
-    }
+
     visitStatement(node.body);
-    errorHandlerLabels.removeLast();
-    addStatement(
-        js.js.statement("$nextName = [#];", [js.number(afterFinallyLabel)]));
+
+    js.Node last = targetsCatchesAndFinallies.removeLast();
+    assert(last == node);
+
     if (node.finallyPart == null) {
       setErrorHandler();
       addGoto(afterFinallyLabel);
     } else {
-      // The handler is set as the first thing in the finally block.
+      // The handler is reset as the first thing in the finally block.
+      addStatement(
+          js.js.statement("$nextName = [#];", [js.number(afterFinallyLabel)]));
       addGoto(finallyLabel);
     }
-    beginLabel(handlerLabel);
+
     if (node.catchPart != null) {
+      beginLabel(handlerLabel);
+      targetsCatchesAndFinallies.add(node.catchPart);
       setErrorHandler();
       // The catch declaration name can shadow outer variables, so a fresh name
       // is needed to avoid collisions.  See Ecma 262, 3rd edition,
@@ -1576,19 +1665,45 @@ class AsyncRewriter extends js.NodeVisitor {
       variableRenamings
           .add(new Pair(node.catchPart.declaration.name, errorRename));
       addExpressionStatement(new js.Assignment(
-          new js.VariableUse(errorRename), new js.VariableUse(resultName)));
+          new js.VariableUse(errorRename),
+          new js.VariableUse(currentErrorName)));
       visitStatement(node.catchPart.body);
       variableRenamings.removeLast();
+      if (node.finallyPart != null) {
+        // The error has been caught, so after the finally, continue after the
+        // try.
+        addStatement(js.js.statement("$nextName = [#];",
+                                     [js.number(afterFinallyLabel)]));
+        addGoto(finallyLabel);
+      } else {
+        addGoto(afterFinallyLabel);
+      }
+      js.Node last = targetsCatchesAndFinallies.removeLast();
+      assert(last == node.catchPart);
     }
-    if (node.finallyPart != null) {
-      targetsAndTries.removeLast();
-      setErrorHandler();
-      // This belongs to the catch-part, but is only needed if there is a
-      // `finally`. Therefore it is in this branch.
-      // This is needed even if there is no explicit catch-branch, because
-      // if an exception is raised the finally part has to be run.
+
+    // The "uncaught"-handler tells the finally-block to continue with
+    // the enclosing finally-blocks until the current catch-handler.
+    beginLabel(uncaughtLabel);
+
+    List<int> enclosingFinallies = _finalliesUpToAndEnclosingHandler();
+
+    int nextLabel = enclosingFinallies.removeLast();
+    if (enclosingFinallies.isNotEmpty) {
+      // enclosingFinallies can be empty if

[floitsch, 2015/02/16 18:26:36]

Unfinished comment.

[sigurdm, 2015/02/17 10:29:06]

Done.

[sigurdm, 2015/02/17 10:29:06]

Done.

       addStatement(
-          js.js.statement("$nextName = [#];", [js.number(afterFinallyLabel)]));
+          js.js.statement("$nextName = #;", new js.ArrayInitializer(
+              enclosingFinallies.map(js.number).toList())));
+    }
+    if (node.finallyPart == null) {
+      // The finally-block belonging to [node] will be visited because of
+      // fallthrough. If it does not exist, add an explicit goto.
+      addGoto(nextLabel);
+    }
+    if (node.finallyPart != null) {
+      js.Node last = targetsCatchesAndFinallies.removeLast();
+      assert(last == node.finallyPart);
+
       beginLabel(finallyLabel);
       setErrorHandler();
       visitStatement(node.finallyPart);
@@ -1658,9 +1773,9 @@ class AsyncRewriter extends js.NodeVisitor {
             new js.Prefix("!", condition), gotoAndBreak(afterLabel)));
       }, store: false);
     }
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     addGoto(continueLabel);
     beginLabel(afterLabel);
   }
@@ -1683,36 +1798,27 @@ class AsyncRewriter extends js.NodeVisitor {
   ///
   /// yield/yield* in an async* function is translated much like the `await` is
   /// translated in [visitAwait], only the object is wrapped in a
-  /// [yieldExpression]/[yieldStarExpression] to let [streamHelper] distinguish.
-  ///
-  /// Because there is no Future that can fail (as there is in await) null is
-  /// passed as the errorCallback.
+  /// [yieldExpression]/[yieldStarExpression] to let [asyncStarHelper]
+  /// distinguish them.
+  /// Also a [nextWhenCanceledName] is set up to

[floitsch, 2015/02/16 18:26:36]

Unfinished sentence.

[sigurdm, 2015/02/17 10:29:05]

Done.

   void addAsyncYield(js.DartYield node, js.Expression expression) {
     assert(isAsyncStar);
     // Find all the finally blocks that should be performed if the stream is
     // canceled during the yield.
     // At the bottom of the stack is the return label.
     List<int> enclosingFinallyLabels = <int>[exitLabel];
-    enclosingFinallyLabels.addAll(targetsAndTries
-        .where((js.Node node) => node is js.Try)
-        .map((js.Try node) => finallyLabels[node]));
-    int destinationOnCancel = enclosingFinallyLabels.removeLast();
-    js.ArrayInitializer finallyListInitializer = new js.ArrayInitializer(
-        enclosingFinallyLabels.map(js.number).toList());
+    enclosingFinallyLabels.addAll(targetsCatchesAndFinallies
+        .where((js.Node node) => finallyLabels[node] != null)
+        .map((js.Block node) => finallyLabels[node]));
+    addStatement(js.js.statement("$nextWhenCanceledName = #",
+        [new js.ArrayInitializer(enclosingFinallyLabels.map(js.number)
+            .toList())]));
     addStatement(js.js.statement("""
         return #streamHelper(#yieldExpression(#expression),
-            $helperName, $controllerName, function () {
-              if (#notEmptyFinallyList)
-                $nextName = #finallyList;
-              $gotoName = #destinationOnCancel;
-              $helperName();
-            });""", {
+            $bodyName, $controllerName);""", {
       "streamHelper": streamHelper,
       "yieldExpression": node.hasStar ? yieldStarExpression : yieldExpression,
       "expression": expression,
-      "notEmptyFinallyList": enclosingFinallyLabels.isNotEmpty,
-      "finallyList": finallyListInitializer,
-      "destinationOnCancel": js.number(destinationOnCancel)
     }));
   }