Add request cancellation to cupertino_http (#1779)

Author: brianquinlan

pkgs/cupertino_http/CHANGELOG.md

@@ -1,5 +1,6 @@
-## 2.2.1-wip
+## 2.3.0-wip
 
+* Add the ability to abort requests.
 * Make `ConnectionException.toString` more helpful.
 
 ## 2.2.0

pkgs/cupertino_http/example/integration_test/client_conformance_test.dart

@@ -49,6 +49,7 @@ void main() {
       canReceiveSetCookieHeaders: true,
       canSendCookieHeaders: true,
       correctlyHandlesNullHeaderValues: false,
+      supportsAbort: true,
     );
   });
 }

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/

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,23 @@ 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 causes `CupertinoClient.send` to throw
+    //    a `RequestAbortedException` exception.
+    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 = RequestAbortedException();
+      } else {
+        exception = NSErrorClientException(error, taskTracker.request.url);
+      }
       if (taskTracker.profile != null &&
           taskTracker.profile!.requestData.endTime == null) {
         // Error occurred during the request.
@@ -230,7 +240,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 +361,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,17 +372,28 @@ 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()));
       }
     }
 
     // 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<Uint8List>(onCancel: () {
       final taskTracker = _tasks[task];
       if (taskTracker == null) return;
@@ -383,7 +407,21 @@ 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.
+      //
+      // See https://github.com/dart-lang/native/issues/2333
+      if (nsStream?.streamStatus != NSStreamStatus.NSStreamStatusClosed) {
+        nsStream?.close();
+      }
+    }
 
     final response = result as HTTPURLResponse;
 

pkgs/cupertino_http/pubspec.yaml

@@ -1,5 +1,5 @@
 name: cupertino_http
-version: 2.2.1-wip
+version: 2.3.0-wip
 description: >-
   A macOS/iOS Flutter plugin that provides access to the Foundation URL
   Loading System.
@@ -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'