Skip to content

Commit

Permalink
Merge pull request #256 from Workiva/complete_done_on_cancel
Browse files Browse the repository at this point in the history
WP-3777 Complete `BaseRequest.done` on cancellation (BACKPATCH)
  • Loading branch information
maxwellpeterson-wf authored Mar 15, 2017
2 parents fabcb9f + 2a44cdd commit 164035e
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## [2.9.7](https://github.com/Workiva/w_transport/compare/2.9.6...2.9.7)
_March 15th, 2017_

- **Bug Fix:** When a request is canceled, it will now always result in the
`done` Future resolving. Previously it was possible for the `done` Future to
never resolve if the request was canceled at a certain point during the
request lifecycle.

## [2.9.6](https://github.com/Workiva/w_transport/compare/2.9.5...2.9.6)
_February 9th, 2017_

Expand Down
3 changes: 3 additions & 0 deletions lib/src/http/common/request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@ abstract class CommonRequest extends Object
abortRequest();
_cancellationError = error;
_cancellationCompleter.complete();
if (!_done.isCompleted) {
_done.complete();
}
}

/// Check if this request has been canceled.
Expand Down
16 changes: 14 additions & 2 deletions test/unit/http/request_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,14 @@ void _runCommonRequestSuiteFor(
() async {
BaseRequest request = requestFactory();
request.abort();
expect(request.get(uri: requestUri),
throwsA(new isInstanceOf<RequestException>()));
Future future = request.get(uri: requestUri);
expect(future, throwsA(new isInstanceOf<RequestException>()));
await future.catchError((_) {});
expect(request.isDone, isTrue,
reason: 'canceled request should be marked as "done"');
expect(request.done, completes,
reason:
'canceled request should trigger completion of `done` future');
});

test(
Expand All @@ -283,6 +289,12 @@ void _runCommonRequestSuiteFor(
await new Future.delayed(new Duration(milliseconds: 100));
request.abort();
expect(future, throwsA(new isInstanceOf<RequestException>()));
await future.catchError((_) {});
expect(request.isDone, isTrue,
reason: 'canceled request should be marked as "done"');
expect(request.done, completes,
reason:
'canceled request should trigger completion of `done` future');
});

test('request cancellation after request has succeeded should do nothing',
Expand Down

0 comments on commit 164035e

Please sign in to comment.