-
Notifications
You must be signed in to change notification settings - Fork 388
feat: support aborting HTTP requests #1773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e1a6c30
b17468d
12ae9df
5c5f59c
e7487c3
b4ab341
88b9692
28e05aa
8563f19
bdac03d
fad438d
55fb040
1c195fb
d19e9e3
05f8ae1
7ee00e6
3914e2b
1088ec8
917d573
6b5d2e2
bfe0031
e707d2c
e02a40c
a57318a
1cc77b0
9001a1d
24025ba
3d5caa7
9da76aa
0b32a5a
b9e8543
d302777
67132f6
5ac3805
baef552
63eabca
c8a0403
5d25620
ba7f6c6
3511b38
a9255fd
8e62b5f
47944b1
0b4c3a0
dc2b4ae
a6807d5
b4d843a
b663463
f04e4fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
## 1.4.1 | ||
## 1.5.0-wip | ||
|
||
* Added support for aborting requests before they complete. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎉indeed 😂 |
||
* Clarify that some header names may not be sent/received. | ||
|
||
## 1.4.0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file | ||
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
import 'dart:async'; | ||
|
||
import 'base_request.dart'; | ||
import 'client.dart'; | ||
import 'exception.dart'; | ||
import 'streamed_response.dart'; | ||
|
||
/// An HTTP request that can be aborted before it completes. | ||
abstract mixin class Abortable implements BaseRequest { | ||
/// Completion of this future aborts this request (if the client supports | ||
/// abortion). | ||
/// | ||
/// Requests/responses may be aborted at any time during their lifecycle. | ||
/// | ||
/// * If completed before the request has been finalized and sent, | ||
/// [Client.send] completes with [RequestAbortedException]. | ||
/// * If completed after the response headers are available, or whilst | ||
/// streaming the response, clients inject [RequestAbortedException] into | ||
/// the [StreamedResponse.stream] then close the stream. | ||
/// * If completed after the response is fully complete, there is no effect. | ||
/// | ||
/// A common pattern is aborting a request when another event occurs (such as | ||
/// a user action): use a [Completer] to implement this. To implement a | ||
/// timeout (to abort the request after a set time has elapsed), use | ||
/// [Future.delayed]. | ||
/// | ||
/// This future must not complete with an error. | ||
/// | ||
/// Some clients may not support abortion, or may not support this trigger. | ||
abstract final Future<void>? abortTrigger; | ||
} | ||
|
||
/// Thrown when an HTTP request is aborted. | ||
/// | ||
/// This exception is triggered when [Abortable.abortTrigger] completes. | ||
class RequestAbortedException extends ClientException { | ||
RequestAbortedException([Uri? uri]) | ||
: super('Request aborted by `abortTrigger`', uri); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -8,12 +8,14 @@ import 'dart:js_interop'; | |||||
import 'package:web/web.dart' | ||||||
show | ||||||
AbortController, | ||||||
DOMException, | ||||||
HeadersInit, | ||||||
ReadableStreamDefaultReader, | ||||||
RequestInfo, | ||||||
RequestInit, | ||||||
Response; | ||||||
|
||||||
import 'abortable.dart'; | ||||||
import 'base_client.dart'; | ||||||
import 'base_request.dart'; | ||||||
import 'exception.dart'; | ||||||
|
@@ -49,15 +51,14 @@ external JSPromise<Response> _fetch( | |||||
/// Responses are streamed but requests are not. A request will only be sent | ||||||
/// once all the data is available. | ||||||
class BrowserClient extends BaseClient { | ||||||
final _abortController = AbortController(); | ||||||
|
||||||
/// Whether to send credentials such as cookies or authorization headers for | ||||||
/// cross-site requests. | ||||||
/// | ||||||
/// Defaults to `false`. | ||||||
bool withCredentials = false; | ||||||
|
||||||
bool _isClosed = false; | ||||||
final _openRequestAbortControllers = <AbortController>[]; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might want a type on the left ('DO type annotate fields and top-level variables if the type isn't obvious'). 'Obvious' isn't precisely defined, but adding one would also match w/ the other fields.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really don't like this style. Prefer as-is! Devon: No offence to your bad ideas. 😛 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @kevmoo here - since the type is clearly indicated on the RHS, adding it explicitly is redundant |
||||||
|
||||||
/// Sends an HTTP request and asynchronously returns the response. | ||||||
@override | ||||||
|
@@ -67,8 +68,17 @@ class BrowserClient extends BaseClient { | |||||
'HTTP request failed. Client is already closed.', request.url); | ||||||
} | ||||||
|
||||||
final abortController = AbortController(); | ||||||
_openRequestAbortControllers.add(abortController); | ||||||
|
||||||
final bodyBytes = await request.finalize().toBytes(); | ||||||
try { | ||||||
if (request case Abortable(:final abortTrigger?)) { | ||||||
// Tear-offs of external extension type interop members are disallowed | ||||||
// ignore: unnecessary_lambdas | ||||||
unawaited(abortTrigger.whenComplete(() => abortController.abort())); | ||||||
} | ||||||
|
||||||
final response = await _fetch( | ||||||
'${request.url}'.toJS, | ||||||
RequestInit( | ||||||
|
@@ -81,7 +91,7 @@ class BrowserClient extends BaseClient { | |||||
for (var header in request.headers.entries) | ||||||
header.key: header.value, | ||||||
}.jsify()! as HeadersInit, | ||||||
signal: _abortController.signal, | ||||||
signal: abortController.signal, | ||||||
redirect: request.followRedirects ? 'follow' : 'error', | ||||||
), | ||||||
).toDart; | ||||||
|
@@ -116,20 +126,28 @@ class BrowserClient extends BaseClient { | |||||
); | ||||||
} catch (e, st) { | ||||||
_rethrowAsClientException(e, st, request); | ||||||
} finally { | ||||||
_openRequestAbortControllers.remove(abortController); | ||||||
} | ||||||
} | ||||||
|
||||||
/// Closes the client. | ||||||
/// | ||||||
/// This terminates all active requests. | ||||||
/// This terminates all active requests, which may cause them to throw | ||||||
/// [RequestAbortedException] or [ClientException]. | ||||||
@override | ||||||
void close() { | ||||||
for (final abortController in _openRequestAbortControllers) { | ||||||
abortController.abort(); | ||||||
} | ||||||
_isClosed = true; | ||||||
_abortController.abort(); | ||||||
} | ||||||
} | ||||||
|
||||||
Never _rethrowAsClientException(Object e, StackTrace st, BaseRequest request) { | ||||||
if (e case DOMException(:final name) when name == 'AbortError') { | ||||||
Error.throwWithStackTrace(RequestAbortedException(request.url), st); | ||||||
} | ||||||
if (e is! ClientException) { | ||||||
var message = e.toString(); | ||||||
if (message.startsWith('TypeError: ')) { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this PR, but we could also address this by moving this repo to a workspace.