Deal with synchronous errors in Future.wait.

Synchronous errors are caught and piped into the returned future. This makes handling errors in Future.wait uniform.

Fixes #23656
BUG= http://dartbug.com/23656
R=lrn@google.com

Review URL: https://codereview.chromium.org/2252823004 .

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.20.0
+
+### Core library changes
+* `dart:async`
+  * `Future.wait` now catches synchronous errors and returns them in the
+    returned Future.
+
 ## 1.19.0
 
 ### Language changes

sdk/lib/async/future.dart

@@ -287,32 +287,51 @@ abstract class Future<T> {
       }
     }
 
-    // As each future completes, put its value into the corresponding
-    // position in the list of values.
-    for (Future future in futures) {
-      int pos = remaining++;
-      future.then((Object/*=T*/ value) {
-        remaining--;
-        if (values != null) {
-          values[pos] = value;
-          if (remaining == 0) {
-            result._completeWithValue(values);
+    try {
+      // As each future completes, put its value into the corresponding
+      // position in the list of values.
+      for (Future future in futures) {
+        int pos = remaining;
+        future.then((Object/*=T*/ value) {
+          remaining--;
+          if (values != null) {
+            values[pos] = value;
+            if (remaining == 0) {
+              result._completeWithValue(values);
+            }
+          } else {
+            if (cleanUp != null && value != null) {
+              // Ensure errors from cleanUp are uncaught.
+              new Future.sync(() { cleanUp(value); });
+            }
+            if (remaining == 0 && !eagerError) {
+              result._completeError(error, stackTrace);
+            }
           }
-        } else {
-          if (cleanUp != null && value != null) {
-            // Ensure errors from cleanUp are uncaught.
-            new Future.sync(() { cleanUp(value); });
-          }
-          if (remaining == 0 && !eagerError) {
-            result._completeError(error, stackTrace);
-          }
-        }
-      }, onError: handleError);
+        }, onError: handleError);
+        // Increment the 'remaining' after the call to 'then'.
+        // If that call throws, we don't expect any future callback from
+        // the future, and we also don't increment remaining.
+        remaining++;
+      }
+      if (remaining == 0) {
+        return new Future.value(const []);
+      }
+      values = new List/*<T>*/(remaining);
+    } catch (e, st) {
+      // The error must have been thrown while iterating over the futures
+      // list, or while installing a callback handler on the future.
+      if (remaining == 0 || eagerError) {
+        // Just complete the error immediately.
+        result._completeError(e, st);
+      } else {
+        // Don't allocate a list for values, thus indicating that there was an
+        // error.
+        // Set error to the caught exception.
+        error = e;
+        stackTrace = st;
+      }
     }
-    if (remaining == 0) {
-      return new Future.value(const []);
-    }
-    values = new List/*<T>*/(remaining);
     return result;
   }
 

tests/lib/async/future_test.dart

@@ -876,6 +876,26 @@ void testWaitCleanUpError() {
   });
 }
 
+void testWaitSyncError() {
+  var cms = const Duration(milliseconds: 100);
+  var cleanups = new List.filled(3, false);
+  var uncaughts = new List.filled(3, false);
+  asyncStart();
+  asyncStart();
+  runZoned(() {
+    Future.wait(new Iterable.generate(5, (i) {
+      if (i != 3) return new Future.delayed(cms * (i + 1), () => i);
+      throw "throwing synchronously in iterable";
+    }), cleanUp: (index) {
+      Expect.isFalse(cleanups[index]);
+      cleanups[index] = true;
+      if (cleanups.every((x) => x)) asyncEnd();
+    });
+  }, onError: (e, s) {
+    asyncEnd();
+  });
+}
+
 void testBadFuture() {
   var bad = new BadFuture();
   // Completing with bad future (then call throws) puts error in result.
@@ -1075,6 +1095,7 @@ main() {
 
   testWaitCleanUp();
   testWaitCleanUpError();
+  testWaitSyncError();
 
   testBadFuture();