Integrate cupertino_http with http_profile

pkgs/cupertino_http/example/pubspec.yaml

@@ -10,7 +10,7 @@ environment:
   flutter: '>=3.10.0'
 
 dependencies:
-  cached_network_image: ^3.2.3
+  cached_network_image: ^3.3.0
   cupertino_http:
     path: ../
   cupertino_icons: ^1.0.2

pkgs/cupertino_http/lib/src/cupertino_client.dart

@@ -12,6 +12,7 @@ import 'dart:typed_data';
 
 import 'package:async/async.dart';
 import 'package:http/http.dart';
+import 'package:http_profile/http_profile.dart';
 
 import 'cupertino_api.dart';
 
@@ -21,9 +22,10 @@ class _TaskTracker {
   final responseCompleter = Completer<URLResponse>();
   final BaseRequest request;
   final responseController = StreamController<Uint8List>();
+  final HttpClientRequestProfile? profile;
   int numRedirects = 0;
 
-  _TaskTracker(this.request);
+  _TaskTracker(this.request, this.profile);
 
   void close() {
     responseController.close();
@@ -152,12 +154,13 @@ class CupertinoClient extends BaseClient {
   static void _onComplete(
       URLSession session, URLSessionTask task, Error? error) {
     final taskTracker = _tracker(task);
-
+    taskTracker.profile?.responseData.endTime = DateTime.now();
     if (error != null) {
       final exception = ClientException(
           error.localizedDescription ?? 'Unknown', taskTracker.request.url);
       if (taskTracker.responseCompleter.isCompleted) {
         taskTracker.responseController.addError(exception);
+        taskTracker.profile?.responseData.error = exception.toString();
       } else {
         taskTracker.responseCompleter.completeError(exception);
       }
@@ -172,6 +175,7 @@ class CupertinoClient extends BaseClient {
   static void _onData(URLSession session, URLSessionTask task, Data data) {
     final taskTracker = _tracker(task);
     taskTracker.responseController.add(data.bytes);
+    taskTracker.profile?.responseBodySink.add(data.bytes);
   }
 
   static URLRequest? _onRedirect(URLSession session, URLSessionTask task,
@@ -180,6 +184,10 @@ class CupertinoClient extends BaseClient {
     ++taskTracker.numRedirects;
     if (taskTracker.request.followRedirects &&
         taskTracker.numRedirects <= taskTracker.request.maxRedirects) {
+      taskTracker.profile?.responseData.addRedirect(HttpProfileRedirectData(
+          statusCode: response.statusCode,
+          method: request.httpMethod,
+          location: request.url!.toString()));
       return request;
     }
     return null;
@@ -250,31 +258,64 @@ class CupertinoClient extends BaseClient {
 
     final stream = request.finalize();
 
+    final profile = HttpClientRequestProfile.profile(
+        requestStartTimestamp: DateTime.now(),
+        requestMethod: request.method,
+        requestUri: request.url.toString());
+    profile?.requestData
+      ?..connectionInfo = {
+        'package': 'package:cupertino_http', // XXX Set for http_impl.dart
+        'client': 'CupertinoClient',
+        'configuration': _urlSession!.configuration.toString(),
+      }
+      ..contentLength = request.contentLength
+      ..cookies = request.headers['cookie']
+      ..followRedirects = request.followRedirects
+      ..headers = request.headers
+      ..maxRedirects = request.maxRedirects;
+
+    // XXX It would be cool to have a "other stuff field" to stick in task
+    // data.
     final urlRequest = MutableURLRequest.fromUrl(request.url)
       ..httpMethod = request.method;
 
     if (request is Request) {
       // Optimize the (typical) `Request` case since assigning to
       // `httpBodyStream` requires a lot of expensive setup and data passing.
       urlRequest.httpBody = Data.fromList(request.bodyBytes);
+      profile?.requestBodySink.add(request.bodyBytes);
     } else if (await _hasData(stream) case (true, final s)) {
       // If the request is supposed to be bodyless (e.g. GET requests)
       // then setting `httpBodyStream` will cause the request to fail -
       // even if the stream is empty.
-      urlRequest.httpBodyStream = s;
+      if (profile == null) {
+        urlRequest.httpBodyStream = s;
+      } else {
+        final splitter = StreamSplitter(s);
+        urlRequest.httpBodyStream = splitter.split();
+        unawaited(profile.requestBodySink.addStream(splitter.split()));
+      }
     }
 
     // This will preserve Apple default headers - is that what we want?
     request.headers.forEach(urlRequest.setValueForHttpHeaderField);
 
     final task = urlSession.dataTaskWithRequest(urlRequest);
-    final taskTracker = _TaskTracker(request);
+    final taskTracker = _TaskTracker(request, profile);
     _tasks[task] = taskTracker;
     task.resume();
 
     final maxRedirects = request.followRedirects ? request.maxRedirects : 0;
 
-    final result = await taskTracker.responseCompleter.future;
+    late URLResponse result;
+    try {
+      result = await taskTracker.responseCompleter.future;
+    } catch (e) {
+      profile?.requestData.error = e.toString();
+      rethrow;
+    }
+
+    profile?.requestEndTimestamp = DateTime.now(); // Not a timestamp!
     final response = result as HTTPURLResponse;
 
     if (request.followRedirects && taskTracker.numRedirects > maxRedirects) {
@@ -292,6 +333,15 @@ class CupertinoClient extends BaseClient {
       );
     }
 
+    final isRedirect = !request.followRedirects && taskTracker.numRedirects > 0;
+    profile?.responseData // Good name?
+      ?..cookies = responseHeaders['cookies']
+      ..headers = responseHeaders
+      ..isRedirect = isRedirect
+      ..reasonPhrase = _findReasonPhrase(response.statusCode)
+      ..startTime = DateTime.now()
+      ..statusCode = response.statusCode;
+
     return StreamedResponse(
       taskTracker.responseController.stream,
       response.statusCode,
@@ -300,7 +350,7 @@ class CupertinoClient extends BaseClient {
           : response.expectedContentLength,
       reasonPhrase: _findReasonPhrase(response.statusCode),
       request: request,
-      isRedirect: !request.followRedirects && taskTracker.numRedirects > 0,
+      isRedirect: isRedirect,
       headers: responseHeaders,
     );
   }

pkgs/cupertino_http/pubspec.yaml

@@ -15,6 +15,8 @@ dependencies:
   flutter:
     sdk: flutter
   http: '>=0.13.4 <2.0.0'
+  http_profile:
+    path: ../http_profile
 
 dev_dependencies:
   dart_flutter_team_lints: ^1.0.0

pkgs/http_profile/lib/http_profile.dart

@@ -93,27 +93,49 @@ final class HttpProfileRequestData {
   }
 
   /// The content length of the request, in bytes.
-  set contentLength(int value) {
-    _data['contentLength'] = value;
+  set contentLength(int? value) {
+    if (value == null) {
+      _data.remove('contentLength');
+    } else {
+      _data['contentLength'] = value;
+    }
     _updated();
   }
 
   /// The cookies presented to the server (in the 'cookie' header).
   ///
+  /// XXX Can have multiple values in the same item.
   /// Usage example:
   ///
   /// ```dart
   /// profile.requestData.cookies = [
   ///   'sessionId=abc123',
   ///   'csrftoken=def456',
   /// ];
+  ///
+  /// or
+  ///
+  /// profile.requestData.cookies = ['sessionId=abc123,csrftoken=def456']
   /// ```
-  set cookies(List<String> value) {
+  set cookiesList(List<String>? value) {
+    if (value == null) {
+      _data.remove('cookies');
+    }
     _data['cookies'] = [...value];
     _updated();
   }
 
+  set cookies(String value) {
+    if (value == null) {
+      _data.remove('cookies');
+    }
+    _data['cookies'] = ",".split(RegExp(r'\s*,\s*'))
+    _updated();
+  }
+
   /// The error associated with a failed request.
+  ///
+  /// Should this be here? It doesn't just seem asssociated with the request.
   set error(String value) {
     _data['error'] = value;
     _updated();
@@ -125,11 +147,24 @@ final class HttpProfileRequestData {
     _updated();
   }
 
-  set headers(Map<String, List<String>> value) {
+  set headersValueList(Map<String, List<String>>? value) {
+    if (value == null) {
+      _data.remove('headers');
+    }
     _data['headers'] = {...value};
     _updated();
   }
 
+  /// XXX should this include cookies or not? It's not obvious why we seperate
+  /// cookies from other headers.
+  set headers(Map<String, String>? value) {
+    if (value == null) {
+      _data.remove('headers');
+    }
+    _data['headers'] = {for (var entry in value.entries) entry.key : entry.value.split(RegExp(r'\s*,\s*'))};
+    _updated();
+  }
+
   /// The maximum number of redirects allowed during the request.
   set maxRedirects(int value) {
     _data['maxRedirects'] = value;
@@ -188,6 +223,9 @@ final class HttpProfileResponseData {
   /// It can contain any arbitrary data as long as the values are of type
   /// [String] or [int]. For example:
   /// { 'localPort': 1285, 'remotePort': 443, 'connectionPoolId': '21x23' }
+  /// 
+  /// XXX what is the difference between the connection info in the request
+  /// and the response? Don't they use the same connection?
   set connectionInfo(Map<String, dynamic /*String|int*/ > value) {
     for (final v in value.values) {
       if (!(v is String || v is int)) {
@@ -214,7 +252,10 @@ final class HttpProfileResponseData {
     _updated();
   }
 
-  set reasonPhrase(String value) {
+  set reasonPhrase(String? value) {
+    if (value == null) {
+      _data.remove('reasonPhrase');
+    }
     _data['reasonPhrase'] = value;
     _updated();
   }
@@ -250,12 +291,17 @@ final class HttpProfileResponseData {
 
   /// The time at which the response was completed. Note that DevTools will not
   /// consider the request to be complete until [endTime] is non-null.
+  ///
+  /// This means that all the data has been received, right?
   set endTime(DateTime value) {
     _data['endTime'] = value.microsecondsSinceEpoch;
     _updated();
   }
 
   /// The error associated with a failed request.
+  ///
+  /// This doesn't seem to be just set for failures. For HttpClient,
+  /// finishResponseWithError('Connection was upgraded')
   set error(String value) {
     _data['error'] = value;
     _updated();
@@ -297,6 +343,10 @@ final class HttpClientRequestProfile {
   /// The time at which the request was completed. Note that DevTools will not
   /// consider the request to be complete until [requestEndTimestamp] is
   /// non-null.
+  ///
+  /// What does this mean? Is it when the response data first arrives? Or
+  /// after the initial request data has been sent? This matters because do
+  /// redirects count as part of the request time?
   set requestEndTimestamp(DateTime value) {
     _data['requestEndTimestamp'] = value.microsecondsSinceEpoch;
     _updated();

pkgs/http_profile/pubspec.yaml

@@ -8,8 +8,5 @@ repository: https://github.com/dart-lang/http/tree/master/pkgs/http_profile
 environment:
   sdk: ^3.0.0
 
-dependencies:
-  test: ^1.24.9
-
 dev_dependencies:
   dart_flutter_team_lints: ^2.1.1