From 181efd76eeb55e2fc6014dd25f00a652da81247a Mon Sep 17 00:00:00 2001 From: "lrn@google.com" Date: Thu, 25 Sep 2014 11:00:49 +0000 Subject: [PATCH] Add missing null-tests to async error functions. Also treat null errors comming out of Zone.errorCallback as NullThrownError. This should prevent, as was always the intention, any async error from having a null value. This is important for async/await syntax, where the distinction between sync and async errors is removed. (For Dart 2.0, we could just make null throwable). R=sgjesse@google.com Review URL: https://codereview.chromium.org//598993002 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@40673 260f80e4-7a28-3924-810f-c04153c831b5 --- sdk/lib/async/broadcast_stream_controller.dart | 3 ++- sdk/lib/async/future.dart | 18 ++++++++++++------ sdk/lib/async/future_impl.dart | 5 ++--- sdk/lib/async/stream.dart | 2 +- sdk/lib/async/stream_controller.dart | 5 ++++- sdk/lib/async/stream_pipe.dart | 14 ++++++++------ sdk/lib/async/zone.dart | 7 ++++--- tests/co19/co19-co19.status | 4 +++- tests/lib/async/future_test.dart | 7 ++++++- .../async/stream_controller_async_test.dart | 2 +- 10 files changed, 43 insertions(+), 24 deletions(-) diff --git a/sdk/lib/async/broadcast_stream_controller.dart b/sdk/lib/async/broadcast_stream_controller.dart index cda7c0f960c..c2cd4ab6c1c 100644 --- a/sdk/lib/async/broadcast_stream_controller.dart +++ b/sdk/lib/async/broadcast_stream_controller.dart @@ -238,10 +238,11 @@ abstract class _BroadcastStreamController } void addError(Object error, [StackTrace stackTrace]) { + error = _nonNullError(error); if (!_mayAddEvent) throw _addEventError(); AsyncError replacement = Zone.current.errorCallback(error, stackTrace); if (replacement != null) { - error = replacement.error; + error = _nonNullError(replacement.error); stackTrace = replacement.stackTrace; } _sendError(error, stackTrace); diff --git a/sdk/lib/async/future.dart b/sdk/lib/async/future.dart index a4971be56e4..508dc07f852 100644 --- a/sdk/lib/async/future.dart +++ b/sdk/lib/async/future.dart @@ -187,13 +187,16 @@ abstract class Future { /** * A future that completes with an error in the next event-loop iteration. * - * Use [Completer] to create a Future and complete it later. + * If [error] is `null`, it is replaced by a [NullThrownError]. + * + * Use [Completer] to create a future and complete it later. */ factory Future.error(Object error, [StackTrace stackTrace]) { + error = _nonNullError(error); if (!identical(Zone.current, _ROOT_ZONE)) { AsyncError replacement = Zone.current.errorCallback(error, stackTrace); if (replacement != null) { - error = replacement.error; + error = _nonNullError(replacement.error); stackTrace = replacement.stackTrace; } } @@ -663,10 +666,13 @@ abstract class Completer { // for error replacement first. void _completeWithErrorCallback(_Future result, error, stackTrace) { AsyncError replacement = Zone.current.errorCallback(error, stackTrace); - if (replacement == null) { - result._completeError(error, stackTrace); - } else { - result._completeError(replacement.error, replacement.stackTrace); + if (replacement != null) { + error = _nonNullError(replacement.error); + stackTrace = replacement.stackTrace; } + result._completeError(error, stackTrace); } +/** Helper function that converts `null` to a [NullThrownError]. */ +Object _nonNullError(Object error) => + (error != null) ? error : new NullThrownError(); diff --git a/sdk/lib/async/future_impl.dart b/sdk/lib/async/future_impl.dart index 124b7ab8606..f3cd5e41c6b 100644 --- a/sdk/lib/async/future_impl.dart +++ b/sdk/lib/async/future_impl.dart @@ -17,11 +17,11 @@ abstract class _Completer implements Completer { void complete([value]); void completeError(Object error, [StackTrace stackTrace]) { - if (error == null) throw new ArgumentError("Error must not be null"); + error = _nonNullError(error); if (!future._mayComplete) throw new StateError("Future already completed"); AsyncError replacement = Zone.current.errorCallback(error, stackTrace); if (replacement != null) { - error = replacement.error; + error = _nonNullError(replacement.error); stackTrace = replacement.stackTrace; } _completeError(error, stackTrace); @@ -47,7 +47,6 @@ class _AsyncCompleter extends _Completer { } class _SyncCompleter extends _Completer { - void complete([value]) { if (!future._mayComplete) throw new StateError("Future already completed"); future._complete(value); diff --git a/sdk/lib/async/stream.dart b/sdk/lib/async/stream.dart index 906476b8858..ff8349a2b2b 100644 --- a/sdk/lib/async/stream.dart +++ b/sdk/lib/async/stream.dart @@ -1382,7 +1382,7 @@ abstract class EventSink implements Sink { void add(T event); /** Send an async error to a stream. */ void addError(errorEvent, [StackTrace stackTrace]); - /** Send a done event to a stream.*/ + /** Send a done event to a stream. */ void close(); } diff --git a/sdk/lib/async/stream_controller.dart b/sdk/lib/async/stream_controller.dart index fe3fbddc729..aa45876e328 100644 --- a/sdk/lib/async/stream_controller.dart +++ b/sdk/lib/async/stream_controller.dart @@ -168,6 +168,8 @@ abstract class StreamController implements StreamSink { /** * Send or enqueue an error event. * + * If [error] is `null`, it is replaced by a [NullThrownError]. + * * Also allows an objection stack trace object, on top of what [EventSink] * allows. */ @@ -414,10 +416,11 @@ abstract class _StreamController implements StreamController, * Send or enqueue an error event. */ void addError(Object error, [StackTrace stackTrace]) { + error = _nonNullError(error); if (!_mayAddEvent) throw _badEventState(); AsyncError replacement = Zone.current.errorCallback(error, stackTrace); if (replacement != null) { - error = replacement.error; + error = _nonNullError(replacement.error); stackTrace = replacement.stackTrace; } _addError(error, stackTrace); diff --git a/sdk/lib/async/stream_pipe.dart b/sdk/lib/async/stream_pipe.dart index 309a7716071..ca1164dea81 100644 --- a/sdk/lib/async/stream_pipe.dart +++ b/sdk/lib/async/stream_pipe.dart @@ -15,7 +15,9 @@ _runUserCode(userCode(), if (replacement == null) { onError(e, s); } else { - onError(replacement.error, replacement.stackTrace); + var error = _nonNullError(replacement.error); + var stackTrace = replacement.stackTrace; + onError(error, stackTrace); } } } @@ -39,7 +41,7 @@ void _cancelAndErrorWithReplacement(StreamSubscription subscription, error, StackTrace stackTrace) { AsyncError replacement = Zone.current.errorCallback(error, stackTrace); if (replacement != null) { - error = replacement.error; + error = _nonNullError(replacement.error); stackTrace = replacement.stackTrace; } _cancelAndError(subscription, future, error, stackTrace); @@ -187,11 +189,11 @@ typedef bool _Predicate(T value); void _addErrorWithReplacement(_EventSink sink, error, stackTrace) { AsyncError replacement = Zone.current.errorCallback(error, stackTrace); - if (replacement == null) { - sink._addError(error, stackTrace); - } else { - sink._addError(replacement.error, replacement.stackTrace); + if (replacement != null) { + error = _nonNullError(replacement.error); + stackTrace = replacement.stackTrace; } + sink._addError(error, stackTrace); } diff --git a/sdk/lib/async/zone.dart b/sdk/lib/async/zone.dart index 1f2993d2f54..465eeef7ee7 100644 --- a/sdk/lib/async/zone.dart +++ b/sdk/lib/async/zone.dart @@ -36,12 +36,13 @@ typedef Zone ForkHandler(Zone self, ZoneDelegate parent, Zone zone, ZoneSpecification specification, Map zoneValues); -/// Pair of error and stack trace. Returned by [Zone.errorCallback]. +/** Pair of error and stack trace. Returned by [Zone.errorCallback]. */ class AsyncError implements Error { final error; final StackTrace stackTrace; AsyncError(this.error, this.stackTrace); + String toString() => error.toString(); } @@ -254,10 +255,10 @@ abstract class Zone { // Private constructor so that it is not possible instantiate a Zone class. Zone._(); - /// The root zone that is implicitly created. + /** The root zone that is implicitly created. */ static const Zone ROOT = _ROOT_ZONE; - /// The currently running zone. + /** The currently running zone. */ static Zone _current = _ROOT_ZONE; static Zone get current => _current; diff --git a/tests/co19/co19-co19.status b/tests/co19/co19-co19.status index 7dc4b371858..f814f2b2b0b 100644 --- a/tests/co19/co19-co19.status +++ b/tests/co19/co19-co19.status @@ -17,7 +17,7 @@ LibTest/isolate/IsolateStream/contains_A02_t01: Fail # co19 issue 668 LibTest/typed_data/ByteData/buffer_A01_t01: Fail # co19 r736 bug - sent comment. # TODO(terry) re-enable the below CSS tests when Chrome 38 and (Dartium 38) are ready issue 21075 -LayoutTests/fast/css/getComputedStyle/computed-style-font_t01: Skip +LayoutTests/fast/css/getComputedStyle/computed-style-font_t01: Skip LayoutTests/fast/css/font-shorthand-from-longhands_t01: Skip Language/07_Classes/6_Constructors/1_Generative_Constructors_A01_t06: Fail, Pass, OK # co19 issue 695 @@ -29,6 +29,8 @@ WebPlatformTest/shadow-dom/elements-and-dom-objects/shadowroot-object/shadowroot [ $compiler != dartanalyzer && $compiler != dart2analyzer ] # Tests that fail on every runtime, but not on the analyzer. +LibTest/async/Future/Future.error_A01_t01: RuntimeError # co19 issue 712 +LibTest/async/Completer/completeError_A02_t01: RuntimeError # co19 issue 712 LibTest/isolate/ReceivePort/asBroadcastStream_A02_t01: Fail # co19 issue 687 LibTest/async/Stream/asBroadcastStream_A02_t01: Fail # co19 issue 687 diff --git a/tests/lib/async/future_test.dart b/tests/lib/async/future_test.dart index df33697d7b2..12cc53b92f5 100644 --- a/tests/lib/async/future_test.dart +++ b/tests/lib/async/future_test.dart @@ -677,8 +677,13 @@ void testCompleteErrorWithCustomFuture() { } void testCompleteErrorWithNull() { + asyncStart(); final completer = new Completer(); - Expect.throws(() => completer.completeError(null)); + completer.future.catchError((e) { + Expect.isTrue(e is NullThrownError); + asyncEnd(); + }); + completer.completeError(null); } void testChainedFutureValue() { diff --git a/tests/lib/async/stream_controller_async_test.dart b/tests/lib/async/stream_controller_async_test.dart index 43a7ca7a460..8d61ef96050 100644 --- a/tests/lib/async/stream_controller_async_test.dart +++ b/tests/lib/async/stream_controller_async_test.dart @@ -380,7 +380,7 @@ testRethrow() { Stream s = streamErrorTransform(c.stream, (e) { throw error; }); s.listen((_) { Expect.fail("unexpected value"); }, onError: expectAsync( (e) { Expect.identical(error, e); })); - c.addError(null); + c.addError("SOME ERROR"); c.close(); }); }