Don't propagate synchronous Future.wait errors immediately.

Don't propagate synchronous Future.wait errors immediately.

Fixes #27249.

R=lrn@google.com

Committed: https://github.com/dart-lang/sdk/commit/70c48b9a2ed6249495b69500622238b81c0bfb62

[floitsch, 2016-09-05 13:56:17]

floitsch@google.com changed reviewers:
+ lrn@google.com

[floitsch, 2016-09-05 13:57:53]

Description was changed from

==========
Don't propagate synchronous Future.wait errors immediately.
==========

to

==========
Don't propagate synchronous Future.wait errors immediately.

Fixes #27249.
==========

[Lasse Reichstein Nielsen, 2016-09-05 14:30:12]

lgtm

https://codereview.chromium.org/2311923002/diff/1/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/2311923002/diff/1/sdk/lib/async/future.dart#n...
sdk/lib/async/future.dart:329: return new Future.error(e, st);
I would say to just do:
  result._asyncCompleteError(e, st);
since that's what Future.error does internally anyway.

But I notice that Future.error also lets the zone intercept the error, which the
existing code didn't, and that's probably a good change.

[floitsch, 2016-09-06 09:48:32]

Added a test.

[Lasse Reichstein Nielsen, 2016-09-06 09:53:27]

lgtm

https://codereview.chromium.org/2311923002/diff/20001/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/2311923002/diff/20001/sdk/lib/async/future.da...
sdk/lib/async/future.dart:326: // Don't just complete the existing `_Future`
since that would propagate
Drop this comment. If you used _asyncCompleteError it wouldn't be a problem. Say
that this also gives the zone a chance to intercept the error.

https://codereview.chromium.org/2311923002/diff/20001/tests/lib/async/future_...
File tests/lib/async/future_test.dart (right):

https://codereview.chromium.org/2311923002/diff/20001/tests/lib/async/future_...
tests/lib/async/future_test.dart:910: void testWaitSyncError3() {
Add comment saying what you are testing: That zone.errorCallback intercepts the
synchronous error when it becomes an async error.

https://codereview.chromium.org/2311923002/diff/20001/tests/lib/async/future_...
tests/lib/async/future_test.dart:926: Expect.isNotNull(caughtError);
Check that caughtError is identical to e.

[floitsch, 2016-09-06 09:57:16]

https://codereview.chromium.org/2311923002/diff/20001/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/2311923002/diff/20001/sdk/lib/async/future.da...
sdk/lib/async/future.dart:326: // Don't just complete the existing `_Future`
since that would propagate
On 2016/09/06 09:53:27, Lasse Reichstein Nielsen wrote:
> Drop this comment. If you used _asyncCompleteError it wouldn't be a problem.
Say
> that this also gives the zone a chance to intercept the error.

Done.

https://codereview.chromium.org/2311923002/diff/20001/tests/lib/async/future_...
File tests/lib/async/future_test.dart (right):

https://codereview.chromium.org/2311923002/diff/20001/tests/lib/async/future_...
tests/lib/async/future_test.dart:910: void testWaitSyncError3() {
On 2016/09/06 09:53:27, Lasse Reichstein Nielsen wrote:
> Add comment saying what you are testing: That zone.errorCallback intercepts
the
> synchronous error when it becomes an async error.

Done.

https://codereview.chromium.org/2311923002/diff/20001/tests/lib/async/future_...
tests/lib/async/future_test.dart:926: Expect.isNotNull(caughtError);
On 2016/09/06 09:53:27, Lasse Reichstein Nielsen wrote:
> Check that caughtError is identical to e.

Done.

[floitsch, 2016-09-06 09:57:48]

Description was changed from

==========
Don't propagate synchronous Future.wait errors immediately.

Fixes #27249.
==========

to

==========
Don't propagate synchronous Future.wait errors immediately.

Fixes #27249.

R=lrn@google.com

Committed:
https://github.com/dart-lang/sdk/commit/70c48b9a2ed6249495b69500622238b81c0bfb62
==========

[floitsch, 2016-09-06 09:57:50]

Committed patchset #3 (id:40001) manually as
70c48b9a2ed6249495b69500622238b81c0bfb62 (presubmit successful).