From e1a6c30a8c50aa9509386f876649e46173ba0399 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 20 May 2025 22:45:16 +0100 Subject: [PATCH 01/47] Added `Abortable` interface for `BaseRequest`s Implemented `Abortable` interface for all available `BaseRequest`s Added abort support to `IOClient` WIP: abort support for `BrowserClient` --- pkgs/http/example/main.dart | 9 ++++++ pkgs/http/lib/http.dart | 1 + pkgs/http/lib/src/abortable.dart | 35 ++++++++++++++++++++++++ pkgs/http/lib/src/base_request.dart | 5 ++++ pkgs/http/lib/src/browser_client.dart | 8 ++++++ pkgs/http/lib/src/io_client.dart | 9 ++++++ pkgs/http/lib/src/multipart_request.dart | 13 +++++++++ pkgs/http/lib/src/request.dart | 12 ++++++++ pkgs/http/lib/src/streamed_request.dart | 13 +++++++++ 9 files changed, 105 insertions(+) create mode 100644 pkgs/http/lib/src/abortable.dart diff --git a/pkgs/http/example/main.dart b/pkgs/http/example/main.dart index 34149188ba..eab5234a60 100644 --- a/pkgs/http/example/main.dart +++ b/pkgs/http/example/main.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert' as convert; import 'package:http/http.dart' as http; @@ -9,6 +10,14 @@ void main(List arguments) async { Uri.https('www.googleapis.com', '/books/v1/volumes', {'q': '{http}'}); // Await the http get response, then decode the json-formatted response. + try { + final e = + await http.AbortableRequest('GET', url, abortTrigger: Future.error(10)) + .send(); + await e.stream.drain(); + } on http.AbortedRequest { + print('Cancelled'); + } var response = await http.get(url); if (response.statusCode == 200) { var jsonResponse = diff --git a/pkgs/http/lib/http.dart b/pkgs/http/lib/http.dart index 317a2c17f3..42561cbc88 100644 --- a/pkgs/http/lib/http.dart +++ b/pkgs/http/lib/http.dart @@ -19,6 +19,7 @@ export 'src/base_request.dart'; export 'src/base_response.dart' show BaseResponse, BaseResponseWithUrl, HeadersWithSplitValues; export 'src/byte_stream.dart'; +export 'src/abortable.dart'; export 'src/client.dart' hide zoneClient; export 'src/exception.dart'; export 'src/multipart_file.dart'; diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart new file mode 100644 index 0000000000..351b521c58 --- /dev/null +++ b/pkgs/http/lib/src/abortable.dart @@ -0,0 +1,35 @@ +// Copyright (c) 2013, 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'; + +/// Enables a request to be recognised by a [Client] as abortable +abstract mixin class Abortable implements BaseRequest { + /// This request will be aborted if this completes + /// + /// A common pattern is aborting a request when another event occurs (such as + /// a user action). A [Completer] may be used to implement this. + /// + /// Another pattern is a timeout. Use [Future.delayed] to achieve this. + /// + /// This future must not complete to an error. + /// + /// This future may complete at any time - a [AbortedRequest] will be thrown + /// by [send]/[Client.send] if it is completed before the request is complete. + /// + /// Non-'package:http' [Client]s may unexpectedly not support this trigger. + abstract final Future? abortTrigger; +} + +/// Thrown when a request is aborted using [Abortable.abortTrigger] +/// +/// This is not thrown when `abortTrigger` is completed after the request has +/// already completed. +class AbortedRequest implements Exception { + /// Indicator that the request has been aborted by [Abortable.abortTrigger] + const AbortedRequest(); +} diff --git a/pkgs/http/lib/src/base_request.dart b/pkgs/http/lib/src/base_request.dart index d718e38e8f..9732b0c750 100644 --- a/pkgs/http/lib/src/base_request.dart +++ b/pkgs/http/lib/src/base_request.dart @@ -7,6 +7,7 @@ import 'dart:collection'; import 'package:meta/meta.dart'; import '../http.dart' show ClientException, get; +import 'abortable.dart'; import 'base_client.dart'; import 'base_response.dart'; import 'byte_stream.dart'; @@ -20,6 +21,10 @@ import 'utils.dart'; /// [BaseClient.send], which allows the user to provide fine-grained control /// over the request properties. However, usually it's easier to use convenience /// methods like [get] or [BaseClient.get]. +/// +/// Subclasses/implementers should mixin/implement [Abortable] to support +/// abortion of requests. A future breaking version of 'package:http' will +/// merge [Abortable] into [BaseRequest], making it a requirement. abstract class BaseRequest { /// The HTTP method of the request. /// diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index acf233448a..e325a7a54d 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -14,6 +14,7 @@ import 'package:web/web.dart' RequestInit, Response; +import 'abortable.dart'; import 'base_client.dart'; import 'base_request.dart'; import 'exception.dart'; @@ -69,6 +70,11 @@ class BrowserClient extends BaseClient { final bodyBytes = await request.finalize().toBytes(); try { + Future? canceller; + if (request case Abortable(:final Future abortTrigger)) { + canceller = abortTrigger.whenComplete(() => _abortController.abort()); + } + final response = await _fetch( '${request.url}'.toJS, RequestInit( @@ -86,6 +92,8 @@ class BrowserClient extends BaseClient { ), ).toDart; + canceller?.ignore(); + final contentLengthHeader = response.headers.get('content-length'); final contentLength = contentLengthHeader != null diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index fe4834bb24..a40b485115 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -4,6 +4,7 @@ import 'dart:io'; +import 'abortable.dart'; import 'base_client.dart'; import 'base_request.dart'; import 'base_response.dart'; @@ -123,8 +124,16 @@ class IOClient extends BaseClient { ioRequest.headers.set(name, value); }); + Future? canceller; + if (request case Abortable(:final Future abortTrigger)) { + canceller = abortTrigger + .whenComplete(() => ioRequest.abort(const AbortedRequest())); + } + var response = await stream.pipe(ioRequest) as HttpClientResponse; + canceller?.ignore(); + var headers = {}; response.headers.forEach((key, values) { // TODO: Remove trimRight() when diff --git a/pkgs/http/lib/src/multipart_request.dart b/pkgs/http/lib/src/multipart_request.dart index 79525421fb..279628576f 100644 --- a/pkgs/http/lib/src/multipart_request.dart +++ b/pkgs/http/lib/src/multipart_request.dart @@ -5,6 +5,7 @@ import 'dart:convert'; import 'dart:math'; +import 'abortable.dart'; import 'base_request.dart'; import 'boundary_characters.dart'; import 'byte_stream.dart'; @@ -160,3 +161,15 @@ class MultipartRequest extends BaseRequest { return '$prefix${String.fromCharCodes(list)}'; } } + +/// A [MultipartRequest] which supports abortion using [abortTrigger] +/// +/// A future breaking version of 'package:http' will merge this into +/// [MultipartRequest], making it a requirement. +final class AbortableMultipartRequest extends MultipartRequest with Abortable { + AbortableMultipartRequest(super.method, super.url, {this.abortTrigger}) + : super(); + + @override + final Future? abortTrigger; +} diff --git a/pkgs/http/lib/src/request.dart b/pkgs/http/lib/src/request.dart index c15e55169d..ad13794bc4 100644 --- a/pkgs/http/lib/src/request.dart +++ b/pkgs/http/lib/src/request.dart @@ -7,6 +7,7 @@ import 'dart:typed_data'; import 'package:http_parser/http_parser.dart'; +import 'abortable.dart'; import 'base_request.dart'; import 'byte_stream.dart'; import 'utils.dart'; @@ -182,3 +183,14 @@ class Request extends BaseRequest { throw StateError("Can't modify a finalized Request."); } } + +/// A [Request] which supports abortion using [abortTrigger] +/// +/// A future breaking version of 'package:http' will merge this into [Request], +/// making it a requirement. +final class AbortableRequest extends Request with Abortable { + AbortableRequest(super.method, super.url, {this.abortTrigger}) : super(); + + @override + final Future? abortTrigger; +} diff --git a/pkgs/http/lib/src/streamed_request.dart b/pkgs/http/lib/src/streamed_request.dart index d10386e263..bca35fb3f1 100644 --- a/pkgs/http/lib/src/streamed_request.dart +++ b/pkgs/http/lib/src/streamed_request.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'abortable.dart'; import 'base_client.dart'; import 'base_request.dart'; import 'byte_stream.dart'; @@ -53,3 +54,15 @@ class StreamedRequest extends BaseRequest { return ByteStream(_controller.stream); } } + +/// A [StreamedRequest] which supports abortion using [abortTrigger] +/// +/// A future breaking version of 'package:http' will merge this into +/// [StreamedRequest], making it a requirement. +final class AbortableStreamedRequest extends StreamedRequest with Abortable { + AbortableStreamedRequest(super.method, super.url, {this.abortTrigger}) + : super(); + + @override + final Future? abortTrigger; +} From b17468dd9f76444b46d69a57bd248173537fc369 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 20 May 2025 22:56:29 +0100 Subject: [PATCH 02/47] Minor improvement --- pkgs/http/lib/src/browser_client.dart | 2 +- pkgs/http/lib/src/io_client.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index e325a7a54d..a72e8864c7 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -71,7 +71,7 @@ class BrowserClient extends BaseClient { final bodyBytes = await request.finalize().toBytes(); try { Future? canceller; - if (request case Abortable(:final Future abortTrigger)) { + if (request case Abortable(:final abortTrigger?)) { canceller = abortTrigger.whenComplete(() => _abortController.abort()); } diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index a40b485115..910417ced4 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -125,7 +125,7 @@ class IOClient extends BaseClient { }); Future? canceller; - if (request case Abortable(:final Future abortTrigger)) { + if (request case Abortable(:final abortTrigger?)) { canceller = abortTrigger .whenComplete(() => ioRequest.abort(const AbortedRequest())); } From 12ae9df8ad63634b0e189922c82fcec7ed6afb9a Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 21 May 2025 13:16:36 +0100 Subject: [PATCH 03/47] Added support to `RetryClient` (conversion of `BaseRequest` to `StreamedRequest` respects `Abortable` presence) --- pkgs/http/lib/retry.dart | 13 ++++++++++++- pkgs/http/lib/src/abortable.dart | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/retry.dart b/pkgs/http/lib/retry.dart index dedba9a9e7..fccbc6a69d 100644 --- a/pkgs/http/lib/retry.dart +++ b/pkgs/http/lib/retry.dart @@ -133,7 +133,18 @@ final class RetryClient extends BaseClient { /// Returns a copy of [original] with the given [body]. StreamedRequest _copyRequest(BaseRequest original, Stream> body) { - final request = StreamedRequest(original.method, original.url) + final StreamedRequest request; + if (original case Abortable(:final abortTrigger?)) { + request = AbortableStreamedRequest( + original.method, + original.url, + abortTrigger: abortTrigger, + ); + } else { + request = StreamedRequest(original.method, original.url); + } + + request ..contentLength = original.contentLength ..followRedirects = original.followRedirects ..headers.addAll(original.headers) diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index 351b521c58..20fc768880 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -25,7 +25,7 @@ abstract mixin class Abortable implements BaseRequest { abstract final Future? abortTrigger; } -/// Thrown when a request is aborted using [Abortable.abortTrigger] +/// Thrown when a HTTP request is aborted using [Abortable.abortTrigger] /// /// This is not thrown when `abortTrigger` is completed after the request has /// already completed. From 5c5f59c215d4bac42da6c0075c2d8d7b3c256437 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 21 May 2025 13:50:19 +0100 Subject: [PATCH 04/47] Added support to `BrowserClient` Improved documentation --- pkgs/http/lib/src/abortable.dart | 11 +++++++---- pkgs/http/lib/src/browser_client.dart | 21 +++++++++++++++++---- pkgs/http/lib/src/io_client.dart | 5 +++-- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index 20fc768880..2050a3111d 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'base_request.dart'; import 'client.dart'; +import 'exception.dart'; /// Enables a request to be recognised by a [Client] as abortable abstract mixin class Abortable implements BaseRequest { @@ -25,11 +26,13 @@ abstract mixin class Abortable implements BaseRequest { abstract final Future? abortTrigger; } -/// Thrown when a HTTP request is aborted using [Abortable.abortTrigger] +/// Thrown when a HTTP request is aborted /// -/// This is not thrown when `abortTrigger` is completed after the request has -/// already completed. +/// Usually, this is due to [Abortable.abortTrigger] completing before the +/// request is already complete. However, some clients' [Client.close] +/// implementation may cause open requests to throw this (or a standard +/// [ClientException]). class AbortedRequest implements Exception { - /// Indicator that the request has been aborted by [Abortable.abortTrigger] + /// Indicator that the request has been aborted const AbortedRequest(); } diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index a72e8864c7..82a16d5ac3 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -8,6 +8,7 @@ import 'dart:js_interop'; import 'package:web/web.dart' show AbortController, + DOMException, HeadersInit, ReadableStreamDefaultReader, RequestInfo, @@ -50,8 +51,6 @@ external JSPromise _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. /// @@ -59,6 +58,7 @@ class BrowserClient extends BaseClient { bool withCredentials = false; bool _isClosed = false; + var _abortController = AbortController(); /// Sends an HTTP request and asynchronously returns the response. @override @@ -72,7 +72,13 @@ class BrowserClient extends BaseClient { try { Future? canceller; if (request case Abortable(:final abortTrigger?)) { - canceller = abortTrigger.whenComplete(() => _abortController.abort()); + canceller = abortTrigger.whenComplete(() { + _abortController.abort(); + // We have to reinstantiate the controller on every abort, otherwise + // all future requests with this client are also aborted. + // We cannot do this within `send`, as `close` requires access. + _abortController = AbortController(); + }); } final response = await _fetch( @@ -122,6 +128,12 @@ class BrowserClient extends BaseClient { url: Uri.parse(response.url), reasonPhrase: response.statusText, ); + } on DOMException catch (e, st) { + // We need to catch the abortion here, not around the `_fetch` method, as + // `_readBody` can also throw + if (e.name == 'AbortError') throw AbortedRequest(); + + _rethrowAsClientException(e, st, request); } catch (e, st) { _rethrowAsClientException(e, st, request); } @@ -129,7 +141,8 @@ class BrowserClient extends BaseClient { /// Closes the client. /// - /// This terminates all active requests. + /// This terminates all active requests, which may cause them to throw + /// [AbortedRequest] or [ClientException]. @override void close() { _isClosed = true; diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 910417ced4..8e9bf305f1 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -168,8 +168,9 @@ class IOClient extends BaseClient { /// Closes the client. /// - /// Terminates all active connections. If a client remains unclosed, the Dart - /// process may not terminate. + /// Terminates all active connections, which may cause them to throw + /// [AbortedRequest] or [ClientException]/[SocketException]. If a client + /// remains unclosed, the Dart process may not terminate. @override void close() { if (_inner != null) { From e7487c3b2d983512977e268d270d29d4326b6666 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 21 May 2025 13:51:38 +0100 Subject: [PATCH 05/47] Revert pkgs/http/example/main.dart --- pkgs/http/example/main.dart | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkgs/http/example/main.dart b/pkgs/http/example/main.dart index eab5234a60..34149188ba 100644 --- a/pkgs/http/example/main.dart +++ b/pkgs/http/example/main.dart @@ -1,4 +1,3 @@ -import 'dart:async'; import 'dart:convert' as convert; import 'package:http/http.dart' as http; @@ -10,14 +9,6 @@ void main(List arguments) async { Uri.https('www.googleapis.com', '/books/v1/volumes', {'q': '{http}'}); // Await the http get response, then decode the json-formatted response. - try { - final e = - await http.AbortableRequest('GET', url, abortTrigger: Future.error(10)) - .send(); - await e.stream.drain(); - } on http.AbortedRequest { - print('Cancelled'); - } var response = await http.get(url); if (response.statusCode == 200) { var jsonResponse = From b4ab3410f4ec8b2a34210d02a4322e323e220580 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 21 May 2025 14:22:22 +0100 Subject: [PATCH 06/47] Fixed bugs in `BrowserClient` --- pkgs/http/lib/src/browser_client.dart | 31 ++++++++++++++++----------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index 82a16d5ac3..d4fa5086dc 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -58,7 +58,7 @@ class BrowserClient extends BaseClient { bool withCredentials = false; bool _isClosed = false; - var _abortController = AbortController(); + final _openRequestAbortControllers = []; /// Sends an HTTP request and asynchronously returns the response. @override @@ -68,17 +68,14 @@ 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 { Future? canceller; if (request case Abortable(:final abortTrigger?)) { - canceller = abortTrigger.whenComplete(() { - _abortController.abort(); - // We have to reinstantiate the controller on every abort, otherwise - // all future requests with this client are also aborted. - // We cannot do this within `send`, as `close` requires access. - _abortController = AbortController(); - }); + canceller = abortTrigger.whenComplete(() => _abortController.abort()); } final response = await _fetch( @@ -129,13 +126,15 @@ class BrowserClient extends BaseClient { reasonPhrase: response.statusText, ); } on DOMException catch (e, st) { - // We need to catch the abortion here, not around the `_fetch` method, as - // `_readBody` can also throw - if (e.name == 'AbortError') throw AbortedRequest(); + if (e.name == 'AbortError') { + Error.throwWithStackTrace(AbortedRequest(), st); + } _rethrowAsClientException(e, st, request); } catch (e, st) { _rethrowAsClientException(e, st, request); + } finally { + _openRequestAbortControllers.remove(_abortController); } } @@ -145,8 +144,10 @@ class BrowserClient extends BaseClient { /// [AbortedRequest] or [ClientException]. @override void close() { + for (final abortController in _openRequestAbortControllers) { + abortController.abort(); + } _isClosed = true; - _abortController.abort(); } } @@ -179,6 +180,12 @@ Stream> _readBody(BaseRequest request, Response response) async* { } yield (chunk.value! as JSUint8Array).toDart; } + } on DOMException catch (e, st) { + isError = true; + + if (e.name == 'AbortError') Error.throwWithStackTrace(AbortedRequest(), st); + + _rethrowAsClientException(e, st, request); } catch (e, st) { isError = true; _rethrowAsClientException(e, st, request); From 88b96920d3fb81e3b0bf3808c0647812032a6ed0 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Thu, 22 May 2025 21:41:23 +0100 Subject: [PATCH 07/47] Add `const` --- pkgs/http/lib/src/browser_client.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index d4fa5086dc..feb98ec299 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -127,7 +127,7 @@ class BrowserClient extends BaseClient { ); } on DOMException catch (e, st) { if (e.name == 'AbortError') { - Error.throwWithStackTrace(AbortedRequest(), st); + Error.throwWithStackTrace(const AbortedRequest(), st); } _rethrowAsClientException(e, st, request); @@ -183,7 +183,9 @@ Stream> _readBody(BaseRequest request, Response response) async* { } on DOMException catch (e, st) { isError = true; - if (e.name == 'AbortError') Error.throwWithStackTrace(AbortedRequest(), st); + if (e.name == 'AbortError') { + Error.throwWithStackTrace(const AbortedRequest(), st); + } _rethrowAsClientException(e, st, request); } catch (e, st) { From 28e05aa82a8971c279b2c85e26b0576b73b57246 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 22 May 2025 12:55:49 -0700 Subject: [PATCH 08/47] Conformance tests for aborting requests --- .../lib/http_client_conformance_tests.dart | 12 +- .../lib/src/abort_server.dart | 43 +++++++ .../lib/src/abort_server_vm.dart | 14 +++ .../lib/src/abort_server_web.dart | 14 +++ .../lib/src/abort_tests.dart | 110 ++++++++++++++++++ 5 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 pkgs/http_client_conformance_tests/lib/src/abort_server.dart create mode 100644 pkgs/http_client_conformance_tests/lib/src/abort_server_vm.dart create mode 100644 pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart create mode 100644 pkgs/http_client_conformance_tests/lib/src/abort_tests.dart diff --git a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart index 0686c9d09c..12884f25e7 100644 --- a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart @@ -4,6 +4,7 @@ import 'package:http/http.dart'; +import 'src/abort_tests.dart'; import 'src/close_tests.dart'; import 'src/compressed_response_body_tests.dart'; import 'src/isolate_test.dart'; @@ -22,6 +23,7 @@ import 'src/response_headers_tests.dart'; import 'src/response_status_line_tests.dart'; import 'src/server_errors_test.dart'; +export 'src/abort_tests.dart' show testAbort; export 'src/close_tests.dart' show testClose; export 'src/compressed_response_body_tests.dart' show testCompressedResponseBody; @@ -49,7 +51,7 @@ export 'src/server_errors_test.dart' show testServerErrors; // /// If [canStreamResponseBody] is `false` then tests that assume that the /// [Client] supports receiving HTTP responses with unbounded body sizes will -/// be skipped +/// be skipped. /// /// If [redirectAlwaysAllowed] is `true` then tests that require the [Client] /// to limit redirects will be skipped. @@ -75,6 +77,8 @@ export 'src/server_errors_test.dart' show testServerErrors; /// If [supportsMultipartRequest] is `false` then tests that assume that /// multipart requests can be sent will be skipped. /// +/// If [supportsAbort] is `false` then tests that assume that requests can be +/// aborted will be skipped. /// The tests are run against a series of HTTP servers that are started by the /// tests. If the tests are run in the browser, then the test servers are /// started in another process. Otherwise, the test servers are run in-process. @@ -90,6 +94,8 @@ void testAll( bool canSendCookieHeaders = false, bool canReceiveSetCookieHeaders = false, bool supportsMultipartRequest = true, + // TODO: make this false, for now true to see what breaks. + bool supportsAbort = true, }) { testRequestBody(clientFactory()); testRequestBodyStreamed(clientFactory(), @@ -116,4 +122,8 @@ void testAll( canSendCookieHeaders: canSendCookieHeaders); testResponseCookies(clientFactory(), canReceiveSetCookieHeaders: canReceiveSetCookieHeaders); + testAbort(clientFactory(), + supportsAbort: supportsAbort, + canStreamRequestBody: canStreamRequestBody, + canStreamResponseBody: canStreamResponseBody); } diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_server.dart b/pkgs/http_client_conformance_tests/lib/src/abort_server.dart new file mode 100644 index 0000000000..c66454e18a --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/abort_server.dart @@ -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 'dart:io'; + +import 'package:async/async.dart'; +import 'package:stream_channel/stream_channel.dart'; + +/// Starts an HTTP server that sends a stream of integers. +/// +/// Channel protocol: +/// On Startup: +/// - send port +/// When Receive Anything: +/// - close current request +/// - exit server +void hybridMain(StreamChannel channel) async { + final channelQueue = StreamQueue(channel.stream); + + late HttpServer server; + server = (await HttpServer.bind('localhost', 0)) + ..listen((request) async { + // TODO: might have to ignore exceptions in the server because it will + // probably be disconnected + + await request.drain(); + request.response.headers.set('Access-Control-Allow-Origin', '*'); + request.response.headers.set('Content-Type', 'text/plain'); + + for (var i = 0; i < 10000; ++i) { + request.response.write('$i\n'); + await request.response.flush(); + // Let the event loop run. + await Future.delayed(const Duration()); + } + await request.response.close(); + }); + + channel.sink.add(server.port); + unawaited(channelQueue.next.then((value) => unawaited(server.close()))); +} diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_server_vm.dart b/pkgs/http_client_conformance_tests/lib/src/abort_server_vm.dart new file mode 100644 index 0000000000..eddcd8a671 --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/abort_server_vm.dart @@ -0,0 +1,14 @@ +// Generated by generate_server_wrappers.dart. Do not edit. + +import 'package:stream_channel/stream_channel.dart'; + +import 'abort_server.dart'; + +export 'server_queue_helpers.dart' show StreamQueueOfNullableObjectExtension; + +/// Starts the redirect test HTTP server in the same process. +Future> startServer() async { + final controller = StreamChannelController(sync: true); + hybridMain(controller.foreign); + return controller.local; +} diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart b/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart new file mode 100644 index 0000000000..0b3520def8 --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart @@ -0,0 +1,14 @@ +// Generated by generate_server_wrappers.dart. Do not edit. + +import 'package:stream_channel/stream_channel.dart'; +import 'package:test/test.dart'; + +export 'server_queue_helpers.dart' show StreamQueueOfNullableObjectExtension; + +/// Starts the redirect test HTTP server out-of-process. +Future> startServer() async => spawnHybridUri( + Uri( + scheme: 'package', + path: 'http_client_conformance_tests/src/abort_server.dart', + ), +); diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart new file mode 100644 index 0000000000..59c6ff2964 --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -0,0 +1,110 @@ +// 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 'package:async/async.dart'; +import 'package:http/http.dart'; +import 'package:stream_channel/stream_channel.dart'; +import 'package:test/test.dart'; + +import 'abort_server_vm.dart' + if (dart.library.js_interop) 'abort_server_web.dart'; + +/// Tests that the client supports aborting requests. +/// +/// If [supportsAbort] is `false` then tests that assume that requests can be +/// aborted will be skipped. +/// +/// If [canStreamResponseBody] is `false` then tests that assume that the +/// [Client] supports receiving HTTP responses with unbounded body sizes will +/// be skipped. +/// +/// If [canStreamRequestBody] is `false` then tests that assume that the +/// [Client] supports sending HTTP requests with unbounded body sizes will be +/// skipped. +void testAbort( + Client client, { + bool supportsAbort = true, + bool canStreamRequestBody = true, + bool canStreamResponseBody = true, +}) { + group('abort', () { + late String host; + late StreamChannel httpServerChannel; + late StreamQueue httpServerQueue; + late Uri serverUrl; + + setUp(() async { + httpServerChannel = await startServer(); + httpServerQueue = StreamQueue(httpServerChannel.stream); + host = 'localhost:${await httpServerQueue.nextAsInt}'; + serverUrl = Uri.http(host, ''); + }); + tearDownAll(() => httpServerChannel.sink.add(null)); + + test('before request', () async { + final request = Request('GET', serverUrl); + + // TODO: Trigger abort + + expect( + client.send(request), + throwsA( + isA().having((e) => e.uri, 'uri', serverUrl))); + }); + + test('during request stream', () async { + final request = StreamedRequest('POST', serverUrl); + + final response = client.send(request); + request.sink.add('Hello World'.codeUnits); + // TODO: Trigger abort + + expect( + response, + throwsA( + isA().having((e) => e.uri, 'uri', serverUrl))); + await request + .sink.done; // Verify that the stream subscription was cancelled. + }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); + + test('after response', () async { + final request = Request('GET', serverUrl); + + final response = await client.send(request); + + // TODO: Trigger abort + + expect( + response.stream.single, + throwsA( + isA().having((e) => e.uri, 'uri', serverUrl))); + }); + + test('while streaming response', () async { + final request = Request('GET', serverUrl); + + final response = await client.send(request); + + var i = 0; + expect( + response.stream.listen((data) { + ++i; + if (i == 1000) { + // TODO: Trigger abort + } + }).asFuture(), + throwsA( + isA().having((e) => e.uri, 'uri', serverUrl))); + expect(i, 1000); + }); + + test('after streaming response', () async { + final request = Request('GET', serverUrl); + + final response = await client.send(request); + await response.stream.drain(); + // Trigger abort, should have no effect. + }); + }); +} From 8563f1938b3d268ab0668e3d6273182af0ae18b3 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 22 May 2025 13:03:14 -0700 Subject: [PATCH 09/47] Update abort_tests.dart --- .../lib/src/abort_tests.dart | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 59c6ff2964..be6d828b66 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -97,7 +97,7 @@ void testAbort( throwsA( isA().having((e) => e.uri, 'uri', serverUrl))); expect(i, 1000); - }); + }, skip: canStreamResponseBody ? false : 'does not stream response bodies'); test('after streaming response', () async { final request = Request('GET', serverUrl); @@ -106,5 +106,19 @@ void testAbort( await response.stream.drain(); // Trigger abort, should have no effect. }); + + test('after response, client still useable', () async { + final request = Request('GET', serverUrl); + + final abortResponse = await client.send(request); + // TODO: Trigger abort + try { + await abortResponse.stream.drain(); + } on ClientException {} + + final response = await client.get(serverUrl); + expect(response.statusCode, 200); + expect(response.body, endsWith('10000\n')); + }); }); } From bdac03dd5eeddfd61db538cce8607b62d3435d0c Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sat, 24 May 2025 13:30:44 +0100 Subject: [PATCH 10/47] Make `IOClient` emit an error on the response stream when aborted Integrated abort tests --- pkgs/http/lib/src/io_client.dart | 73 +++++++++---- .../lib/src/abort_tests.dart | 103 ++++++++++++------ .../pubspec.yaml | 7 +- 3 files changed, 127 insertions(+), 56 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 8e9bf305f1..c3fb46ae49 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -2,6 +2,7 @@ // 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 'dart:io'; import 'abortable.dart'; @@ -124,15 +125,45 @@ class IOClient extends BaseClient { ioRequest.headers.set(name, value); }); - Future? canceller; + // We can only abort the actual connection up until the point we obtain + // the response. + // After that point, the full response bytes are always available. + // However, we instead inject an error into the response stream to match + // the behaviour of `BrowserClient`. + + StreamSubscription>? subscription; + final controller = StreamController>(sync: true); + if (request case Abortable(:final abortTrigger?)) { - canceller = abortTrigger - .whenComplete(() => ioRequest.abort(const AbortedRequest())); + abortTrigger.whenComplete(() async { + if (subscription == null) { + ioRequest.abort(const AbortedRequest()); + } else { + if (!controller.isClosed) { + controller.addError(const AbortedRequest()); + } + await subscription.cancel(); + } + await controller.close(); + }); } - var response = await stream.pipe(ioRequest) as HttpClientResponse; - - canceller?.ignore(); + final response = await stream.pipe(ioRequest) as HttpClientResponse; + + subscription = response.listen( + controller.add, + onDone: controller.close, + onError: (Object err, StackTrace stackTrace) { + if (err is HttpException) { + controller.addError( + ClientException(err.message, err.uri), + stackTrace, + ); + } else { + controller.addError(err, stackTrace); + } + }, + ); var headers = {}; response.headers.forEach((key, values) { @@ -143,22 +174,20 @@ class IOClient extends BaseClient { }); return _IOStreamedResponseV2( - response.handleError((Object error) { - final httpException = error as HttpException; - throw ClientException(httpException.message, httpException.uri); - }, test: (error) => error is HttpException), - response.statusCode, - contentLength: - response.contentLength == -1 ? null : response.contentLength, - request: request, - headers: headers, - isRedirect: response.isRedirect, - url: response.redirects.isNotEmpty - ? response.redirects.last.location - : request.url, - persistentConnection: response.persistentConnection, - reasonPhrase: response.reasonPhrase, - inner: response); + controller.stream, + response.statusCode, + contentLength: + response.contentLength == -1 ? null : response.contentLength, + request: request, + headers: headers, + isRedirect: response.isRedirect, + url: response.redirects.isNotEmpty + ? response.redirects.last.location + : request.url, + persistentConnection: response.persistentConnection, + reasonPhrase: response.reasonPhrase, + inner: response, + ); } on SocketException catch (error) { throw _ClientSocketException(error, request.url); } on HttpException catch (error) { diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index be6d828b66..6c18572b96 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -2,6 +2,8 @@ // 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 'package:async/async.dart'; import 'package:http/http.dart'; import 'package:stream_channel/stream_channel.dart'; @@ -43,82 +45,119 @@ void testAbort( tearDownAll(() => httpServerChannel.sink.add(null)); test('before request', () async { - final request = Request('GET', serverUrl); - - // TODO: Trigger abort + final abortTrigger = Completer(); + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); + abortTrigger.complete(); expect( - client.send(request), - throwsA( - isA().having((e) => e.uri, 'uri', serverUrl))); + client.send(request), + throwsA(isA()), + ); }); test('during request stream', () async { - final request = StreamedRequest('POST', serverUrl); + final abortTrigger = Completer(); + + final request = AbortableStreamedRequest( + 'POST', + serverUrl, + abortTrigger: abortTrigger.future, + ); final response = client.send(request); request.sink.add('Hello World'.codeUnits); - // TODO: Trigger abort + + abortTrigger.complete(); expect( - response, - throwsA( - isA().having((e) => e.uri, 'uri', serverUrl))); + response, + throwsA(isA()), + ); await request .sink.done; // Verify that the stream subscription was cancelled. }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); test('after response', () async { - final request = Request('GET', serverUrl); + final abortTrigger = Completer(); + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); final response = await client.send(request); - // TODO: Trigger abort + abortTrigger.complete(); expect( - response.stream.single, - throwsA( - isA().having((e) => e.uri, 'uri', serverUrl))); + response.stream.single, + throwsA(isA()), + ); }); test('while streaming response', () async { - final request = Request('GET', serverUrl); + final abortTrigger = Completer(); + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); final response = await client.send(request); var i = 0; - expect( - response.stream.listen((data) { - ++i; - if (i == 1000) { - // TODO: Trigger abort - } - }).asFuture(), - throwsA( - isA().having((e) => e.uri, 'uri', serverUrl))); + final subscription = response.stream.listen((data) { + ++i; + if (i == 1000) abortTrigger.complete(); + }).asFuture(); + expect(subscription, throwsA(isA())); + await subscription.catchError((_) => null); expect(i, 1000); }, skip: canStreamResponseBody ? false : 'does not stream response bodies'); test('after streaming response', () async { - final request = Request('GET', serverUrl); + final abortTrigger = Completer(); + + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); final response = await client.send(request); await response.stream.drain(); - // Trigger abort, should have no effect. + + abortTrigger.complete(); }); test('after response, client still useable', () async { - final request = Request('GET', serverUrl); + final abortTrigger = Completer(); + + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); final abortResponse = await client.send(request); - // TODO: Trigger abort + + abortTrigger.complete(); + + bool triggeredAbortedRequest = false; try { await abortResponse.stream.drain(); - } on ClientException {} + } on AbortedRequest { + triggeredAbortedRequest = true; + } final response = await client.get(serverUrl); expect(response.statusCode, 200); - expect(response.body, endsWith('10000\n')); + expect(response.body, endsWith('9999\n')); + expect(triggeredAbortedRequest, true); }); }); } diff --git a/pkgs/http_client_conformance_tests/pubspec.yaml b/pkgs/http_client_conformance_tests/pubspec.yaml index 2d8d652300..dd566b8798 100644 --- a/pkgs/http_client_conformance_tests/pubspec.yaml +++ b/pkgs/http_client_conformance_tests/pubspec.yaml @@ -3,7 +3,6 @@ description: >- A library that tests whether implementations of package:http's `Client` class behave as expected. repository: https://github.com/dart-lang/http/tree/master/pkgs/http_client_conformance_tests - publish_to: none environment: @@ -11,10 +10,14 @@ environment: dependencies: async: ^2.8.2 - dart_style: '>=2.3.7 <4.0.0' + dart_style: ">=2.3.7 <4.0.0" http: ^1.2.0 stream_channel: ^2.1.1 test: ^1.21.2 +dependency_overrides: + http: + path: ../http + dev_dependencies: dart_flutter_team_lints: ^3.0.0 From fad438db290bbc3e1f7e1f7828968365998b622d Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 25 May 2025 13:16:11 +0100 Subject: [PATCH 11/47] Fix tests --- .../lib/src/abort_tests.dart | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 6c18572b96..0d5c949306 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -109,14 +109,19 @@ void testAbort( ); final response = await client.send(request); + // We want to count data bytes, not 'packet' count, because different + // clients will use different size/numbers of 'packet's var i = 0; - final subscription = response.stream.listen((data) { - ++i; - if (i == 1000) abortTrigger.complete(); - }).asFuture(); - expect(subscription, throwsA(isA())); - await subscription.catchError((_) => null); - expect(i, 1000); + expect( + response.stream.listen( + (data) { + i += data.length; + if (i >= 1000 && !abortTrigger.isCompleted) abortTrigger.complete(); + }, + ).asFuture(), + throwsA(isA()), + ); + expect(i, lessThan(48890)); }, skip: canStreamResponseBody ? false : 'does not stream response bodies'); test('after streaming response', () async { From 55fb0408b5a128c9ece6eb7cda3b29f231ecbaba Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 25 May 2025 14:36:06 +0100 Subject: [PATCH 12/47] Added section to README Improved documentation --- pkgs/http/README.md | 81 ++++++++++++++++++++++++++++++++ pkgs/http/lib/src/abortable.dart | 33 +++++++------ 2 files changed, 100 insertions(+), 14 deletions(-) diff --git a/pkgs/http/README.md b/pkgs/http/README.md index 07e4394eae..52c98ba958 100644 --- a/pkgs/http/README.md +++ b/pkgs/http/README.md @@ -113,6 +113,87 @@ the [`RetryClient()`][new RetryClient] constructor. [new RetryClient]: https://pub.dev/documentation/http/latest/retry/RetryClient/RetryClient.html +## Aborting requests + +Some clients, such as [`BrowserClient`][browserclient], [`IOClient`][ioclient], and +[`RetryClient`][retryclient], support abortion of requests in-flight. + +Abortion in this way can only be performed when using [`Client.send`][clientsend] or +[`BaseRequest.send`][baserequestsend] with an [`Abortable`][abortable] request (such +as [`AbortableRequest`][abortablerequest]). + +To abort a request, complete the [`Abortable.abortTrigger`][aborttrigger]. + +Depending on when the abortion is triggered, an [`AbortedRequest`][abortedrequest] may be thrown in different places. + +```dart +import 'dart:async'; +import 'package:http/http.dart' as http; + +Future main() async { + final abortTrigger = Completer(); + final client = Client(); + final request = AbortableRequest( + 'GET', + Uri.parse('http://example.org'), + abortTrigger: abortTrigger.future, + ); + + // Whenever abortion is required: + // > abortTrigger.complete(); + + // Send request + final StreamedResponse response; + try { + response = await client.send(request); + } on AbortedRequest { + // request aborted before it was fully sent + rethrow; + } + + // Using full response bytes listener + response.stream.listen( + (data) { + // consume response bytes + }, + onError: (Object err) { + if (err is AbortedRequest) { + // request aborted whilst response bytes are being streamed; + // the stream will always be finished early + } + }, + onDone: () { + // response bytes consumed, or partially consumed if finished + // early due to abortion + }, + ); + + // Alternatively, using `asFuture` + try { + await response.stream.listen( + (data) { + // consume response bytes + }, + ).asFuture(); + } on AbortedRequest { + // request aborted whilst response bytes are being streamed + rethrow; + } + // response bytes fully consumed +} +``` + +[browserclient]: https://pub.dev/documentation/http/latest/browser_client/BrowserClient-class.html +[ioclient]: https://pub.dev/documentation/http/latest/io_client/IOClient-class.html +[retryclient]: https://pub.dev/documentation/http/latest/retry/RetryClient-class.html +[clientsend]: https://pub.dev/documentation/http/latest/http/Client/send.html +[baserequestsend]: https://pub.dev/documentation/http/latest/http/BaseRequest/send.html +[abortable]: https://pub.dev/documentation/http/latest/http/Abortable-class.html +[abortablerequest]: https://pub.dev/documentation/http/latest/http/AbortableRequest-class.html +[aborttrigger]: https://pub.dev/documentation/http/latest/http/Abortable/abortTrigger.html +[abortedrequest]: https://pub.dev/documentation/http/latest/http/AbortedRequest-class.html + + ## Choosing an implementation There are multiple implementations of the `package:http` [`Client`][client] interface. By default, `package:http` uses [`BrowserClient`][browserclient] on the web and [`IOClient`][ioclient] on all other platforms. You can choose a different [`Client`][client] implementation based on the needs of your application. diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index 2050a3111d..2190d2fe2c 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -6,32 +6,37 @@ import 'dart:async'; import 'base_request.dart'; import 'client.dart'; -import 'exception.dart'; +import 'streamed_response.dart'; /// Enables a request to be recognised by a [Client] as abortable abstract mixin class Abortable implements BaseRequest { - /// This request will be aborted if this completes + /// Completion of this future aborts this request (if the client supports + /// abortion) /// - /// A common pattern is aborting a request when another event occurs (such as - /// a user action). A [Completer] may be used to implement this. + /// Requests/responses may be aborted at any time during their lifecycle. /// - /// Another pattern is a timeout. Use [Future.delayed] to achieve this. + /// * If completed before the request has been finalized and sent, + /// [Client.send] completes with an [AbortedRequest] exception + /// * If completed after the response headers are available, or whilst + /// streaming response bytes, clients inject [AbortedRequest] into the + /// [StreamedResponse.stream] then finish it early + /// * If completed after the response is fully complete, there is no effect /// - /// This future must not complete to an error. + /// 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 may complete at any time - a [AbortedRequest] will be thrown - /// by [send]/[Client.send] if it is completed before the request is complete. + /// This future must not complete to an error. /// - /// Non-'package:http' [Client]s may unexpectedly not support this trigger. + /// Some clients may not support abortion, or may not support this trigger. abstract final Future? abortTrigger; } -/// Thrown when a HTTP request is aborted +/// Thrown when an HTTP request is aborted /// -/// Usually, this is due to [Abortable.abortTrigger] completing before the -/// request is already complete. However, some clients' [Client.close] -/// implementation may cause open requests to throw this (or a standard -/// [ClientException]). +/// Usually, this is due to [Abortable.abortTrigger]. See documentation on that +/// property for more info. class AbortedRequest implements Exception { /// Indicator that the request has been aborted const AbortedRequest(); From 1c195fbf77667e70bdb16fe16745c9f2348f7a84 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 25 May 2025 14:38:56 +0100 Subject: [PATCH 13/47] Bumped to v1.5.0 Updated CHANGELOG --- pkgs/http/CHANGELOG.md | 3 ++- pkgs/http/pubspec.yaml | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkgs/http/CHANGELOG.md b/pkgs/http/CHANGELOG.md index 73e036cae4..606faf8d1d 100644 --- a/pkgs/http/CHANGELOG.md +++ b/pkgs/http/CHANGELOG.md @@ -1,5 +1,6 @@ -## 1.4.1 +## 1.5.0 +* Added support for abortion of requests * Clarify that some header names may not be sent/received. ## 1.4.0 diff --git a/pkgs/http/pubspec.yaml b/pkgs/http/pubspec.yaml index bd915cf4f7..2c8eafc94f 100644 --- a/pkgs/http/pubspec.yaml +++ b/pkgs/http/pubspec.yaml @@ -1,5 +1,5 @@ name: http -version: 1.4.1-wip +version: 1.5.0-wip description: A composable, multi-platform, Future-based API for HTTP requests. repository: https://github.com/dart-lang/http/tree/master/pkgs/http @@ -15,7 +15,7 @@ dependencies: async: ^2.5.0 http_parser: ^4.0.0 meta: ^1.3.0 - web: '>=0.5.0 <2.0.0' + web: ">=0.5.0 <2.0.0" dev_dependencies: dart_flutter_team_lints: ^3.0.0 From d19e9e37f7f2c787064c1c1ac4ab1650b7946ca7 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 27 May 2025 18:17:01 +0100 Subject: [PATCH 14/47] Fixed formatting --- .../lib/src/abort_server_web.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart b/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart index 0b3520def8..3dd1104a31 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_server_web.dart @@ -7,8 +7,8 @@ export 'server_queue_helpers.dart' show StreamQueueOfNullableObjectExtension; /// Starts the redirect test HTTP server out-of-process. Future> startServer() async => spawnHybridUri( - Uri( - scheme: 'package', - path: 'http_client_conformance_tests/src/abort_server.dart', - ), -); + Uri( + scheme: 'package', + path: 'http_client_conformance_tests/src/abort_server.dart', + ), + ); From 05f8ae1423477475465d5eb5b3227968e42efc15 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 27 May 2025 18:32:14 +0100 Subject: [PATCH 15/47] Fixed linting/analysis --- pkgs/http/lib/http.dart | 2 +- pkgs/http/lib/src/browser_client.dart | 5 +---- pkgs/http/lib/src/io_client.dart | 4 ++-- pkgs/http_client_conformance_tests/lib/src/abort_tests.dart | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pkgs/http/lib/http.dart b/pkgs/http/lib/http.dart index 42561cbc88..31043f0e32 100644 --- a/pkgs/http/lib/http.dart +++ b/pkgs/http/lib/http.dart @@ -14,12 +14,12 @@ import 'src/request.dart'; import 'src/response.dart'; import 'src/streamed_request.dart'; +export 'src/abortable.dart'; export 'src/base_client.dart'; export 'src/base_request.dart'; export 'src/base_response.dart' show BaseResponse, BaseResponseWithUrl, HeadersWithSplitValues; export 'src/byte_stream.dart'; -export 'src/abortable.dart'; export 'src/client.dart' hide zoneClient; export 'src/exception.dart'; export 'src/multipart_file.dart'; diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index feb98ec299..819dc29bbd 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -73,9 +73,8 @@ class BrowserClient extends BaseClient { final bodyBytes = await request.finalize().toBytes(); try { - Future? canceller; if (request case Abortable(:final abortTrigger?)) { - canceller = abortTrigger.whenComplete(() => _abortController.abort()); + unawaited(abortTrigger.whenComplete(_abortController.abort)); } final response = await _fetch( @@ -95,8 +94,6 @@ class BrowserClient extends BaseClient { ), ).toDart; - canceller?.ignore(); - final contentLengthHeader = response.headers.get('content-length'); final contentLength = contentLengthHeader != null diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index c3fb46ae49..78c08d889f 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -135,7 +135,7 @@ class IOClient extends BaseClient { final controller = StreamController>(sync: true); if (request case Abortable(:final abortTrigger?)) { - abortTrigger.whenComplete(() async { + unawaited(abortTrigger.whenComplete(() async { if (subscription == null) { ioRequest.abort(const AbortedRequest()); } else { @@ -145,7 +145,7 @@ class IOClient extends BaseClient { await subscription.cancel(); } await controller.close(); - }); + })); } final response = await stream.pipe(ioRequest) as HttpClientResponse; diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 0d5c949306..f4112b9482 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -152,7 +152,7 @@ void testAbort( abortTrigger.complete(); - bool triggeredAbortedRequest = false; + var triggeredAbortedRequest = false; try { await abortResponse.stream.drain(); } on AbortedRequest { From 7ee00e6d9cebd05873d172b932ab067fc955f370 Mon Sep 17 00:00:00 2001 From: Luka S Date: Tue, 27 May 2025 21:46:42 +0100 Subject: [PATCH 16/47] Apply basic suggestions from code review Co-authored-by: Brian Quinlan --- pkgs/http/CHANGELOG.md | 4 ++-- pkgs/http/README.md | 12 +++++++---- pkgs/http/lib/src/abortable.dart | 20 +++++++++---------- pkgs/http/lib/src/io_client.dart | 6 +++++- pkgs/http/lib/src/multipart_request.dart | 2 +- pkgs/http/lib/src/request.dart | 2 +- pkgs/http/lib/src/streamed_request.dart | 2 +- .../lib/http_client_conformance_tests.dart | 3 +-- .../lib/src/abort_server.dart | 2 -- .../pubspec.yaml | 1 + 10 files changed, 30 insertions(+), 24 deletions(-) diff --git a/pkgs/http/CHANGELOG.md b/pkgs/http/CHANGELOG.md index 606faf8d1d..ee8bfa73b1 100644 --- a/pkgs/http/CHANGELOG.md +++ b/pkgs/http/CHANGELOG.md @@ -1,6 +1,6 @@ -## 1.5.0 +## 1.5.0-wip -* Added support for abortion of requests +* Added support for aborting requests before they complete. * Clarify that some header names may not be sent/received. ## 1.4.0 diff --git a/pkgs/http/README.md b/pkgs/http/README.md index 52c98ba958..c10f22ee35 100644 --- a/pkgs/http/README.md +++ b/pkgs/http/README.md @@ -116,15 +116,19 @@ the [`RetryClient()`][new RetryClient] constructor. ## Aborting requests Some clients, such as [`BrowserClient`][browserclient], [`IOClient`][ioclient], and -[`RetryClient`][retryclient], support abortion of requests in-flight. +[`RetryClient`][retryclient], support aborting requests before they complete. -Abortion in this way can only be performed when using [`Client.send`][clientsend] or +Aborting in this way can only be performed when using [`Client.send`][clientsend] or [`BaseRequest.send`][baserequestsend] with an [`Abortable`][abortable] request (such as [`AbortableRequest`][abortablerequest]). To abort a request, complete the [`Abortable.abortTrigger`][aborttrigger]. -Depending on when the abortion is triggered, an [`AbortedRequest`][abortedrequest] may be thrown in different places. +If the request is aborted before the response `Future` completes, then the response +`Future` will complete with [`AbortedRequest`][abortedrequest]. If the response is +a `StreamedResponse` and the the request is cancelled while the response stream +is being consumed, then the response stream will contain a +[`AbortedRequest`][abortedrequest]. ```dart import 'dart:async'; @@ -135,7 +139,7 @@ Future main() async { final client = Client(); final request = AbortableRequest( 'GET', - Uri.parse('http://example.org'), + Uri.https('example.com'), abortTrigger: abortTrigger.future, ); diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index 2190d2fe2c..518ad51731 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// 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. @@ -8,34 +8,34 @@ import 'base_request.dart'; import 'client.dart'; import 'streamed_response.dart'; -/// Enables a request to be recognised by a [Client] as abortable +/// 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) + /// 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 an [AbortedRequest] exception + /// [Client.send] completes with an [AbortedRequest] exception. /// * If completed after the response headers are available, or whilst - /// streaming response bytes, clients inject [AbortedRequest] into the - /// [StreamedResponse.stream] then finish it early - /// * If completed after the response is fully complete, there is no effect + /// streaming the response, clients inject [AbortedRequest] 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 to an error. + /// This future must not complete with an error. /// /// Some clients may not support abortion, or may not support this trigger. abstract final Future? abortTrigger; } -/// Thrown when an HTTP request is aborted +/// Thrown when an HTTP request is aborted. /// -/// Usually, this is due to [Abortable.abortTrigger]. See documentation on that +/// This exception is triggered when [Abortable.abortTrigger] completes. /// property for more info. class AbortedRequest implements Exception { /// Indicator that the request has been aborted diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 78c08d889f..97b8da9906 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -197,7 +197,11 @@ class IOClient extends BaseClient { /// Closes the client. /// - /// Terminates all active connections, which may cause them to throw + /// Terminates all active connections. If a client remains unclosed, the Dart + /// process may not terminate. + /// + /// The behavior of `close` is not defined if there are requests executing when `close` + /// is called. /// [AbortedRequest] or [ClientException]/[SocketException]. If a client /// remains unclosed, the Dart process may not terminate. @override diff --git a/pkgs/http/lib/src/multipart_request.dart b/pkgs/http/lib/src/multipart_request.dart index 279628576f..e571574d21 100644 --- a/pkgs/http/lib/src/multipart_request.dart +++ b/pkgs/http/lib/src/multipart_request.dart @@ -162,7 +162,7 @@ class MultipartRequest extends BaseRequest { } } -/// A [MultipartRequest] which supports abortion using [abortTrigger] +/// A [MultipartRequest] which supports abortion using [abortTrigger]. /// /// A future breaking version of 'package:http' will merge this into /// [MultipartRequest], making it a requirement. diff --git a/pkgs/http/lib/src/request.dart b/pkgs/http/lib/src/request.dart index ad13794bc4..b7e56ab559 100644 --- a/pkgs/http/lib/src/request.dart +++ b/pkgs/http/lib/src/request.dart @@ -184,7 +184,7 @@ class Request extends BaseRequest { } } -/// A [Request] which supports abortion using [abortTrigger] +/// A [Request] which supports abortion using [abortTrigger]. /// /// A future breaking version of 'package:http' will merge this into [Request], /// making it a requirement. diff --git a/pkgs/http/lib/src/streamed_request.dart b/pkgs/http/lib/src/streamed_request.dart index bca35fb3f1..8a910c54f9 100644 --- a/pkgs/http/lib/src/streamed_request.dart +++ b/pkgs/http/lib/src/streamed_request.dart @@ -55,7 +55,7 @@ class StreamedRequest extends BaseRequest { } } -/// A [StreamedRequest] which supports abortion using [abortTrigger] +/// A [StreamedRequest] which supports abortion using [abortTrigger]. /// /// A future breaking version of 'package:http' will merge this into /// [StreamedRequest], making it a requirement. diff --git a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart index 12884f25e7..8f7823250b 100644 --- a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart @@ -94,8 +94,7 @@ void testAll( bool canSendCookieHeaders = false, bool canReceiveSetCookieHeaders = false, bool supportsMultipartRequest = true, - // TODO: make this false, for now true to see what breaks. - bool supportsAbort = true, + bool supportsAbort = false, }) { testRequestBody(clientFactory()); testRequestBodyStreamed(clientFactory(), diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_server.dart b/pkgs/http_client_conformance_tests/lib/src/abort_server.dart index c66454e18a..dbd654bc7a 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_server.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_server.dart @@ -22,8 +22,6 @@ void hybridMain(StreamChannel channel) async { late HttpServer server; server = (await HttpServer.bind('localhost', 0)) ..listen((request) async { - // TODO: might have to ignore exceptions in the server because it will - // probably be disconnected await request.drain(); request.response.headers.set('Access-Control-Allow-Origin', '*'); diff --git a/pkgs/http_client_conformance_tests/pubspec.yaml b/pkgs/http_client_conformance_tests/pubspec.yaml index dd566b8798..60f8a2fd7f 100644 --- a/pkgs/http_client_conformance_tests/pubspec.yaml +++ b/pkgs/http_client_conformance_tests/pubspec.yaml @@ -15,6 +15,7 @@ dependencies: stream_channel: ^2.1.1 test: ^1.21.2 +# TODO(brianquinlan): Remove dependency_overrides when package:http 1.5.0 is released. dependency_overrides: http: path: ../http From 3914e2bdb85e9705cf8a18b9d9f1ebca8730e874 Mon Sep 17 00:00:00 2001 From: Luka S Date: Tue, 27 May 2025 21:48:49 +0100 Subject: [PATCH 17/47] Fix applied suggestion --- pkgs/http/lib/src/io_client.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 97b8da9906..ddf672fb5d 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -202,8 +202,6 @@ class IOClient extends BaseClient { /// /// The behavior of `close` is not defined if there are requests executing when `close` /// is called. - /// [AbortedRequest] or [ClientException]/[SocketException]. If a client - /// remains unclosed, the Dart process may not terminate. @override void close() { if (_inner != null) { From 1088ec87ce8156491e22fda1ad1c18b5e8997a99 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 27 May 2025 22:22:42 +0100 Subject: [PATCH 18/47] Made `AbortedRequest` extend `ClientException` Fixed compilation bug Apply more suggestions from code review --- pkgs/http/lib/retry.dart | 6 +-- pkgs/http/lib/src/abortable.dart | 6 +-- pkgs/http/lib/src/browser_client.dart | 21 +++------- pkgs/http/lib/src/io_client.dart | 38 ++++++++++--------- .../test/html/client_conformance_test.dart | 15 +++++--- .../http/test/io/client_conformance_test.dart | 1 + .../lib/src/abort_server.dart | 1 - 7 files changed, 42 insertions(+), 46 deletions(-) diff --git a/pkgs/http/lib/retry.dart b/pkgs/http/lib/retry.dart index fccbc6a69d..a82d2b3c02 100644 --- a/pkgs/http/lib/retry.dart +++ b/pkgs/http/lib/retry.dart @@ -113,6 +113,8 @@ final class RetryClient extends BaseClient { StreamedResponse? response; try { response = await _inner.send(_copyRequest(request, splitter.split())); + } on AbortedRequest { + rethrow; } catch (error, stackTrace) { if (i == _retries || !await _whenError(error, stackTrace)) rethrow; } @@ -122,7 +124,7 @@ final class RetryClient extends BaseClient { // Make sure the response stream is listened to so that we don't leave // dangling connections. - _unawaited(response.stream.listen((_) {}).cancel().catchError((_) {})); + unawaited(response.stream.listen((_) {}).cancel().catchError((_) {})); } await Future.delayed(_delay(i)); @@ -169,5 +171,3 @@ bool _defaultWhenError(Object error, StackTrace stackTrace) => false; Duration _defaultDelay(int retryCount) => const Duration(milliseconds: 500) * math.pow(1.5, retryCount); - -void _unawaited(Future? f) {} diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index 518ad51731..a531be1367 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -6,6 +6,7 @@ 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. @@ -37,7 +38,6 @@ abstract mixin class Abortable implements BaseRequest { /// /// This exception is triggered when [Abortable.abortTrigger] completes. /// property for more info. -class AbortedRequest implements Exception { - /// Indicator that the request has been aborted - const AbortedRequest(); +class AbortedRequest extends ClientException { + AbortedRequest([Uri? uri]) : super('Request aborted by `abortTrigger`', uri); } diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index 819dc29bbd..df9b726432 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -74,7 +74,9 @@ class BrowserClient extends BaseClient { final bodyBytes = await request.finalize().toBytes(); try { if (request case Abortable(:final abortTrigger?)) { - unawaited(abortTrigger.whenComplete(_abortController.abort)); + // Tear-offs of external extension type interop members are disallowed + // ignore: unnecessary_lambdas + unawaited(abortTrigger.whenComplete(() => _abortController.abort())); } final response = await _fetch( @@ -122,12 +124,6 @@ class BrowserClient extends BaseClient { url: Uri.parse(response.url), reasonPhrase: response.statusText, ); - } on DOMException catch (e, st) { - if (e.name == 'AbortError') { - Error.throwWithStackTrace(const AbortedRequest(), st); - } - - _rethrowAsClientException(e, st, request); } catch (e, st) { _rethrowAsClientException(e, st, request); } finally { @@ -149,6 +145,9 @@ class BrowserClient extends BaseClient { } Never _rethrowAsClientException(Object e, StackTrace st, BaseRequest request) { + if (e case DOMException(:final name) when name == 'AbortError') { + Error.throwWithStackTrace(AbortedRequest(request.url), st); + } if (e is! ClientException) { var message = e.toString(); if (message.startsWith('TypeError: ')) { @@ -177,14 +176,6 @@ Stream> _readBody(BaseRequest request, Response response) async* { } yield (chunk.value! as JSUint8Array).toDart; } - } on DOMException catch (e, st) { - isError = true; - - if (e.name == 'AbortError') { - Error.throwWithStackTrace(const AbortedRequest(), st); - } - - _rethrowAsClientException(e, st, request); } catch (e, st) { isError = true; _rethrowAsClientException(e, st, request); diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index ddf672fb5d..cefd0f9c4d 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -131,36 +131,38 @@ class IOClient extends BaseClient { // However, we instead inject an error into the response stream to match // the behaviour of `BrowserClient`. - StreamSubscription>? subscription; - final controller = StreamController>(sync: true); + StreamSubscription>? ioResponseSubscription; + final responseController = StreamController>(sync: true); if (request case Abortable(:final abortTrigger?)) { - unawaited(abortTrigger.whenComplete(() async { - if (subscription == null) { - ioRequest.abort(const AbortedRequest()); - } else { - if (!controller.isClosed) { - controller.addError(const AbortedRequest()); + unawaited( + abortTrigger.whenComplete(() async { + if (ioResponseSubscription == null) { + ioRequest.abort(AbortedRequest(request.url)); + } else { + if (!responseController.isClosed) { + responseController.addError(AbortedRequest(request.url)); + } + await ioResponseSubscription.cancel(); } - await subscription.cancel(); - } - await controller.close(); - })); + await responseController.close(); + }), + ); } final response = await stream.pipe(ioRequest) as HttpClientResponse; - subscription = response.listen( - controller.add, - onDone: controller.close, + ioResponseSubscription = response.listen( + responseController.add, + onDone: responseController.close, onError: (Object err, StackTrace stackTrace) { if (err is HttpException) { - controller.addError( + responseController.addError( ClientException(err.message, err.uri), stackTrace, ); } else { - controller.addError(err, stackTrace); + responseController.addError(err, stackTrace); } }, ); @@ -174,7 +176,7 @@ class IOClient extends BaseClient { }); return _IOStreamedResponseV2( - controller.stream, + responseController.stream, response.statusCode, contentLength: response.contentLength == -1 ? null : response.contentLength, diff --git a/pkgs/http/test/html/client_conformance_test.dart b/pkgs/http/test/html/client_conformance_test.dart index 4400c6a8a1..51a8134945 100644 --- a/pkgs/http/test/html/client_conformance_test.dart +++ b/pkgs/http/test/html/client_conformance_test.dart @@ -10,10 +10,13 @@ import 'package:http_client_conformance_tests/http_client_conformance_tests.dart import 'package:test/test.dart'; void main() { - testAll(BrowserClient.new, - redirectAlwaysAllowed: true, - canStreamRequestBody: false, - canStreamResponseBody: true, - canWorkInIsolates: false, - supportsMultipartRequest: false); + testAll( + BrowserClient.new, + redirectAlwaysAllowed: true, + canStreamRequestBody: false, + canStreamResponseBody: true, + canWorkInIsolates: false, + supportsMultipartRequest: false, + supportsAbort: true, + ); } diff --git a/pkgs/http/test/io/client_conformance_test.dart b/pkgs/http/test/io/client_conformance_test.dart index cc4b788ebd..149b041a4e 100644 --- a/pkgs/http/test/io/client_conformance_test.dart +++ b/pkgs/http/test/io/client_conformance_test.dart @@ -16,5 +16,6 @@ void main() { canSendCookieHeaders: true, correctlyHandlesNullHeaderValues: false, // https://github.com/dart-lang/sdk/issues/56636 + supportsAbort: true, ); } diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_server.dart b/pkgs/http_client_conformance_tests/lib/src/abort_server.dart index dbd654bc7a..3075d779b0 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_server.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_server.dart @@ -22,7 +22,6 @@ void hybridMain(StreamChannel channel) async { late HttpServer server; server = (await HttpServer.bind('localhost', 0)) ..listen((request) async { - await request.drain(); request.response.headers.set('Access-Control-Allow-Origin', '*'); request.response.headers.set('Content-Type', 'text/plain'); From 917d573ba32616df1a512cf224237296311afb9d Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 28 May 2025 11:04:25 -0700 Subject: [PATCH 19/47] s/_abortController/abortController/g --- pkgs/http/lib/src/browser_client.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index df9b726432..a5c9e5baf8 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -68,15 +68,15 @@ class BrowserClient extends BaseClient { 'HTTP request failed. Client is already closed.', request.url); } - final _abortController = AbortController(); - _openRequestAbortControllers.add(_abortController); + 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())); + unawaited(abortTrigger.whenComplete(() => abortController.abort())); } final response = await _fetch( @@ -91,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; @@ -127,7 +127,7 @@ class BrowserClient extends BaseClient { } catch (e, st) { _rethrowAsClientException(e, st, request); } finally { - _openRequestAbortControllers.remove(_abortController); + _openRequestAbortControllers.remove(abortController); } } From 6b5d2e241b6aeac345d8651f080a2efe25d27097 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 28 May 2025 11:07:11 -0700 Subject: [PATCH 20/47] Update io_client.dart --- pkgs/http/lib/src/io_client.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index cefd0f9c4d..707d9bfb3f 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -202,8 +202,8 @@ class IOClient extends BaseClient { /// Terminates all active connections. If a client remains unclosed, the Dart /// process may not terminate. /// - /// The behavior of `close` is not defined if there are requests executing when `close` - /// is called. + /// The behavior of `close` is not defined if there are requests executing + /// when `close` is called. @override void close() { if (_inner != null) { From bfe0031b9ae8908c5d47b3535b3791ded803fe68 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 3 Jun 2025 22:26:57 +0100 Subject: [PATCH 21/47] Test test fix --- pkgs/http/lib/src/io_client.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 707d9bfb3f..53c0f68e05 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:io'; +import '../http.dart'; import 'abortable.dart'; import 'base_client.dart'; import 'base_request.dart'; @@ -138,6 +139,10 @@ class IOClient extends BaseClient { unawaited( abortTrigger.whenComplete(() async { if (ioResponseSubscription == null) { + // TODO: This is ugly, I want to see what happens + if (request is AbortableStreamedRequest) { + unawaited(request.sink.close()); + } ioRequest.abort(AbortedRequest(request.url)); } else { if (!responseController.isClosed) { From e707d2cce7bee5b94ee3edee4712de59e8a54282 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Tue, 3 Jun 2025 22:30:32 +0100 Subject: [PATCH 22/47] Fix the analysis issue --- pkgs/http/lib/src/io_client.dart | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 53c0f68e05..e424fc85d4 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -6,12 +6,6 @@ import 'dart:async'; import 'dart:io'; import '../http.dart'; -import 'abortable.dart'; -import 'base_client.dart'; -import 'base_request.dart'; -import 'base_response.dart'; -import 'client.dart'; -import 'exception.dart'; import 'io_streamed_response.dart'; /// Create an [IOClient]. From e02a40c6914e224638a71bec4e373bc035608ded Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 15:12:29 -0700 Subject: [PATCH 23/47] Close the request sink in `during request stream` test --- pkgs/http/lib/src/io_client.dart | 4 ---- .../lib/src/abort_tests.dart | 10 ++++++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index e424fc85d4..63d981c1be 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -133,10 +133,6 @@ class IOClient extends BaseClient { unawaited( abortTrigger.whenComplete(() async { if (ioResponseSubscription == null) { - // TODO: This is ugly, I want to see what happens - if (request is AbortableStreamedRequest) { - unawaited(request.sink.close()); - } ioRequest.abort(AbortedRequest(request.url)); } else { if (!responseController.isClosed) { diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index f4112b9482..be0c45da8e 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -77,8 +77,14 @@ void testAbort( response, throwsA(isA()), ); - await request - .sink.done; // Verify that the stream subscription was cancelled. + + // Ensure that `request.sink` is still writeable after the request is + // aborted. + for (int i = 0; i < 1000; ++i) { + request.sink.add('Hello World'.codeUnits); + await Future.delayed(const Duration()); + } + await request.sink.close(); }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); test('after response', () async { From a57318a988be2fe58620deb5e2a9e861a3e72503 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 15:14:47 -0700 Subject: [PATCH 24/47] Update abort_tests.dart --- pkgs/http_client_conformance_tests/lib/src/abort_tests.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index be0c45da8e..86f21c619e 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -80,7 +80,7 @@ void testAbort( // Ensure that `request.sink` is still writeable after the request is // aborted. - for (int i = 0; i < 1000; ++i) { + for (var i = 0; i < 1000; ++i) { request.sink.add('Hello World'.codeUnits); await Future.delayed(const Duration()); } From 1cc77b04e6ba0222cf280b470b22eb05c4a63c32 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 15:44:57 -0700 Subject: [PATCH 25/47] Add an extra test --- .../lib/src/abort_tests.dart | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 86f21c619e..91bc1a4094 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -59,6 +59,33 @@ void testAbort( ); }); + test('before first streamed item', () async { + final abortTrigger = Completer(); + + final request = AbortableStreamedRequest( + 'POST', + serverUrl, + abortTrigger: abortTrigger.future, + ); + + final response = client.send(request); + + abortTrigger.complete(); + + expect( + response, + throwsA(isA()), + ); + + // Ensure that `request.sink` is still writeable after the request is + // aborted. + for (var i = 0; i < 1000; ++i) { + request.sink.add('Hello World'.codeUnits); + await Future.delayed(const Duration()); + } + await request.sink.close(); + }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); + test('during request stream', () async { final abortTrigger = Completer(); From 9001a1d7580f8586fef6d2a610083c999dd8a574 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 16:23:21 -0700 Subject: [PATCH 26/47] Don't listen to `HttpClientResponse` until `StreamedResponse` is listened to --- pkgs/http/lib/src/io_client.dart | 35 ++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 63d981c1be..bb40a111ee 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -127,7 +127,7 @@ class IOClient extends BaseClient { // the behaviour of `BrowserClient`. StreamSubscription>? ioResponseSubscription; - final responseController = StreamController>(sync: true); + var responseController = StreamController>(); if (request case Abortable(:final abortTrigger?)) { unawaited( @@ -138,7 +138,7 @@ class IOClient extends BaseClient { if (!responseController.isClosed) { responseController.addError(AbortedRequest(request.url)); } - await ioResponseSubscription.cancel(); + await ioResponseSubscription!.cancel(); } await responseController.close(); }), @@ -147,20 +147,25 @@ class IOClient extends BaseClient { final response = await stream.pipe(ioRequest) as HttpClientResponse; - ioResponseSubscription = response.listen( - responseController.add, - onDone: responseController.close, - onError: (Object err, StackTrace stackTrace) { - if (err is HttpException) { - responseController.addError( - ClientException(err.message, err.uri), - stackTrace, + // DO NOT SUBMIT: This controller handling is probably not sufficient! + responseController = StreamController>( + onListen: () { + ioResponseSubscription = response.listen( + responseController.add, + onDone: responseController.close, + onError: (Object err, StackTrace stackTrace) { + if (err is HttpException) { + responseController.addError( + ClientException(err.message, err.uri), + stackTrace, + ); + } else { + responseController.addError(err, stackTrace); + } + }, ); - } else { - responseController.addError(err, stackTrace); - } - }, - ); + }, + sync: true); var headers = {}; response.headers.forEach((key, values) { From 24025ba6d2ec4b9588704d80cfb4cd9273fd31ae Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 16:39:30 -0700 Subject: [PATCH 27/47] Update io_client.dart --- pkgs/http/lib/src/io_client.dart | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index bb40a111ee..cbe57832ea 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -147,7 +147,9 @@ class IOClient extends BaseClient { final response = await stream.pipe(ioRequest) as HttpClientResponse; - // DO NOT SUBMIT: This controller handling is probably not sufficient! + // !!! DO NOT SUBMIT !!! + // This is incorrect because errors added to the response controller + // before they are listened to by the client will be lost. responseController = StreamController>( onListen: () { ioResponseSubscription = response.listen( @@ -165,6 +167,9 @@ class IOClient extends BaseClient { }, ); }, + onPause: () => ioResponseSubscription!.pause(), + onResume: () => ioResponseSubscription!.resume(), + onCancel: () => ioResponseSubscription!.cancel(), sync: true); var headers = {}; From 3d5caa79b1c156df8a1d3c01b3553f12c0fbc074 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 17:01:21 -0700 Subject: [PATCH 28/47] Add a test to verify the behavior when the response is not being listened to --- .../lib/src/abort_tests.dart | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 91bc1a4094..0344b21898 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -114,7 +114,7 @@ void testAbort( await request.sink.close(); }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); - test('after response', () async { + test('after response, response stream listener', () async { final abortTrigger = Completer(); final request = AbortableRequest( @@ -132,6 +132,27 @@ void testAbort( ); }); + test('after response, response stream no listener', () async { + final abortTrigger = Completer(); + + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); + final response = await client.send(request); + + abortTrigger.complete(); + // Ensure that the abort has time to run before listening to the response + // stream + await Future.delayed(const Duration()); + + expect( + response.stream.single, + throwsA(isA()), + ); + }); + test('while streaming response', () async { final abortTrigger = Completer(); From 9da76aaa391dddaa236cbb988fcd61fd24da1f5b Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 17:22:57 -0700 Subject: [PATCH 29/47] Add a test for a paused stream --- .../lib/src/abort_tests.dart | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 0344b21898..a0b404805e 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -153,6 +153,29 @@ void testAbort( ); }); + test('after response, response stream paused', () async { + final abortTrigger = Completer(); + + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); + final response = await client.send(request); + + final subscription = response.stream.listen(print)..pause(); + abortTrigger.complete(); + // Ensure that the abort has time to run before listening to the response + // stream + await Future.delayed(const Duration()); + subscription.resume(); + + expect( + subscription.asFuture(), + throwsA(isA()), + ); + }); + test('while streaming response', () async { final abortTrigger = Completer(); From 0b32a5a920619f646584392634cadaa98410d7b6 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 17:52:19 -0700 Subject: [PATCH 30/47] Update `pubspec.yaml`s to use local http --- pkgs/cronet_http/example/pubspec.yaml | 6 ++++++ pkgs/cupertino_http/example/pubspec.yaml | 6 ++++++ pkgs/ok_http/example/pubspec.yaml | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/pkgs/cronet_http/example/pubspec.yaml b/pkgs/cronet_http/example/pubspec.yaml index dfaa39a8e7..87c8234218 100644 --- a/pkgs/cronet_http/example/pubspec.yaml +++ b/pkgs/cronet_http/example/pubspec.yaml @@ -29,3 +29,9 @@ dev_dependencies: flutter: uses-material-design: true + +# TODO(brianquinlan): Remove this when a release version of `package:http` +# supports abortable requests. +dependency_overrides: + http: + path: ../../http/ diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index 38f75b0cf9..2dcf97298b 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -38,3 +38,9 @@ dev_dependencies: flutter: uses-material-design: true + +# TODO(brianquinlan): Remove this when a release version of `package:http` +# supports abortable requests. +dependency_overrides: + http: + path: ../../http/ \ No newline at end of file diff --git a/pkgs/ok_http/example/pubspec.yaml b/pkgs/ok_http/example/pubspec.yaml index 235b649c90..873d9aaa5c 100644 --- a/pkgs/ok_http/example/pubspec.yaml +++ b/pkgs/ok_http/example/pubspec.yaml @@ -35,3 +35,9 @@ flutter: uses-material-design: true assets: - test_certs/ # Used in integration tests. + +# TODO(brianquinlan): Remove this when a release version of `package:http` +# supports abortable requests. +dependency_overrides: + http: + path: ../../http/ From b9e8543837b8a26cfb822a6a18839b4a28ece95d Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 18:02:32 -0700 Subject: [PATCH 31/47] Skip abort tests if disabled --- pkgs/http_client_conformance_tests/lib/src/abort_tests.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index a0b404805e..a3db81da38 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -26,7 +26,7 @@ import 'abort_server_vm.dart' /// skipped. void testAbort( Client client, { - bool supportsAbort = true, + bool supportsAbort = false, bool canStreamRequestBody = true, bool canStreamResponseBody = true, }) { @@ -241,5 +241,5 @@ void testAbort( expect(response.body, endsWith('9999\n')); expect(triggeredAbortedRequest, true); }); - }); + }, skip: supportsAbort ? true : 'does not support aborting requests'); } From d302777eb75d6c9509812dc02baa627ffad666dd Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 4 Jun 2025 18:12:13 -0700 Subject: [PATCH 32/47] Update abort_tests.dart --- pkgs/http_client_conformance_tests/lib/src/abort_tests.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index a3db81da38..df42c2b36a 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -241,5 +241,5 @@ void testAbort( expect(response.body, endsWith('9999\n')); expect(triggeredAbortedRequest, true); }); - }, skip: supportsAbort ? true : 'does not support aborting requests'); + }, skip: supportsAbort ? false : 'does not support aborting requests'); } From 67132f649c73db054d53bb0095297192cd7aa47d Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 25 Jun 2025 13:50:14 +0100 Subject: [PATCH 33/47] Fix bugs with abortion timing in IOClient --- pkgs/http/lib/src/io_client.dart | 103 ++++++++++++++++++------------- 1 file changed, 61 insertions(+), 42 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index cbe57832ea..cea7eb4d4e 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -120,57 +120,76 @@ class IOClient extends BaseClient { ioRequest.headers.set(name, value); }); - // We can only abort the actual connection up until the point we obtain - // the response. - // After that point, the full response bytes are always available. - // However, we instead inject an error into the response stream to match - // the behaviour of `BrowserClient`. - - StreamSubscription>? ioResponseSubscription; - var responseController = StreamController>(); + // SDK request aborting is only effective up until the request is closed, + // at which point the full response always becomes available. + // This occurs at `pipe`, which automatically closes the request once the + // request stream has been pumped in. + // Therefore, we have multiple strategies: + // * If the user aborts before we have a response, we can use SDK abort, + // which causes the `pipe` (and therefore this method) to throw the + // aborted error + // * If the user aborts after we have a response but before they listen + // to it, we immediately emit the aborted error then close the response + // as soon as they listen to it + // * If the user aborts whilst streaming the response, we inject the + // aborted error, then close the response + + bool isAborted = false; + bool hasResponse = false; if (request case Abortable(:final abortTrigger?)) { unawaited( - abortTrigger.whenComplete(() async { - if (ioResponseSubscription == null) { - ioRequest.abort(AbortedRequest(request.url)); - } else { - if (!responseController.isClosed) { - responseController.addError(AbortedRequest(request.url)); - } - await ioResponseSubscription!.cancel(); - } - await responseController.close(); + abortTrigger.whenComplete(() { + isAborted = true; + if (!hasResponse) ioRequest.abort(AbortedRequest(request.url)); }), ); } final response = await stream.pipe(ioRequest) as HttpClientResponse; + hasResponse = true; + + StreamSubscription>? ioResponseSubscription; - // !!! DO NOT SUBMIT !!! - // This is incorrect because errors added to the response controller - // before they are listened to by the client will be lost. - responseController = StreamController>( - onListen: () { - ioResponseSubscription = response.listen( - responseController.add, - onDone: responseController.close, - onError: (Object err, StackTrace stackTrace) { - if (err is HttpException) { - responseController.addError( - ClientException(err.message, err.uri), - stackTrace, - ); - } else { - responseController.addError(err, stackTrace); - } - }, - ); - }, - onPause: () => ioResponseSubscription!.pause(), - onResume: () => ioResponseSubscription!.resume(), - onCancel: () => ioResponseSubscription!.cancel(), - sync: true); + late final StreamController> responseController; + responseController = StreamController( + onListen: () { + if (isAborted) { + responseController + ..addError(AbortedRequest(request.url)) + ..close(); + return; + } else if (request case Abortable(:final abortTrigger?)) { + abortTrigger.whenComplete(() { + if (!responseController.isClosed) { + responseController + ..addError(AbortedRequest(request.url)) + ..close(); + } + ioResponseSubscription?.cancel(); + }); + } + + ioResponseSubscription = response.listen( + responseController.add, + onDone: responseController.close, + onError: (Object err, StackTrace stackTrace) { + if (err is HttpException) { + responseController.addError( + ClientException(err.message, err.uri), + stackTrace, + ); + } else { + responseController.addError(err, stackTrace); + } + }, + ); + }, + onPause: () => ioResponseSubscription?.pause(), + onResume: () => ioResponseSubscription?.resume(), + onCancel: () => ioResponseSubscription?.cancel(), + sync: true, + ); var headers = {}; response.headers.forEach((key, values) { From 5ac3805c600e9072fc0bd50cf7bb1a3f491e3b1e Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 25 Jun 2025 13:52:11 +0100 Subject: [PATCH 34/47] Renamed `AbortedRequest` to `RequestAborted` --- pkgs/http/lib/retry.dart | 2 +- pkgs/http/lib/src/abortable.dart | 8 ++++---- pkgs/http/lib/src/browser_client.dart | 4 ++-- pkgs/http/lib/src/io_client.dart | 6 +++--- .../lib/src/abort_tests.dart | 16 ++++++++-------- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkgs/http/lib/retry.dart b/pkgs/http/lib/retry.dart index a82d2b3c02..71a107655e 100644 --- a/pkgs/http/lib/retry.dart +++ b/pkgs/http/lib/retry.dart @@ -113,7 +113,7 @@ final class RetryClient extends BaseClient { StreamedResponse? response; try { response = await _inner.send(_copyRequest(request, splitter.split())); - } on AbortedRequest { + } on RequestAborted { rethrow; } catch (error, stackTrace) { if (i == _retries || !await _whenError(error, stackTrace)) rethrow; diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index a531be1367..288d3e292c 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -17,9 +17,9 @@ abstract mixin class Abortable implements BaseRequest { /// 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 an [AbortedRequest] exception. + /// [Client.send] completes with an [RequestAborted] exception. /// * If completed after the response headers are available, or whilst - /// streaming the response, clients inject [AbortedRequest] into the + /// streaming the response, clients inject [RequestAborted] into the /// [StreamedResponse.stream] then close the stream. /// * If completed after the response is fully complete, there is no effect. /// @@ -38,6 +38,6 @@ abstract mixin class Abortable implements BaseRequest { /// /// This exception is triggered when [Abortable.abortTrigger] completes. /// property for more info. -class AbortedRequest extends ClientException { - AbortedRequest([Uri? uri]) : super('Request aborted by `abortTrigger`', uri); +class RequestAborted extends ClientException { + RequestAborted([Uri? uri]) : super('Request aborted by `abortTrigger`', uri); } diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index a5c9e5baf8..3e82473456 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -134,7 +134,7 @@ class BrowserClient extends BaseClient { /// Closes the client. /// /// This terminates all active requests, which may cause them to throw - /// [AbortedRequest] or [ClientException]. + /// [RequestAborted] or [ClientException]. @override void close() { for (final abortController in _openRequestAbortControllers) { @@ -146,7 +146,7 @@ class BrowserClient extends BaseClient { Never _rethrowAsClientException(Object e, StackTrace st, BaseRequest request) { if (e case DOMException(:final name) when name == 'AbortError') { - Error.throwWithStackTrace(AbortedRequest(request.url), st); + Error.throwWithStackTrace(RequestAborted(request.url), st); } if (e is! ClientException) { var message = e.toString(); diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index cea7eb4d4e..da57ba96ce 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -141,7 +141,7 @@ class IOClient extends BaseClient { unawaited( abortTrigger.whenComplete(() { isAborted = true; - if (!hasResponse) ioRequest.abort(AbortedRequest(request.url)); + if (!hasResponse) ioRequest.abort(RequestAborted(request.url)); }), ); } @@ -156,14 +156,14 @@ class IOClient extends BaseClient { onListen: () { if (isAborted) { responseController - ..addError(AbortedRequest(request.url)) + ..addError(RequestAborted(request.url)) ..close(); return; } else if (request case Abortable(:final abortTrigger?)) { abortTrigger.whenComplete(() { if (!responseController.isClosed) { responseController - ..addError(AbortedRequest(request.url)) + ..addError(RequestAborted(request.url)) ..close(); } ioResponseSubscription?.cancel(); diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index df42c2b36a..263bdc7da5 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -55,7 +55,7 @@ void testAbort( expect( client.send(request), - throwsA(isA()), + throwsA(isA()), ); }); @@ -74,7 +74,7 @@ void testAbort( expect( response, - throwsA(isA()), + throwsA(isA()), ); // Ensure that `request.sink` is still writeable after the request is @@ -102,7 +102,7 @@ void testAbort( expect( response, - throwsA(isA()), + throwsA(isA()), ); // Ensure that `request.sink` is still writeable after the request is @@ -128,7 +128,7 @@ void testAbort( expect( response.stream.single, - throwsA(isA()), + throwsA(isA()), ); }); @@ -149,7 +149,7 @@ void testAbort( expect( response.stream.single, - throwsA(isA()), + throwsA(isA()), ); }); @@ -172,7 +172,7 @@ void testAbort( expect( subscription.asFuture(), - throwsA(isA()), + throwsA(isA()), ); }); @@ -196,7 +196,7 @@ void testAbort( if (i >= 1000 && !abortTrigger.isCompleted) abortTrigger.complete(); }, ).asFuture(), - throwsA(isA()), + throwsA(isA()), ); expect(i, lessThan(48890)); }, skip: canStreamResponseBody ? false : 'does not stream response bodies'); @@ -232,7 +232,7 @@ void testAbort( var triggeredAbortedRequest = false; try { await abortResponse.stream.drain(); - } on AbortedRequest { + } on RequestAborted { triggeredAbortedRequest = true; } From baef5522fff78971692ac2369f7ce20e1100819b Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 25 Jun 2025 21:30:18 +0100 Subject: [PATCH 35/47] Fixed linting/analysis --- pkgs/http/lib/src/io_client.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index da57ba96ce..4f8ae8559f 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -134,8 +134,8 @@ class IOClient extends BaseClient { // * If the user aborts whilst streaming the response, we inject the // aborted error, then close the response - bool isAborted = false; - bool hasResponse = false; + var isAborted = false; + var hasResponse = false; if (request case Abortable(:final abortTrigger?)) { unawaited( From 63eabcaba5c5ced3f96a714d7b67880bfb0a2919 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 30 Jun 2025 14:56:16 -0700 Subject: [PATCH 36/47] Skips abort tests when `supportsAbort` is `false` --- pkgs/cupertino_http/example/pubspec.yaml | 2 +- .../lib/http_client_conformance_tests.dart | 1 + .../lib/src/abort_tests.dart | 12 ++++++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index 2dcf97298b..0d44ce4477 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -43,4 +43,4 @@ flutter: # supports abortable requests. dependency_overrides: http: - path: ../../http/ \ No newline at end of file + path: ../../http/ diff --git a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart index 8f7823250b..d7e21cea8b 100644 --- a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart @@ -79,6 +79,7 @@ export 'src/server_errors_test.dart' show testServerErrors; /// /// If [supportsAbort] is `false` then tests that assume that requests can be /// aborted will be skipped. +/// /// The tests are run against a series of HTTP servers that are started by the /// tests. If the tests are run in the browser, then the test servers are /// started in another process. Otherwise, the test servers are run in-process. diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 263bdc7da5..2eb6a7290c 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -84,7 +84,10 @@ void testAbort( await Future.delayed(const Duration()); } await request.sink.close(); - }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); + }, + skip: supportsAbort + ? (canStreamRequestBody ? false : 'does not stream response bodies') + : 'does not stream request bodies'); test('during request stream', () async { final abortTrigger = Completer(); @@ -199,7 +202,12 @@ void testAbort( throwsA(isA()), ); expect(i, lessThan(48890)); - }, skip: canStreamResponseBody ? false : 'does not stream response bodies'); + }, + skip: supportsAbort + ? (canStreamResponseBody + ? false + : 'does not stream response bodies') + : 'does not support aborting requests'); test('after streaming response', () async { final abortTrigger = Completer(); From c8a04033cd85c7801c6f8f0ea8c8ca0f976bb5a0 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 30 Jun 2025 15:06:03 -0700 Subject: [PATCH 37/47] Update abort_tests.dart --- .../http_client_conformance_tests/lib/src/abort_tests.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 2eb6a7290c..a06b4faf96 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -87,7 +87,7 @@ void testAbort( }, skip: supportsAbort ? (canStreamRequestBody ? false : 'does not stream response bodies') - : 'does not stream request bodies'); + : 'does not support aborting requests'); test('during request stream', () async { final abortTrigger = Completer(); @@ -115,7 +115,10 @@ void testAbort( await Future.delayed(const Duration()); } await request.sink.close(); - }, skip: canStreamRequestBody ? false : 'does not stream request bodies'); + }, + skip: supportsAbort + ? (canStreamRequestBody ? false : 'does not stream request bodies') + : 'does not support aborting requests'); test('after response, response stream listener', () async { final abortTrigger = Completer(); From 5d2562084be33b2b1e896966c1de3f23219c5d52 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 30 Jun 2025 15:24:32 -0700 Subject: [PATCH 38/47] Add a minor clarification about `abortTrigger` --- pkgs/http/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/http/README.md b/pkgs/http/README.md index c10f22ee35..3887dc1657 100644 --- a/pkgs/http/README.md +++ b/pkgs/http/README.md @@ -122,7 +122,7 @@ Aborting in this way can only be performed when using [`Client.send`][clientsend [`BaseRequest.send`][baserequestsend] with an [`Abortable`][abortable] request (such as [`AbortableRequest`][abortablerequest]). -To abort a request, complete the [`Abortable.abortTrigger`][aborttrigger]. +To abort a request, complete the [`Abortable.abortTrigger`][aborttrigger] `Future`. If the request is aborted before the response `Future` completes, then the response `Future` will complete with [`AbortedRequest`][abortedrequest]. If the response is From ba7f6c664be1b57d0fa8a1367e262c43df62ba51 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 30 Jun 2025 16:38:45 -0700 Subject: [PATCH 39/47] Remove spurious words --- pkgs/http/lib/src/abortable.dart | 1 - pkgs/http/lib/src/base_request.dart | 2 +- pkgs/http/lib/src/io_client.dart | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index 288d3e292c..b1354a9915 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -37,7 +37,6 @@ abstract mixin class Abortable implements BaseRequest { /// Thrown when an HTTP request is aborted. /// /// This exception is triggered when [Abortable.abortTrigger] completes. -/// property for more info. class RequestAborted extends ClientException { RequestAborted([Uri? uri]) : super('Request aborted by `abortTrigger`', uri); } diff --git a/pkgs/http/lib/src/base_request.dart b/pkgs/http/lib/src/base_request.dart index 9732b0c750..616eff9b60 100644 --- a/pkgs/http/lib/src/base_request.dart +++ b/pkgs/http/lib/src/base_request.dart @@ -23,7 +23,7 @@ import 'utils.dart'; /// methods like [get] or [BaseClient.get]. /// /// Subclasses/implementers should mixin/implement [Abortable] to support -/// abortion of requests. A future breaking version of 'package:http' will +/// request cancellation. A future breaking version of 'package:http' will /// merge [Abortable] into [BaseRequest], making it a requirement. abstract class BaseRequest { /// The HTTP method of the request. diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 4f8ae8559f..b9d4564a25 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -124,6 +124,7 @@ class IOClient extends BaseClient { // at which point the full response always becomes available. // This occurs at `pipe`, which automatically closes the request once the // request stream has been pumped in. + // // Therefore, we have multiple strategies: // * If the user aborts before we have a response, we can use SDK abort, // which causes the `pipe` (and therefore this method) to throw the From 3511b38a6b4c02ae7b94ba0ccaab8ee0cfbd1719 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 2 Jul 2025 16:00:42 +0100 Subject: [PATCH 40/47] Rename aborted exception to `RequestAbortedException` --- pkgs/http/README.md | 16 +++++++------- pkgs/http/lib/retry.dart | 2 +- pkgs/http/lib/src/abortable.dart | 11 +++++----- pkgs/http/lib/src/browser_client.dart | 4 ++-- pkgs/http/lib/src/io_client.dart | 8 ++++--- .../lib/src/abort_tests.dart | 22 +++++++++---------- 6 files changed, 33 insertions(+), 30 deletions(-) diff --git a/pkgs/http/README.md b/pkgs/http/README.md index 3887dc1657..a4f24a3938 100644 --- a/pkgs/http/README.md +++ b/pkgs/http/README.md @@ -125,10 +125,10 @@ as [`AbortableRequest`][abortablerequest]). To abort a request, complete the [`Abortable.abortTrigger`][aborttrigger] `Future`. If the request is aborted before the response `Future` completes, then the response -`Future` will complete with [`AbortedRequest`][abortedrequest]. If the response is -a `StreamedResponse` and the the request is cancelled while the response stream -is being consumed, then the response stream will contain a -[`AbortedRequest`][abortedrequest]. +`Future` will complete with [`RequestAbortedException`][requestabortedexception]. If +the response is a `StreamedResponse` and the the request is cancelled while the +response stream is being consumed, then the response stream will contain a +[`RequestAbortedException`][requestabortedexception]. ```dart import 'dart:async'; @@ -150,7 +150,7 @@ Future main() async { final StreamedResponse response; try { response = await client.send(request); - } on AbortedRequest { + } on RequestAbortedException { // request aborted before it was fully sent rethrow; } @@ -161,7 +161,7 @@ Future main() async { // consume response bytes }, onError: (Object err) { - if (err is AbortedRequest) { + if (err is RequestAbortedException) { // request aborted whilst response bytes are being streamed; // the stream will always be finished early } @@ -179,7 +179,7 @@ Future main() async { // consume response bytes }, ).asFuture(); - } on AbortedRequest { + } on RequestAbortedException { // request aborted whilst response bytes are being streamed rethrow; } @@ -195,7 +195,7 @@ Future main() async { [abortable]: https://pub.dev/documentation/http/latest/http/Abortable-class.html [abortablerequest]: https://pub.dev/documentation/http/latest/http/AbortableRequest-class.html [aborttrigger]: https://pub.dev/documentation/http/latest/http/Abortable/abortTrigger.html -[abortedrequest]: https://pub.dev/documentation/http/latest/http/AbortedRequest-class.html +[requestabortedexception]: https://pub.dev/documentation/http/latest/http/RequestAbortedException-class.html ## Choosing an implementation diff --git a/pkgs/http/lib/retry.dart b/pkgs/http/lib/retry.dart index 71a107655e..30d49eb156 100644 --- a/pkgs/http/lib/retry.dart +++ b/pkgs/http/lib/retry.dart @@ -113,7 +113,7 @@ final class RetryClient extends BaseClient { StreamedResponse? response; try { response = await _inner.send(_copyRequest(request, splitter.split())); - } on RequestAborted { + } on RequestAbortedException { rethrow; } catch (error, stackTrace) { if (i == _retries || !await _whenError(error, stackTrace)) rethrow; diff --git a/pkgs/http/lib/src/abortable.dart b/pkgs/http/lib/src/abortable.dart index b1354a9915..dd26a48a00 100644 --- a/pkgs/http/lib/src/abortable.dart +++ b/pkgs/http/lib/src/abortable.dart @@ -17,10 +17,10 @@ abstract mixin class Abortable implements BaseRequest { /// 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 an [RequestAborted] exception. + /// [Client.send] completes with [RequestAbortedException]. /// * If completed after the response headers are available, or whilst - /// streaming the response, clients inject [RequestAborted] into the - /// [StreamedResponse.stream] then close the stream. + /// 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 @@ -37,6 +37,7 @@ abstract mixin class Abortable implements BaseRequest { /// Thrown when an HTTP request is aborted. /// /// This exception is triggered when [Abortable.abortTrigger] completes. -class RequestAborted extends ClientException { - RequestAborted([Uri? uri]) : super('Request aborted by `abortTrigger`', uri); +class RequestAbortedException extends ClientException { + RequestAbortedException([Uri? uri]) + : super('Request aborted by `abortTrigger`', uri); } diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index 3e82473456..0f57d5c38d 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -134,7 +134,7 @@ class BrowserClient extends BaseClient { /// Closes the client. /// /// This terminates all active requests, which may cause them to throw - /// [RequestAborted] or [ClientException]. + /// [RequestAbortedException] or [ClientException]. @override void close() { for (final abortController in _openRequestAbortControllers) { @@ -146,7 +146,7 @@ class BrowserClient extends BaseClient { Never _rethrowAsClientException(Object e, StackTrace st, BaseRequest request) { if (e case DOMException(:final name) when name == 'AbortError') { - Error.throwWithStackTrace(RequestAborted(request.url), st); + Error.throwWithStackTrace(RequestAbortedException(request.url), st); } if (e is! ClientException) { var message = e.toString(); diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index b9d4564a25..7d64af23b8 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -142,7 +142,9 @@ class IOClient extends BaseClient { unawaited( abortTrigger.whenComplete(() { isAborted = true; - if (!hasResponse) ioRequest.abort(RequestAborted(request.url)); + if (!hasResponse) { + ioRequest.abort(RequestAbortedException(request.url)); + } }), ); } @@ -157,14 +159,14 @@ class IOClient extends BaseClient { onListen: () { if (isAborted) { responseController - ..addError(RequestAborted(request.url)) + ..addError(RequestAbortedException(request.url)) ..close(); return; } else if (request case Abortable(:final abortTrigger?)) { abortTrigger.whenComplete(() { if (!responseController.isClosed) { responseController - ..addError(RequestAborted(request.url)) + ..addError(RequestAbortedException(request.url)) ..close(); } ioResponseSubscription?.cancel(); diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index a06b4faf96..23f182e820 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -55,7 +55,7 @@ void testAbort( expect( client.send(request), - throwsA(isA()), + throwsA(isA()), ); }); @@ -74,7 +74,7 @@ void testAbort( expect( response, - throwsA(isA()), + throwsA(isA()), ); // Ensure that `request.sink` is still writeable after the request is @@ -105,7 +105,7 @@ void testAbort( expect( response, - throwsA(isA()), + throwsA(isA()), ); // Ensure that `request.sink` is still writeable after the request is @@ -134,7 +134,7 @@ void testAbort( expect( response.stream.single, - throwsA(isA()), + throwsA(isA()), ); }); @@ -155,7 +155,7 @@ void testAbort( expect( response.stream.single, - throwsA(isA()), + throwsA(isA()), ); }); @@ -178,7 +178,7 @@ void testAbort( expect( subscription.asFuture(), - throwsA(isA()), + throwsA(isA()), ); }); @@ -202,7 +202,7 @@ void testAbort( if (i >= 1000 && !abortTrigger.isCompleted) abortTrigger.complete(); }, ).asFuture(), - throwsA(isA()), + throwsA(isA()), ); expect(i, lessThan(48890)); }, @@ -240,17 +240,17 @@ void testAbort( abortTrigger.complete(); - var triggeredAbortedRequest = false; + var requestAbortCaught = false; try { await abortResponse.stream.drain(); - } on RequestAborted { - triggeredAbortedRequest = true; + } on RequestAbortedException { + requestAbortCaught = true; } final response = await client.get(serverUrl); expect(response.statusCode, 200); expect(response.body, endsWith('9999\n')); - expect(triggeredAbortedRequest, true); + expect(requestAbortCaught, true); }); }, skip: supportsAbort ? false : 'does not support aborting requests'); } From a9255fd8c9845464d13b9d549690e7b25477150e Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 2 Jul 2025 16:23:48 +0100 Subject: [PATCH 41/47] Adjust "while streaming response" test to count response lines --- .../lib/src/abort_tests.dart | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 23f182e820..36ebb97cc5 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'package:async/async.dart'; import 'package:http/http.dart'; @@ -182,35 +183,41 @@ void testAbort( ); }); - test('while streaming response', () async { - final abortTrigger = Completer(); - - final request = AbortableRequest( - 'GET', - serverUrl, - abortTrigger: abortTrigger.future, - ); - final response = await client.send(request); - - // We want to count data bytes, not 'packet' count, because different - // clients will use different size/numbers of 'packet's - var i = 0; - expect( - response.stream.listen( - (data) { - i += data.length; - if (i >= 1000 && !abortTrigger.isCompleted) abortTrigger.complete(); - }, - ).asFuture(), - throwsA(isA()), - ); - expect(i, lessThan(48890)); - }, - skip: supportsAbort - ? (canStreamResponseBody - ? false - : 'does not stream response bodies') - : 'does not support aborting requests'); + test( + 'while streaming response', + () async { + final abortTrigger = Completer(); + + final request = AbortableRequest( + 'GET', + serverUrl, + abortTrigger: abortTrigger.future, + ); + final response = await client.send(request); + + // We split the recieved response into lines and ensure we recieved + // fewer than the 10000 we sent + // 1 line = 1 `i` + var i = 0; + await expectLater( + response.stream + .transform(Utf8Decoder()) + .transform(LineSplitter()) + .listen( + (_) { + if (++i >= 1000 && !abortTrigger.isCompleted) { + abortTrigger.complete(); + } + }, + ).asFuture(), + throwsA(isA()), + ); + expect(i, lessThan(10000)); + }, + skip: supportsAbort + ? (canStreamResponseBody ? false : 'does not stream response bodies') + : 'does not support aborting requests', + ); test('after streaming response', () async { final abortTrigger = Completer(); From 0b4c3a06e1844e841ad1ce049de921dd8527ce8e Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 2 Jul 2025 13:10:55 -0700 Subject: [PATCH 42/47] Add const where needed --- pkgs/http_client_conformance_tests/lib/src/abort_tests.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index 36ebb97cc5..e6308e52e0 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -201,8 +201,8 @@ void testAbort( var i = 0; await expectLater( response.stream - .transform(Utf8Decoder()) - .transform(LineSplitter()) + .transform(const Utf8Decoder()) + .transform(const LineSplitter()) .listen( (_) { if (++i >= 1000 && !abortTrigger.isCompleted) { From dc2b4ae13c19db0b7f46dd2f762458070a685f36 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 2 Jul 2025 13:34:14 -0700 Subject: [PATCH 43/47] Update abort_tests.dart --- pkgs/http_client_conformance_tests/lib/src/abort_tests.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart index e6308e52e0..bfc691b269 100644 --- a/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/abort_tests.dart @@ -195,9 +195,8 @@ void testAbort( ); final response = await client.send(request); - // We split the recieved response into lines and ensure we recieved - // fewer than the 10000 we sent - // 1 line = 1 `i` + // Verify that fewer than the 10000 lines sent by the server are + // received. var i = 0; await expectLater( response.stream From a6807d510fd5a29c325177c8ada9aae78bfa4c0a Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 6 Jul 2025 15:15:49 +0100 Subject: [PATCH 44/47] Added abort tests for retry client Added re-request prevention to retry client on abortion even when inner client does not throw --- pkgs/http/lib/retry.dart | 14 +++++ pkgs/http/lib/src/mock_client.dart | 5 ++ pkgs/http/test/http_retry_test.dart | 90 +++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/pkgs/http/lib/retry.dart b/pkgs/http/lib/retry.dart index 30d49eb156..8166fd24b4 100644 --- a/pkgs/http/lib/retry.dart +++ b/pkgs/http/lib/retry.dart @@ -52,6 +52,11 @@ final class RetryClient extends BaseClient { /// the client has a chance to perform side effects like logging. The /// `response` parameter will be null if the request was retried due to an /// error for which [whenError] returned `true`. + /// + /// If the inner client supports aborting requests, then this client will + /// forward any [RequestAbortedException]s thrown. A request will not be + /// retried if it is aborted (even if the inner client does not support + /// aborting requests). RetryClient( this._inner, { int retries = 3, @@ -108,10 +113,19 @@ final class RetryClient extends BaseClient { Future send(BaseRequest request) async { final splitter = StreamSplitter(request.finalize()); + bool aborted = false; + if (request case Abortable(:final abortTrigger?)) { + abortTrigger.whenComplete(() => aborted = true); + } + var i = 0; for (;;) { StreamedResponse? response; try { + // If the inner client doesn't support abortable, we still try to avoid + // re-requests when aborted + if (aborted) throw RequestAbortedException(request.url); + response = await _inner.send(_copyRequest(request, splitter.split())); } on RequestAbortedException { rethrow; diff --git a/pkgs/http/lib/src/mock_client.dart b/pkgs/http/lib/src/mock_client.dart index 52f108a577..cc02ffc028 100644 --- a/pkgs/http/lib/src/mock_client.dart +++ b/pkgs/http/lib/src/mock_client.dart @@ -4,6 +4,7 @@ import 'dart:convert'; +import 'abortable.dart'; import 'base_client.dart'; import 'base_request.dart'; import 'byte_stream.dart'; @@ -26,6 +27,10 @@ final _pngImageData = base64Decode( /// This client allows you to define a handler callback for all requests that /// are made through it so that you can mock a server without having to send /// real HTTP requests. +/// +/// This client does not support aborting requests directly - it is the +/// handler's responsibility to throw [RequestAbortedException] as and when +/// necessary. class MockClient extends BaseClient { /// The handler for receiving [StreamedRequest]s and sending /// [StreamedResponse]s. diff --git a/pkgs/http/test/http_retry_test.dart b/pkgs/http/test/http_retry_test.dart index da51154c4a..301a4046da 100644 --- a/pkgs/http/test/http_retry_test.dart +++ b/pkgs/http/test/http_retry_test.dart @@ -2,6 +2,8 @@ // 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 'package:fake_async/fake_async.dart'; import 'package:http/http.dart'; import 'package:http/retry.dart'; @@ -252,4 +254,92 @@ void main() { final response = await client.get(Uri.http('example.org', '')); expect(response.statusCode, equals(200)); }); + + test('abort in first response', () async { + final client = RetryClient( + MockClient(expectAsync1((_) async => throw RequestAbortedException())), + delay: (_) => Duration.zero, + ); + + expect( + client.get(Uri.http('example.org', '')), + throwsA(isA()), + ); + }); + + test('abort in second response', () async { + var count = 0; + final client = RetryClient( + MockClient( + expectAsync1( + (_) async { + if (++count == 1) return Response('', 503); + throw RequestAbortedException(); + }, + count: 2, + ), + ), + delay: (_) => Duration.zero, + ); + + expect( + client.get(Uri.http('example.org', '')), + throwsA(isA()), + ); + }); + + test('abort in second response stream', () async { + var count = 0; + final client = RetryClient( + MockClient.streaming( + expectAsync2( + (_, __) async { + if (++count == 1) return StreamedResponse(Stream.empty(), 503); + return StreamedResponse( + Stream.error(RequestAbortedException()), + 200, + ); + }, + count: 2, + ), + ), + delay: (_) => Duration.zero, + ); + + expect( + (await client.send(StreamedRequest('GET', Uri.http('example.org', '')))) + .stream + .single, + throwsA(isA()), + ); + }); + + test('abort without supporting inner client', () async { + final abortCompleter = Completer(); + + var count = 0; + final client = RetryClient( + MockClient( + expectAsync1( + (_) async { + if (++count == 2) abortCompleter.complete(); + return Response('', 503); + }, + count: 2, + ), + ), + delay: (_) => Duration.zero, + ); + + expect( + client.send( + AbortableRequest( + 'GET', + Uri.http('example.org', ''), + abortTrigger: abortCompleter.future, + ), + ), + throwsA(isA()), + ); + }); } From b4d843a26c48d1cc972db3edacff8ac48fd5e83c Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Mon, 7 Jul 2025 18:34:01 +0100 Subject: [PATCH 45/47] Fix lints --- pkgs/http/lib/retry.dart | 4 ++-- pkgs/http/test/http_retry_test.dart | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkgs/http/lib/retry.dart b/pkgs/http/lib/retry.dart index 8166fd24b4..8d8c370d71 100644 --- a/pkgs/http/lib/retry.dart +++ b/pkgs/http/lib/retry.dart @@ -113,9 +113,9 @@ final class RetryClient extends BaseClient { Future send(BaseRequest request) async { final splitter = StreamSplitter(request.finalize()); - bool aborted = false; + var aborted = false; if (request case Abortable(:final abortTrigger?)) { - abortTrigger.whenComplete(() => aborted = true); + unawaited(abortTrigger.whenComplete(() => aborted = true)); } var i = 0; diff --git a/pkgs/http/test/http_retry_test.dart b/pkgs/http/test/http_retry_test.dart index 301a4046da..c102f43960 100644 --- a/pkgs/http/test/http_retry_test.dart +++ b/pkgs/http/test/http_retry_test.dart @@ -294,7 +294,9 @@ void main() { MockClient.streaming( expectAsync2( (_, __) async { - if (++count == 1) return StreamedResponse(Stream.empty(), 503); + if (++count == 1) { + return StreamedResponse(const Stream.empty(), 503); + } return StreamedResponse( Stream.error(RequestAbortedException()), 200, From b66346327c9603aec28391157f2a07b55ee3cf8c Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 7 Jul 2025 11:23:02 -0700 Subject: [PATCH 46/47] Nit --- pkgs/http/test/http_retry_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/http/test/http_retry_test.dart b/pkgs/http/test/http_retry_test.dart index c102f43960..f29be9abec 100644 --- a/pkgs/http/test/http_retry_test.dart +++ b/pkgs/http/test/http_retry_test.dart @@ -316,7 +316,7 @@ void main() { ); }); - test('abort without supporting inner client', () async { + test('abort without abortable inner client', () async { final abortCompleter = Completer(); var count = 0; From f04e4fbe3dda27cbc5420e15de62a03ff7329a7b Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 7 Jul 2025 14:31:45 -0700 Subject: [PATCH 47/47] Update pkgs/http/README.md Co-authored-by: Devon Carew --- pkgs/http/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/http/README.md b/pkgs/http/README.md index a4f24a3938..547cb64e1f 100644 --- a/pkgs/http/README.md +++ b/pkgs/http/README.md @@ -132,6 +132,7 @@ response stream is being consumed, then the response stream will contain a ```dart import 'dart:async'; + import 'package:http/http.dart' as http; Future main() async {