Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Commit

Permalink
Revert change of visible behavior in CancelableOperation. (#208)
Browse files Browse the repository at this point in the history
* Revert change of visible behavior in `CancelableOperation`.

A prior change made it so that a `.then` on a `CancelableOperation`
would call the `onValue`/`onError` callback even if the
returned operation was cancelled beteen the time of the `then`
and the time the original operation completed.
The result would not show up in the cancelled operation,
but the callbacks would run.

This change blocks calling the callbacks if the returned operation
has been cancelled.
  • Loading branch information
lrhn authored Mar 18, 2022
1 parent 1a3ee49 commit 82094ce
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 9 deletions.
52 changes: 43 additions & 9 deletions lib/src/cancelable_operation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,50 @@ class CancelableOperation<T> {
final completer =
CancelableCompleter<R>(onCancel: propagateCancel ? cancel : null);

_completer._inner?.future
.then(onValue, onError: onError)
.then(completer.complete, onError: completer.completeError);
_completer._cancelCompleter?.future.then((_) {
if (onCancel != null) {
completer.complete(Future.sync(onCancel));
} else {
completer._cancel();
// if `_completer._inner` completes before `completer` is cancelled
// call `onValue` or `onError` with the result, and complete `completer`
// with the result of that call (unless cancelled in the meantime).
//
// If `_completer._cancelCompleter` completes (always with a value)
// before `completer` is cancelled, then call `onCancel` (if supplied)
// with that that value and complete `completer` with the result of that
// call (unless cancelled in the meantime).
//
// If any of the callbacks throw synchronously, the `completer` is
// completed with that error.
//
// If no `onCancel` is provided, and `_completer._cancelCompleter`
// completes before `completer` is cancelled,
// then cancel `cancelCompleter`. (Cancelling twice is safe.)

_completer._inner?.future.then<void>((value) {
if (completer.isCanceled) return;
try {
completer.complete(onValue(value));
} catch (error, stack) {
completer.completeError(error, stack);
}
});
},
onError: onError == null
? completer.completeError // Is ignored if already cancelled.
: (Object error, StackTrace stack) {
if (completer.isCanceled) return;
try {
completer.complete(onError(error, stack));
} catch (error2, stack2) {
completer.completeError(error2, stack2);
}
});
_completer._cancelCompleter?.future.whenComplete(onCancel == null
? completer._cancel
: () {
if (completer.isCanceled) return;
try {
completer.complete(onCancel());
} catch (error, stack) {
completer.completeError(error, stack);
}
});

return completer.operation;
}
Expand Down
39 changes: 39 additions & 0 deletions test/cancelable_operation_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,45 @@ void main() {

expect(originalCompleter.isCanceled, false);
});

test('onValue callback not called after cancel', () async {
var called = false;
onValue = expectAsync1((_) {
called = true;
fail("onValue unreachable");
}, count: 0);

await runThen().cancel();
originalCompleter.complete(0);
await flushMicrotasks();
expect(called, false);
});

test('onError callback not called after cancel', () async {
var called = false;
onError = expectAsync2((_, __) {
called = true;
fail("onError unreachable");
}, count: 0);

await runThen().cancel();
originalCompleter.completeError("Error", StackTrace.empty);
await flushMicrotasks();
expect(called, false);
});

test('onCancel callback not called after cancel', () async {
var called = false;
onCancel = expectAsync0(() {
called = true;
fail("onCancel unreachable");
}, count: 0);

await runThen().cancel();
await originalCompleter.operation.cancel();
await flushMicrotasks();
expect(called, false);
});
});
});

Expand Down

0 comments on commit 82094ce

Please sign in to comment.