From 3e54ab9ed3a2ff2c218b02ee0e41e257d0c3ffa0 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 27 May 2025 16:57:24 -0700 Subject: [PATCH 1/6] Add request cancellation to cupertino_http/cronet_http --- pkgs/cupertino_http/example/pubspec.yaml | 4 ++ .../lib/src/cupertino_client.dart | 57 +++++++++++++++---- pkgs/cupertino_http/pubspec.yaml | 4 ++ 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index 38f75b0cf9..b04d310779 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -38,3 +38,7 @@ dev_dependencies: flutter: uses-material-design: true + +dependency_overrides: + http: + path: ../../http \ No newline at end of file diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index 6273c48518..36c0a3f642 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -59,6 +59,7 @@ class _TaskTracker { /// Whether the response stream subscription has been cancelled. bool responseListenerCancelled = false; + bool requestAborted = false; final HttpClientRequestProfile? profile; int numRedirects = 0; Uri? lastUrl; // The last URL redirected to. @@ -192,14 +193,22 @@ class CupertinoClient extends BaseClient { static void _onComplete( URLSession session, URLSessionTask task, NSError? error) { final taskTracker = _tracker(task); - // The task will only be cancelled if the user calls - // `StreamedResponse.stream.cancel()`, which can only happen if the response - // has already been received. Therefore, it is safe to handle task - // cancellation errors as if the response completed normally. + + // There are two ways that the request can be cancelled: + // 1. The user calls `StreamedResponse.stream.cancel()`, which can only + // happen if the response has already been received. + // 2. The user aborts the request, which can happen at any point in the + // request lifecycle and results in `AbortedRequest` being thrown. + final isCancelError = error?.domain.toDartString() == 'NSURLErrorDomain' && + error?.code == _nsurlErrorCancelled; if (error != null && - !(error.domain.toDartString() == 'NSURLErrorDomain' && - error.code == _nsurlErrorCancelled)) { - final exception = NSErrorClientException(error, taskTracker.request.url); + !(isCancelError && taskTracker.responseListenerCancelled)) { + final Exception exception; + if (isCancelError) { + exception = const AbortedRequest(); + } else { + exception = NSErrorClientException(error, taskTracker.request.url); + } if (taskTracker.profile != null && taskTracker.profile!.requestData.endTime == null) { // Error occurred during the request. @@ -230,7 +239,9 @@ class CupertinoClient extends BaseClient { static void _onData(URLSession session, URLSessionTask task, NSData data) { final taskTracker = _tracker(task); - if (taskTracker.responseListenerCancelled) return; + if (taskTracker.responseListenerCancelled || taskTracker.requestAborted) { + return; + } taskTracker.responseController.add(data.toList()); taskTracker.profile?.responseData.bodySink.add(data.toList()); } @@ -349,6 +360,7 @@ class CupertinoClient extends BaseClient { 'Content-Length', '${request.contentLength}'); } + NSInputStream? nsStream; if (request is Request) { // Optimize the (typical) `Request` case since assigning to // `httpBodyStream` requires a lot of expensive setup and data passing. @@ -359,10 +371,12 @@ class CupertinoClient extends BaseClient { // then setting `httpBodyStream` will cause the request to fail - // even if the stream is empty. if (profile == null) { - urlRequest.httpBodyStream = s.toNSInputStream(); + nsStream = s.toNSInputStream(); + urlRequest.httpBodyStream = nsStream; } else { final splitter = StreamSplitter(s); - urlRequest.httpBodyStream = splitter.split().toNSInputStream(); + nsStream = splitter.split().toNSInputStream(); + urlRequest.httpBodyStream = nsStream; unawaited(profile.requestData.bodySink.addStream(splitter.split())); } } @@ -370,6 +384,15 @@ class CupertinoClient extends BaseClient { // This will preserve Apple default headers - is that what we want? request.headers.forEach(urlRequest.setValueForHttpHeaderField); final task = urlSession.dataTaskWithRequest(urlRequest); + if (request case Abortable(:final abortTrigger?)) { + unawaited(abortTrigger.whenComplete(() { + final taskTracker = _tasks[task]; + if (taskTracker == null) return; + taskTracker.requestAborted = true; + task.cancel(); + })); + } + final subscription = StreamController(onCancel: () { final taskTracker = _tasks[task]; if (taskTracker == null) return; @@ -383,7 +406,19 @@ class CupertinoClient extends BaseClient { final maxRedirects = request.followRedirects ? request.maxRedirects : 0; late URLResponse result; - result = await taskTracker.responseCompleter.future; + try { + result = await taskTracker.responseCompleter.future; + } finally { + // If the request is aborted before the `NSUrlSessionTask` opens the + // `NSInputStream` attached to `NSMutableURLRequest.HTTPBodyStream`, then + // the task will not close the `NSInputStream`. + // + // This will cause the Dart portion of the `NSInputStream` implementation + // to hang waiting for a close message. + if (nsStream?.streamStatus != NSStreamStatus.NSStreamStatusClosed) { + nsStream?.close(); + } + } final response = result as HTTPURLResponse; diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index f04fd14357..fb4cf6c83b 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -32,3 +32,7 @@ flutter: macos: ffiPlugin: true sharedDarwinSource: true + +dependency_overrides: + http: + path: ../http \ No newline at end of file From 24b9493b7b62a5d7dc95f7a1e16602a054692412 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 8 Jul 2025 10:58:31 -0700 Subject: [PATCH 2/6] Update from master --- pkgs/cupertino_http/lib/src/cupertino_client.dart | 2 +- pkgs/cupertino_http/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index 36c0a3f642..b3fcb6956d 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -205,7 +205,7 @@ class CupertinoClient extends BaseClient { !(isCancelError && taskTracker.responseListenerCancelled)) { final Exception exception; if (isCancelError) { - exception = const AbortedRequest(); + exception = RequestAbortedException(); } else { exception = NSErrorClientException(error, taskTracker.request.url); } diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index fb4cf6c83b..fe448125eb 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -14,7 +14,7 @@ dependencies: ffi: ^2.1.0 flutter: sdk: flutter - http: ^1.2.0 + http: ^1.5.0-beta http_profile: ^0.1.0 objective_c: ^7.0.0 web_socket: '>=0.1.5 <2.0.0' From c67c7f64a6c8200e366da3dd3670c65b6fba4e96 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 8 Jul 2025 10:59:17 -0700 Subject: [PATCH 3/6] Update client_conformance_test.dart --- .../example/integration_test/client_conformance_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/cupertino_http/example/integration_test/client_conformance_test.dart b/pkgs/cupertino_http/example/integration_test/client_conformance_test.dart index dc76d2146d..9882537a2e 100644 --- a/pkgs/cupertino_http/example/integration_test/client_conformance_test.dart +++ b/pkgs/cupertino_http/example/integration_test/client_conformance_test.dart @@ -49,6 +49,7 @@ void main() { canReceiveSetCookieHeaders: true, canSendCookieHeaders: true, correctlyHandlesNullHeaderValues: false, + supportsAbort: true, ); }); } From 59c2586460a4d20f3586c08df4f309da87a360f4 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 8 Jul 2025 11:06:04 -0700 Subject: [PATCH 4/6] Release is good. --- pkgs/cupertino_http/example/pubspec.yaml | 6 ------ pkgs/cupertino_http/lib/src/cupertino_client.dart | 3 ++- pkgs/cupertino_http/pubspec.yaml | 4 ---- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index 0d44ce4477..38f75b0cf9 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -38,9 +38,3 @@ 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/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index b3fcb6956d..a5be9238b2 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -198,7 +198,8 @@ class CupertinoClient extends BaseClient { // 1. The user calls `StreamedResponse.stream.cancel()`, which can only // happen if the response has already been received. // 2. The user aborts the request, which can happen at any point in the - // request lifecycle and results in `AbortedRequest` being thrown. + // request lifecycle and causes `CupertinoClient.send` to throw + // a `RequestAbortedException` exception. final isCancelError = error?.domain.toDartString() == 'NSURLErrorDomain' && error?.code == _nsurlErrorCancelled; if (error != null && diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index fe448125eb..4c9579a2bb 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -32,7 +32,3 @@ flutter: macos: ffiPlugin: true sharedDarwinSource: true - -dependency_overrides: - http: - path: ../http \ No newline at end of file From 54fb93e672c0078dcab6dffc51e06a1357597007 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 8 Jul 2025 11:29:43 -0700 Subject: [PATCH 5/6] Update version --- pkgs/cupertino_http/CHANGELOG.md | 4 ++++ pkgs/cupertino_http/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/cupertino_http/CHANGELOG.md b/pkgs/cupertino_http/CHANGELOG.md index b5b1c56c8d..3dfea51e72 100644 --- a/pkgs/cupertino_http/CHANGELOG.md +++ b/pkgs/cupertino_http/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.3.0-wip + +* Add the ability to abort requests. + ## 2.2.0 * Cancel requests when the response stream is cancelled. diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index 4c9579a2bb..995c75efb5 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -1,5 +1,5 @@ name: cupertino_http -version: 2.2.0 +version: 2.3.0-wip description: >- A macOS/iOS Flutter plugin that provides access to the Foundation URL Loading System. From 01f4a38f4212f315fb1f0706089eb2bbdf92a6b7 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 16 Jul 2025 15:46:08 -0700 Subject: [PATCH 6/6] Add reference to bug --- pkgs/cupertino_http/lib/src/cupertino_client.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index a5be9238b2..456254304e 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -416,6 +416,8 @@ class CupertinoClient extends BaseClient { // // This will cause the Dart portion of the `NSInputStream` implementation // to hang waiting for a close message. + // + // See https://github.com/dart-lang/native/issues/2333 if (nsStream?.streamStatus != NSStreamStatus.NSStreamStatusClosed) { nsStream?.close(); }