From 206a7ffbe273c21f8bdbfb744417c6b6a1c1e608 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Tue, 14 Nov 2023 13:10:15 -0500 Subject: [PATCH 01/26] Populate package:http_profile --- pkgs/http_profile/lib/http_profile.dart | 308 +++++++++++++++++++++++- 1 file changed, 301 insertions(+), 7 deletions(-) diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index ea27665fb1..98fc2fcf2b 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -2,8 +2,208 @@ // 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' show StreamController, StreamSink; +import 'dart:developer' show addHttpClientProfilingData, Timeline; import 'dart:io'; +/// Describes an event related to an HTTP request. +final class HttpProfileRequestEvent { + final int _timestamp; + final String _name; + + /// [timestamp] should be the time at which the event occurred, as a + /// microsecond value on the monotonic clock used by the [Timeline]. + HttpProfileRequestEvent({required int timestamp, required String name}) + : _timestamp = timestamp, + _name = name; + + Map _toJson() => { + 'timestamp': _timestamp, + 'event': _name, + }; +} + +/// Describes proxy authentication details associated with an HTTP request. +final class HttpProfileProxyData { + final String? _host; + final String? _username; + final bool? _isDirect; + final int? _port; + + HttpProfileProxyData( + {String? host, String? username, bool? isDirect, int? port}) + : _host = host, + _username = username, + _isDirect = isDirect, + _port = port; + + Map _toJson() => { + if (_host != null) 'host': _host, + if (_username != null) 'username': _username, + if (_isDirect != null) 'isDirect': _isDirect, + if (_port != null) 'port': _port, + }; +} + +/// Describes details about an HTTP request. +final class HttpProfileRequestData { + final Map _data; + + final void Function() _updated; + + /// The elements of [connectionInfo] can either be [String]s or [int]s. + set connectionInfo(Map value) { + _data['connectionInfo'] = value; + _updated(); + } + + /// The content length of the request, in bytes. + set contentLength(int value) { + _data['contentLength'] = value; + _updated(); + } + + /// The cookies presented to the server (in the 'cookie' header). + set cookies(List value) { + _data['cookies'] = value; + _updated(); + } + + /// The error associated with the failed request. + set error(String value) { + _data['error'] = value; + _updated(); + } + + /// Whether redirects were followed automatically. + set followRedirects(bool value) { + _data['followRedirects'] = value; + _updated(); + } + + set headers(Map> value) { + _data['headers'] = value; + _updated(); + } + + /// If [followRedirects] is true, this is the maximum number of redirects that + /// were followed. + set maxRedirects(int value) { + _data['maxRedirects'] = value; + _updated(); + } + + /// The requested persistent connection state. + set persistentConnection(bool value) { + _data['persistentConnection'] = value; + _updated(); + } + + /// Proxy authentication details for the request. + set proxyDetails(HttpProfileProxyData value) { + _data['proxyDetails'] = value._toJson(); + _updated(); + } + + HttpProfileRequestData._( + Map this._data, void Function() this._updated); +} + +/// Describes details about a response to an HTTP request. +final class HttpProfileResponseData { + final Map _data; + + final void Function() _updated; + + /// Records a redirect that the connection went through. The elements of + /// [redirect] can either be [String]s or [int]s. + void addRedirect(Map redirect) { + _data['redirects'].add(redirect); + _updated(); + } + + /// The cookies set by the server (from the 'set-cookie' header). + set cookies(List value) { + _data['cookies'] = value; + _updated(); + } + + /// The elements of [connectionInfo] can either be [String]s or [int]s. + set connectionInfo(Map value) { + _data['connectionInfo'] = value; + _updated(); + } + + set headers(Map> value) { + _data['headers'] = value; + _updated(); + } + + // The compression state of the response. + // + // This specifies whether the response bytes were compressed when they were + // received across the wire and whether callers will receive compressed or + // uncompressed bytes when they listen to the response body byte stream. + set compressionState(String value) { + _data['compressionState'] = value; + _updated(); + } + + set reasonPhrase(String value) { + _data['reasonPhrase'] = value; + _updated(); + } + + /// Whether the status code was one of the normal redirect codes. + set isRedirect(bool value) { + _data['isRedirect'] = value; + _updated(); + } + + /// The persistent connection state returned by the server. + set persistentConnection(bool value) { + _data['persistentConnection'] = value; + _updated(); + } + + /// The content length of the response body, in bytes. + set contentLength(int value) { + _data['contentLength'] = value; + _updated(); + } + + set statusCode(int value) { + _data['statusCode'] = value; + _updated(); + } + + /// The time at which the initial response was received, as a microsecond + /// value on the monotonic clock used by the [Timeline]. + set startTime(int value) { + _data['startTime'] = value; + _updated(); + } + + /// The time at which the response was completed, as a microsecond value on + /// the monotonic clock used by the [Timeline]. Note that DevTools will not + /// consider the request to be complete until [endTime] is non-null. + set endTime(int value) { + _data['endTime'] = value; + _updated(); + } + + /// The error associated with the failed request. + set error(String value) { + _data['error'] = value; + _updated(); + } + + HttpProfileResponseData._( + Map this._data, void Function() this._updated) { + _data['redirects'] = >[]; + } +} + /// A record of debugging information about an HTTP request. final class HttpClientRequestProfile { /// Whether HTTP profiling is enabled or not. @@ -14,20 +214,114 @@ final class HttpClientRequestProfile { static set profilingEnabled(bool enabled) => HttpClient.enableTimelineLogging = enabled; - String? requestMethod; - String? requestUri; + final _data = {}; + + /// The ID of the isolate the request was issued from. + String? get isolateId => _data['isolateId'] as String?; + set isolateId(String? value) { + _data['isolateId'] = value; + _updated(); + } + + /// The HTTP request method associated with the request. + String? get requestMethod => _data['requestMethod'] as String?; + set requestMethod(String? value) { + _data['requestMethod'] = value; + _updated(); + } + + /// The URI to which the request was sent. + String? get requestUri => _data['requestUri'] as String?; + set requestUri(String? value) { + _data['requestUri'] = value; + _updated(); + } + + /// Records an event related to the request. + /// + /// Usage example: + /// + /// ```dart + /// profile.addEvent(HttpProfileRequestEvent(Timeline.now, "Connection Established"); + /// profile.addEvent(HttpProfileRequestEvent(Timeline.now, "Remote Disconnected"); + /// ``` + void addEvent(HttpProfileRequestEvent event) { + _data['events'].add(event._toJson()); + _updated(); + } + + /// The time at which the request was initiated, as a microsecond value on the + /// monotonic clock used by the [Timeline]. + int? get requestStartTimestamp => _data['requestStartTimestamp'] as int?; + set requestStartTimestamp(int? value) { + _data['requestStartTimestamp'] = value; + _updated(); + } + + /// The time at which the request was completed, as a microsecond value on the + /// monotonic clock used by the [Timeline]. Note that DevTools will not + /// consider the request to be complete until [requestEndTimestamp] is + /// non-null. + int? get requestEndTimestamp => _data['requestEndTimestamp'] as int?; + set requestEndTimestamp(int? value) { + _data['requestEndTimestamp'] = value; + _updated(); + } - HttpClientRequestProfile._(); + /// Details about the request. + late final HttpProfileRequestData requestData; + + final StreamController> _requestBody = + StreamController>(); + + /// The body of the request. + StreamSink> get requestBodySink { + _updated(); + return _requestBody.sink; + } + + /// Details about the response. + late final HttpProfileResponseData responseData; + + final StreamController> _responseBody = + StreamController>(); + + /// The body of the response. + StreamSink> get responseBodySink { + _updated(); + return _responseBody.sink; + } + + void _updated() => _data['_lastUpdateTime'] = Timeline.now; + + HttpClientRequestProfile._() { + _data['events'] = >[]; + _data['requestData'] = {}; + requestData = HttpProfileRequestData._( + _data['requestData'] as Map, _updated); + _data['responseData'] = {}; + responseData = HttpProfileResponseData._( + _data['responseData'] as Map, _updated); + _data['_requestBodyStream'] = _requestBody.stream; + _data['_responseBodyStream'] = _responseBody.stream; + // This entry is needed to support the updatedSince parameter of + // ext.dart.io.getHttpProfile. + _data['_lastUpdateTime'] = 0; + } - /// If HTTP profiling is enabled, returns - /// a [HttpClientRequestProfile] otherwise returns `null`. + /// If HTTP profiling is enabled, returns an [HttpClientRequestProfile], + /// otherwise returns `null`. static HttpClientRequestProfile? profile() { - // Always return `null` in product mode so that the - // profiling code can be tree shaken away. + // Always return `null` in product mode so that the profiling code can be + // tree shaken away. if (const bool.fromEnvironment('dart.vm.product') || !profilingEnabled) { return null; } final requestProfile = HttpClientRequestProfile._(); + // This entry is needed to support the id parameter of + // ext.dart.io.getHttpProfileRequest. + requestProfile._data['id'] = + 'from_package/${addHttpClientProfilingData(requestProfile._data)}'; return requestProfile; } } From 6618444fe6cc93a02f7acdd959fe5bba47f8e0c4 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Mon, 4 Dec 2023 17:18:07 -0500 Subject: [PATCH 02/26] Address comments --- pkgs/http_profile/lib/http_profile.dart | 144 +++++++++++++++--------- 1 file changed, 93 insertions(+), 51 deletions(-) diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index 98fc2fcf2b..dd1795cc56 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -11,10 +11,8 @@ final class HttpProfileRequestEvent { final int _timestamp; final String _name; - /// [timestamp] should be the time at which the event occurred, as a - /// microsecond value on the monotonic clock used by the [Timeline]. - HttpProfileRequestEvent({required int timestamp, required String name}) - : _timestamp = timestamp, + HttpProfileRequestEvent({required DateTime timestamp, required String name}) + : _timestamp = timestamp.microsecondsSinceEpoch, _name = name; Map _toJson() => { @@ -30,9 +28,12 @@ final class HttpProfileProxyData { final bool? _isDirect; final int? _port; - HttpProfileProxyData( - {String? host, String? username, bool? isDirect, int? port}) - : _host = host, + HttpProfileProxyData({ + String? host, + String? username, + bool? isDirect, + int? port, + }) : _host = host, _username = username, _isDirect = isDirect, _port = port; @@ -45,15 +46,48 @@ final class HttpProfileProxyData { }; } +/// Describes a redirect that an HTTP connection went through. +class HttpProfileRedirectData { + int _statusCode; + String _method; + String _location; + + HttpProfileRedirectData({ + required int statusCode, + required String method, + required String location, + }) : _statusCode = statusCode, + _method = method, + _location = location; + + Map _toJson() => { + 'statusCode': _statusCode, + 'method': _method, + 'location': _location, + }; +} + /// Describes details about an HTTP request. final class HttpProfileRequestData { final Map _data; final void Function() _updated; - /// The elements of [connectionInfo] can either be [String]s or [int]s. + /// Information about the networking connection used in the HTTP request. + /// + /// This information is meant to be used for debugging. + /// + /// 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' } set connectionInfo(Map value) { - _data['connectionInfo'] = value; + for (final v in value.values) { + if (!(v is String || v is int)) { + throw ArgumentError( + "The values in connectionInfo must be of type String or int."); + } + } + _data['connectionInfo'] = {...value}; _updated(); } @@ -64,30 +98,29 @@ final class HttpProfileRequestData { } /// The cookies presented to the server (in the 'cookie' header). - set cookies(List value) { - _data['cookies'] = value; + set cookies(List value) { + _data['cookies'] = [for (final cookie in value) cookie.toString()]; _updated(); } - /// The error associated with the failed request. + /// The error associated with a failed request. set error(String value) { _data['error'] = value; _updated(); } - /// Whether redirects were followed automatically. + /// Whether automatic redirect following was enabled for the request. set followRedirects(bool value) { _data['followRedirects'] = value; _updated(); } set headers(Map> value) { - _data['headers'] = value; + _data['headers'] = {...value}; _updated(); } - /// If [followRedirects] is true, this is the maximum number of redirects that - /// were followed. + /// The maximum number of redirects allowed during the request. set maxRedirects(int value) { _data['maxRedirects'] = value; _updated(); @@ -105,8 +138,10 @@ final class HttpProfileRequestData { _updated(); } - HttpProfileRequestData._( - Map this._data, void Function() this._updated); + const HttpProfileRequestData._( + Map this._data, + void Function() this._updated, + ); } /// Describes details about a response to an HTTP request. @@ -115,27 +150,38 @@ final class HttpProfileResponseData { final void Function() _updated; - /// Records a redirect that the connection went through. The elements of - /// [redirect] can either be [String]s or [int]s. - void addRedirect(Map redirect) { - _data['redirects'].add(redirect); + /// Records a redirect that the connection went through. + void addRedirect(HttpProfileRedirectData redirect) { + _data['redirects'].add(redirect._toJson()); _updated(); } /// The cookies set by the server (from the 'set-cookie' header). - set cookies(List value) { - _data['cookies'] = value; + set cookies(List value) { + _data['cookies'] = [for (final cookie in value) cookie.toString()]; _updated(); } - /// The elements of [connectionInfo] can either be [String]s or [int]s. + /// Information about the networking connection used in the HTTP response. + /// + /// This information is meant to be used for debugging. + /// + /// 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' } set connectionInfo(Map value) { - _data['connectionInfo'] = value; + for (final v in value.values) { + if (!(v is String || v is int)) { + throw ArgumentError( + "The values in connectionInfo must be of type String or int."); + } + } + _data['connectionInfo'] = {...value}; _updated(); } set headers(Map> value) { - _data['headers'] = value; + _data['headers'] = {...value}; _updated(); } @@ -144,8 +190,8 @@ final class HttpProfileResponseData { // This specifies whether the response bytes were compressed when they were // received across the wire and whether callers will receive compressed or // uncompressed bytes when they listen to the response body byte stream. - set compressionState(String value) { - _data['compressionState'] = value; + set compressionState(HttpClientResponseCompressionState value) { + _data['compressionState'] = value.name; _updated(); } @@ -177,29 +223,29 @@ final class HttpProfileResponseData { _updated(); } - /// The time at which the initial response was received, as a microsecond - /// value on the monotonic clock used by the [Timeline]. - set startTime(int value) { - _data['startTime'] = value; + /// The time at which the initial response was received. + set startTime(DateTime value) { + _data['startTime'] = value.microsecondsSinceEpoch; _updated(); } - /// The time at which the response was completed, as a microsecond value on - /// the monotonic clock used by the [Timeline]. Note that DevTools will not + /// The time at which the response was completed. Note that DevTools will not /// consider the request to be complete until [endTime] is non-null. - set endTime(int value) { - _data['endTime'] = value; + set endTime(DateTime value) { + _data['endTime'] = value.microsecondsSinceEpoch; _updated(); } - /// The error associated with the failed request. + /// The error associated with a failed request. set error(String value) { _data['error'] = value; _updated(); } HttpProfileResponseData._( - Map this._data, void Function() this._updated) { + Map this._data, + void Function() this._updated, + ) { _data['redirects'] = >[]; } } @@ -242,29 +288,25 @@ final class HttpClientRequestProfile { /// Usage example: /// /// ```dart - /// profile.addEvent(HttpProfileRequestEvent(Timeline.now, "Connection Established"); - /// profile.addEvent(HttpProfileRequestEvent(Timeline.now, "Remote Disconnected"); + /// profile.addEvent(HttpProfileRequestEvent(DateTime.now(), "Connection Established"); + /// profile.addEvent(HttpProfileRequestEvent(DateTime.now(), "Remote Disconnected"); /// ``` void addEvent(HttpProfileRequestEvent event) { _data['events'].add(event._toJson()); _updated(); } - /// The time at which the request was initiated, as a microsecond value on the - /// monotonic clock used by the [Timeline]. - int? get requestStartTimestamp => _data['requestStartTimestamp'] as int?; - set requestStartTimestamp(int? value) { - _data['requestStartTimestamp'] = value; + /// The time at which the request was initiated. + set requestStartTimestamp(DateTime value) { + _data['requestStartTimestamp'] = value.microsecondsSinceEpoch; _updated(); } - /// The time at which the request was completed, as a microsecond value on the - /// monotonic clock used by the [Timeline]. Note that DevTools will not + /// The time at which the request was completed. Note that DevTools will not /// consider the request to be complete until [requestEndTimestamp] is /// non-null. - int? get requestEndTimestamp => _data['requestEndTimestamp'] as int?; - set requestEndTimestamp(int? value) { - _data['requestEndTimestamp'] = value; + set requestEndTimestamp(DateTime value) { + _data['requestEndTimestamp'] = value.microsecondsSinceEpoch; _updated(); } From 5d8b3b8282d5a5764ad1bca98cd2e2d5a7263e13 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Tue, 5 Dec 2023 11:25:42 -0500 Subject: [PATCH 03/26] Add examples showing how to use the `cookies` setters --- pkgs/http_profile/lib/http_profile.dart | 26 +++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index dd1795cc56..f3adc2a91f 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -98,8 +98,17 @@ final class HttpProfileRequestData { } /// The cookies presented to the server (in the 'cookie' header). - set cookies(List value) { - _data['cookies'] = [for (final cookie in value) cookie.toString()]; + /// + /// Usage example: + /// + /// ```dart + /// profile.requestData.cookies = [ + /// 'sessionId=abc123', + /// 'csrftoken=def456', + /// ]; + /// ``` + set cookies(List value) { + _data['cookies'] = [...value]; _updated(); } @@ -157,8 +166,17 @@ final class HttpProfileResponseData { } /// The cookies set by the server (from the 'set-cookie' header). - set cookies(List value) { - _data['cookies'] = [for (final cookie in value) cookie.toString()]; + /// + /// Usage example: + /// + /// ```dart + /// profile.responseData.cookies = [ + /// 'sessionId=abc123', + /// 'id=def456; Max-Age=2592000; Domain=example.com', + /// ]; + /// ``` + set cookies(List value) { + _data['cookies'] = [...value]; _updated(); } From 359a1568547994401ffbd11af7fcd751e5f8d8b2 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Tue, 12 Dec 2023 14:19:16 -0500 Subject: [PATCH 04/26] Update --- pkgs/http_profile/lib/http_profile.dart | 64 ++++++++++++------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index f3adc2a91f..a980d02e7f 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -3,8 +3,9 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async' show StreamController, StreamSink; -import 'dart:developer' show addHttpClientProfilingData, Timeline; -import 'dart:io'; +import 'dart:developer' show addHttpClientProfilingData, Service, Timeline; +import 'dart:io' show HttpClient, HttpClientResponseCompressionState; +import 'dart:isolate' show Isolate; /// Describes an event related to an HTTP request. final class HttpProfileRequestEvent { @@ -280,27 +281,6 @@ final class HttpClientRequestProfile { final _data = {}; - /// The ID of the isolate the request was issued from. - String? get isolateId => _data['isolateId'] as String?; - set isolateId(String? value) { - _data['isolateId'] = value; - _updated(); - } - - /// The HTTP request method associated with the request. - String? get requestMethod => _data['requestMethod'] as String?; - set requestMethod(String? value) { - _data['requestMethod'] = value; - _updated(); - } - - /// The URI to which the request was sent. - String? get requestUri => _data['requestUri'] as String?; - set requestUri(String? value) { - _data['requestUri'] = value; - _updated(); - } - /// Records an event related to the request. /// /// Usage example: @@ -314,12 +294,6 @@ final class HttpClientRequestProfile { _updated(); } - /// The time at which the request was initiated. - set requestStartTimestamp(DateTime value) { - _data['requestStartTimestamp'] = value.microsecondsSinceEpoch; - _updated(); - } - /// The time at which the request was completed. Note that DevTools will not /// consider the request to be complete until [requestEndTimestamp] is /// non-null. @@ -354,7 +328,16 @@ final class HttpClientRequestProfile { void _updated() => _data['_lastUpdateTime'] = Timeline.now; - HttpClientRequestProfile._() { + HttpClientRequestProfile._({ + required DateTime requestStartTimestamp, + required String requestMethod, + required String requestUri, + }) { + _data['isolateId'] = Service.getIsolateId(Isolate.current)!; + _data['requestStartTimestamp'] = + requestStartTimestamp.microsecondsSinceEpoch; + _data['requestMethod'] = requestMethod; + _data['requestUri'] = requestUri; _data['events'] = >[]; _data['requestData'] = {}; requestData = HttpProfileRequestData._( @@ -366,22 +349,35 @@ final class HttpClientRequestProfile { _data['_responseBodyStream'] = _responseBody.stream; // This entry is needed to support the updatedSince parameter of // ext.dart.io.getHttpProfile. - _data['_lastUpdateTime'] = 0; + _data['_lastUpdateTime'] = Timeline.now; } /// If HTTP profiling is enabled, returns an [HttpClientRequestProfile], /// otherwise returns `null`. - static HttpClientRequestProfile? profile() { + static HttpClientRequestProfile? profile({ + /// The time at which the request was initiated. + required DateTime requestStartTimestamp, + + /// The HTTP request method associated with the request. + required String requestMethod, + + /// The URI to which the request was sent. + required String requestUri, + }) { // Always return `null` in product mode so that the profiling code can be // tree shaken away. if (const bool.fromEnvironment('dart.vm.product') || !profilingEnabled) { return null; } - final requestProfile = HttpClientRequestProfile._(); + final requestProfile = HttpClientRequestProfile._( + requestStartTimestamp: requestStartTimestamp, + requestMethod: requestMethod, + requestUri: requestUri, + ); // This entry is needed to support the id parameter of // ext.dart.io.getHttpProfileRequest. requestProfile._data['id'] = - 'from_package/${addHttpClientProfilingData(requestProfile._data)}'; + addHttpClientProfilingData(requestProfile._data); return requestProfile; } } From d7f3d95de1a03c24bd40cb251ebb7d68adbcb53f Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 14 Dec 2023 08:10:50 -0800 Subject: [PATCH 05/26] Integrate cupertino_http with http_profile --- pkgs/cupertino_http/example/pubspec.yaml | 2 +- .../lib/src/cupertino_client.dart | 62 +++++++++++++++++-- pkgs/cupertino_http/pubspec.yaml | 2 + pkgs/http_profile/lib/http_profile.dart | 60 ++++++++++++++++-- pkgs/http_profile/pubspec.yaml | 3 - 5 files changed, 114 insertions(+), 15 deletions(-) diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index be727312a2..eeab4843f6 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/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 diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index a4c7715de5..71176a306f 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/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(); final BaseRequest request; final responseController = StreamController(); + 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,6 +258,24 @@ 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; @@ -257,24 +283,39 @@ class CupertinoClient extends BaseClient { // 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, ); } diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index 9d639a8450..97655e66f7 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/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 diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index a980d02e7f..d5770b64d5 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -93,13 +93,18 @@ 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 @@ -107,13 +112,30 @@ final class HttpProfileRequestData { /// 'sessionId=abc123', /// 'csrftoken=def456', /// ]; + /// + /// or + /// + /// profile.requestData.cookies = ['sessionId=abc123,csrftoken=def456'] /// ``` - set cookies(List value) { + set cookiesList(List? 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> value) { + set headersValueList(Map>? 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? 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 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(); diff --git a/pkgs/http_profile/pubspec.yaml b/pkgs/http_profile/pubspec.yaml index 9e3b69e12f..1ada043a89 100644 --- a/pkgs/http_profile/pubspec.yaml +++ b/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 From 1fd25f0f03519f8d0319065c68dc0ccc97ec5639 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 14 Dec 2023 08:28:06 -0800 Subject: [PATCH 06/26] Add note --- 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 71176a306f..e9d2a4e7c4 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -266,6 +266,8 @@ class CupertinoClient extends BaseClient { ?..connectionInfo = { 'package': 'package:cupertino_http', // XXX Set for http_impl.dart 'client': 'CupertinoClient', + // XXX this is a long string, maybe better to break it out like: + // "config.allowsCellularAccess": "true" 'configuration': _urlSession!.configuration.toString(), } ..contentLength = request.contentLength From 603d783f4827caf1f187f86dcb70a4e2e6bec7ec Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Fri, 23 Feb 2024 11:16:19 -0800 Subject: [PATCH 07/26] Update --- pkgs/cronet_http/.idea/workspace.xml | 21 ++++++++ .../example/android/build.gradle | 2 +- .../example/devtools_options.yaml | 3 ++ .../ios/Flutter/AppFrameworkInfo.plist | 2 +- pkgs/cupertino_http/example/ios/Podfile | 2 +- pkgs/cupertino_http/example/ios/Podfile.lock | 8 +-- .../ios/Runner.xcodeproj/project.pbxproj | 9 ++-- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- pkgs/cupertino_http/example/macos/Podfile | 2 +- .../cupertino_http/example/macos/Podfile.lock | 6 +-- .../macos/Runner.xcodeproj/project.pbxproj | 8 +-- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- .../lib/src/cupertino_client.dart | 16 ++++-- pkgs/http_profile/lib/http_profile.dart | 49 ++++++++++++------- 14 files changed, 89 insertions(+), 43 deletions(-) create mode 100644 pkgs/cronet_http/.idea/workspace.xml create mode 100644 pkgs/cupertino_http/example/devtools_options.yaml diff --git a/pkgs/cronet_http/.idea/workspace.xml b/pkgs/cronet_http/.idea/workspace.xml new file mode 100644 index 0000000000..e18b8f2975 --- /dev/null +++ b/pkgs/cronet_http/.idea/workspace.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/pkgs/cupertino_http/example/android/build.gradle b/pkgs/cupertino_http/example/android/build.gradle index 58a8c74b14..713d7f6e6d 100644 --- a/pkgs/cupertino_http/example/android/build.gradle +++ b/pkgs/cupertino_http/example/android/build.gradle @@ -26,6 +26,6 @@ subprojects { project.evaluationDependsOn(':app') } -task clean(type: Delete) { +tasks.register("clean", Delete) { delete rootProject.buildDir } diff --git a/pkgs/cupertino_http/example/devtools_options.yaml b/pkgs/cupertino_http/example/devtools_options.yaml new file mode 100644 index 0000000000..fa0b357c4f --- /dev/null +++ b/pkgs/cupertino_http/example/devtools_options.yaml @@ -0,0 +1,3 @@ +description: This file stores settings for Dart & Flutter DevTools. +documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states +extensions: diff --git a/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist b/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist index 9625e105df..7c56964006 100644 --- a/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist +++ b/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist @@ -21,6 +21,6 @@ CFBundleVersion 1.0 MinimumOSVersion - 11.0 + 12.0 diff --git a/pkgs/cupertino_http/example/ios/Podfile b/pkgs/cupertino_http/example/ios/Podfile index 88359b225f..279576f388 100644 --- a/pkgs/cupertino_http/example/ios/Podfile +++ b/pkgs/cupertino_http/example/ios/Podfile @@ -1,5 +1,5 @@ # Uncomment this line to define a global platform for your project -# platform :ios, '11.0' +# platform :ios, '12.0' # CocoaPods analytics sends network stats synchronously affecting flutter build latency. ENV['COCOAPODS_DISABLE_STATS'] = 'true' diff --git a/pkgs/cupertino_http/example/ios/Podfile.lock b/pkgs/cupertino_http/example/ios/Podfile.lock index 709672b9d7..79e5d5abc6 100644 --- a/pkgs/cupertino_http/example/ios/Podfile.lock +++ b/pkgs/cupertino_http/example/ios/Podfile.lock @@ -20,9 +20,9 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: cupertino_http: 5f8b1161107fe6c8d94a0c618735a033d93fa7db - Flutter: f04841e97a9d0b0a8025694d0796dd46242b2854 - integration_test: a1e7d09bd98eca2fc37aefd79d4f41ad37bdbbe5 + Flutter: e0871f40cf51350855a761d2e70bf5af5b9b5de7 + integration_test: ce0a3ffa1de96d1a89ca0ac26fca7ea18a749ef4 -PODFILE CHECKSUM: ef19549a9bc3046e7bb7d2fab4d021637c0c58a3 +PODFILE CHECKSUM: c4c93c5f6502fe2754f48404d3594bf779584011 -COCOAPODS: 1.11.2 +COCOAPODS: 1.11.3 diff --git a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj index 3f233bb8b9..64fe2b898d 100644 --- a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj +++ b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj @@ -156,7 +156,7 @@ 97C146E61CF9000F007C117D /* Project object */ = { isa = PBXProject; attributes = { - LastUpgradeCheck = 1300; + LastUpgradeCheck = 1510; ORGANIZATIONNAME = ""; TargetAttributes = { 97C146ED1CF9000F007C117D = { @@ -205,6 +205,7 @@ files = ( ); inputPaths = ( + "${TARGET_BUILD_DIR}/${INFOPLIST_PATH}", ); name = "Thin Binary"; outputPaths = ( @@ -342,7 +343,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; MTL_ENABLE_DEBUG_INFO = NO; SDKROOT = iphoneos; SUPPORTED_PLATFORMS = iphoneos; @@ -420,7 +421,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; MTL_ENABLE_DEBUG_INFO = YES; ONLY_ACTIVE_ARCH = YES; SDKROOT = iphoneos; @@ -469,7 +470,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; MTL_ENABLE_DEBUG_INFO = NO; SDKROOT = iphoneos; SUPPORTED_PLATFORMS = iphoneos; diff --git a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme index c87d15a335..5e31d3d342 100644 --- a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme +++ b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme @@ -1,6 +1,6 @@ 0; profile?.responseData // Good name? - ?..cookies = responseHeaders['cookies'] - ..headers = responseHeaders + ?..headers = responseHeaders ..isRedirect = isRedirect ..reasonPhrase = _findReasonPhrase(response.statusCode) ..startTime = DateTime.now() ..statusCode = response.statusCode; +/*..cookies = responseHeaders['cookies']*/ return _StreamedResponseWithUrl( taskTracker.responseController.stream, diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index eb4cd0f8bb..e688eb41f9 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -114,16 +114,12 @@ final class HttpProfileRequestData { /// 'csrftoken=def456', /// ]; /// ``` - set cookies(List value) { - _data['cookies'] = [...value]; - _updated(); - } - - set cookies(String value) { + set cookies(List? value) { if (value == null) { _data.remove('cookies'); + } else { + _data['cookies'] = [...value]; } - _data['cookies'] = ",".split(RegExp(r'\s*,\s*')) _updated(); } @@ -143,21 +139,26 @@ final class HttpProfileRequestData { } set headersValueList(Map>? value) { + _updated(); if (value == null) { _data.remove('headers'); + return; } _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? value) { + _updated(); if (value == null) { _data.remove('headers'); + return; } - _data['headers'] = {for (var entry in value.entries) entry.key : entry.value.split(RegExp(r'\s*,\s*'))}; - _updated(); + _data['headers'] = { + for (var entry in value.entries) + entry.key: entry.value.split(RegExp(r'\s*,\s*')) + }; } /// The maximum number of redirects allowed during the request. @@ -218,7 +219,7 @@ 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 value) { @@ -233,9 +234,27 @@ final class HttpProfileResponseData { _updated(); } - set headers(Map> value) { + set headersValueList(Map>? value) { + _updated(); + if (value == null) { + _data.remove('headers'); + return; + } _data['headers'] = {...value}; + } + + /// XXX should this include cookies or not? It's not obvious why we seperate + /// cookies from other headers. + set headers(Map? value) { _updated(); + if (value == null) { + _data.remove('headers'); + return; + } + _data['headers'] = { + for (var entry in value.entries) + entry.key: entry.value.split(RegExp(r'\s*,\s*')) + }; } // The compression state of the response. @@ -407,7 +426,7 @@ final class HttpClientRequestProfile { _data['_responseBodyStream'] = _responseBody.stream; // This entry is needed to support the updatedSince parameter of // ext.dart.io.getHttpProfile. - _data['_lastUpdateTime'] = Timeline.now; + _updated(); } /// If HTTP profiling is enabled, returns an [HttpClientRequestProfile], @@ -432,10 +451,6 @@ final class HttpClientRequestProfile { requestMethod: requestMethod, requestUri: requestUri, ); - // This entry is needed to support the id parameter of - // ext.dart.io.getHttpProfileRequest. - requestProfile._data['id'] = - addHttpClientProfilingData(requestProfile._data); addHttpClientProfilingData(requestProfile._data); return requestProfile; } From c8ab1496d70d37284d2df924115ae376a39129ee Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 26 Feb 2024 14:06:07 -0800 Subject: [PATCH 08/26] Changes --- .../lib/src/cupertino_client.dart | 33 +- pkgs/http_profile/lib/http_profile.dart | 353 +++++++++++------- .../test/populating_profiles_test.dart | 6 +- .../test/profiling_enabled_test.dart | 4 +- 4 files changed, 232 insertions(+), 164 deletions(-) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index 68352c6f6f..315955e999 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -45,8 +45,7 @@ class _TaskTracker { void close() { responseController.close(); - profile?.responseBodySink.close(); - profile?.responseData.endTime = DateTime.now(); + profile?.responseData.close(); } } @@ -172,13 +171,11 @@ 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); } @@ -193,7 +190,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); + taskTracker.profile?.responseData.bodySink.add(data.bytes); } static URLRequest? _onRedirect(URLSession session, URLSessionTask task, @@ -278,21 +275,18 @@ class CupertinoClient extends BaseClient { final stream = request.finalize(); final profile = HttpClientRequestProfile.profile( - requestStartTimestamp: DateTime.now(), + requestStartTime: DateTime.now(), requestMethod: request.method, requestUri: request.url.toString()); profile?.requestData ?..connectionInfo = { 'package': 'package:cupertino_http', // XXX Set for http_impl.dart 'client': 'CupertinoClient', - // XXX this is a long string, maybe better to break it out like: - // "config.allowsCellularAccess": "true" 'configuration': _urlSession!.configuration.toString(), } ..contentLength = request.contentLength -/* ..cookies = request.headers['cookie'] */ ..followRedirects = request.followRedirects - ..headers = request.headers + ..headersCommaValues = request.headers ..maxRedirects = request.maxRedirects; // XXX It would be cool to have a "other stuff field" to stick in task @@ -304,9 +298,8 @@ class CupertinoClient extends BaseClient { // 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); - profile?.requestBodySink.close(); - profile?.requestEndTimestamp = DateTime.now(); + profile?.requestData.bodySink.add(request.bodyBytes); + profile?.requestData.close(); } 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 - @@ -316,9 +309,9 @@ class CupertinoClient extends BaseClient { } else { final splitter = StreamSplitter(s); urlRequest.httpBodyStream = splitter.split(); - unawaited(profile.requestBodySink.addStream(splitter.split()).then((e) { - profile.requestBodySink.close(); - profile?.requestEndTimestamp = DateTime.now(); + unawaited( + profile.requestData.bodySink.addStream(splitter.split()).then((e) { + profile?.requestData.close(); })); } } @@ -336,11 +329,10 @@ class CupertinoClient extends BaseClient { try { result = await taskTracker.responseCompleter.future; } catch (e) { - profile?.requestData.error = e.toString(); + profile?.responseData.closeWithError(e.toString()); rethrow; } - profile?.requestEndTimestamp = DateTime.now(); // Not a timestamp! final response = result as HTTPURLResponse; if (request.followRedirects && taskTracker.numRedirects > maxRedirects) { @@ -359,13 +351,12 @@ class CupertinoClient extends BaseClient { } final isRedirect = !request.followRedirects && taskTracker.numRedirects > 0; - profile?.responseData // Good name? - ?..headers = responseHeaders + profile?.responseData + ?..headersCommaValues = responseHeaders ..isRedirect = isRedirect ..reasonPhrase = _findReasonPhrase(response.statusCode) ..startTime = DateTime.now() ..statusCode = response.statusCode; -/*..cookies = responseHeaders['cookies']*/ return _StreamedResponseWithUrl( taskTracker.responseController.stream, diff --git a/pkgs/http_profile/lib/http_profile.dart b/pkgs/http_profile/lib/http_profile.dart index e688eb41f9..cd524235f2 100644 --- a/pkgs/http_profile/lib/http_profile.dart +++ b/pkgs/http_profile/lib/http_profile.dart @@ -7,6 +7,54 @@ import 'dart:developer' show Service, addHttpClientProfilingData; import 'dart:io' show HttpClient, HttpClientResponseCompressionState; import 'dart:isolate' show Isolate; +/// "token" as defined in RFC 2616, 2.2 +/// See https://datatracker.ietf.org/doc/html/rfc2616#section-2.2 +const _tokenChars = r"!#$%&'*+\-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`" + 'abcdefghijklmnopqrstuvwxyz|~'; + +/// Splits comma-seperated header values. +var _headerSplitter = RegExp(r'[ \t]*,[ \t]*'); + +/// Splits comma-seperated "Set-Cookie" header values. +/// +/// Set-Cookie strings can contain commas. In particular, the following +/// productions defined in RFC-6265, section 4.1.1: +/// - e.g. "Expires=Sun, 06 Nov 1994 08:49:37 GMT" +/// - e.g. "Path=somepath," +/// - e.g. "AnyString,Really," +/// +/// Some values are ambiguous e.g. +/// "Set-Cookie: lang=en; Path=/foo/" +/// "Set-Cookie: SID=x23" +/// and: +/// "Set-Cookie: lang=en; Path=/foo/,SID=x23" +/// would both be result in `response.headers` => "lang=en; Path=/foo/,SID=x23" +/// +/// The idea behind this regex is that ",=" is more likely to +/// start a new then be part of or . +/// +/// See https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1 +var _setCookieSplitter = RegExp(r'[ \t]*,[ \t]*(?=[' + _tokenChars + r']+=)'); + +/// Splits comma-seperated header values into a [List]. +/// +/// Copied from `package:http`. +Map> _splitHeaderValues(Map headers) { + var headersWithFieldLists = >{}; + headers.forEach((key, value) { + if (!value.contains(',')) { + headersWithFieldLists[key] = [value]; + } else { + if (key == 'set-cookie') { + headersWithFieldLists[key] = value.split(_setCookieSplitter); + } else { + headersWithFieldLists[key] = value.split(_headerSplitter); + } + } + }); + return headersWithFieldLists; +} + /// Describes an event related to an HTTP request. final class HttpProfileRequestEvent { final int _timestamp; @@ -71,9 +119,13 @@ class HttpProfileRedirectData { /// Describes details about an HTTP request. final class HttpProfileRequestData { final Map _data; - + final StreamController> _body = StreamController>(); + bool _isClosed = false; final void Function() _updated; + /// The body of the request. + StreamSink> get bodySink => _body.sink; + /// Information about the networking connection used in the HTTP request. /// /// This information is meant to be used for debugging. @@ -82,6 +134,7 @@ final class HttpProfileRequestData { /// [String] or [int]. For example: /// { 'localPort': 1285, 'remotePort': 443, 'connectionPoolId': '21x23' } set connectionInfo(Map value) { + _checkAndUpdate(); for (final v in value.values) { if (!(v is String || v is int)) { throw ArgumentError( @@ -90,56 +143,37 @@ final class HttpProfileRequestData { } } _data['connectionInfo'] = {...value}; - _updated(); } /// The content length of the request, in bytes. set contentLength(int? value) { + _checkAndUpdate(); 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', - /// ]; - /// ``` - set cookies(List? value) { - if (value == null) { - _data.remove('cookies'); - } else { - _data['cookies'] = [...value]; - } - _updated(); - } - - /// The error associated with a failed request. - /// - /// Should this be here? It doesn't just seem asssociated with the request. - /// The error associated with a failed request. - set error(String value) { - _data['error'] = value; - _updated(); } /// Whether automatic redirect following was enabled for the request. set followRedirects(bool value) { + _checkAndUpdate(); _data['followRedirects'] = value; - _updated(); } - set headersValueList(Map>? value) { - _updated(); + /// The request headers where duplicate headers are represented using a list + /// of values. + /// + /// For example: + /// + /// ```dart + /// // Foo: Bar + /// // Foo: Baz + /// + /// profile?.requestData.headersListValues({'Foo', ['Bar', 'Baz']}); + /// ``` + set headersListValues(Map>? value) { + _checkAndUpdate(); if (value == null) { _data.remove('headers'); return; @@ -147,70 +181,107 @@ final class HttpProfileRequestData { _data['headers'] = {...value}; } - /// XXX should this include cookies or not? It's not obvious why we seperate - /// cookies from other headers. - set headers(Map? value) { - _updated(); + /// The request headers where duplicate headers are represented using + /// comma-seperated list of values. + /// + /// For example: + /// + /// ```dart + /// // Foo: Bar + /// // Foo: Baz + /// + /// profile?.requestData.headersCommaValues({'Foo', 'Bar, Baz']}); + /// ``` + set headersCommaValues(Map? value) { + _checkAndUpdate(); if (value == null) { _data.remove('headers'); return; } - _data['headers'] = { - for (var entry in value.entries) - entry.key: entry.value.split(RegExp(r'\s*,\s*')) - }; + _data['headers'] = _splitHeaderValues(value); } /// The maximum number of redirects allowed during the request. set maxRedirects(int value) { + _checkAndUpdate(); _data['maxRedirects'] = value; - _updated(); } /// The requested persistent connection state. set persistentConnection(bool value) { + _checkAndUpdate(); _data['persistentConnection'] = value; - _updated(); } /// Proxy authentication details for the request. set proxyDetails(HttpProfileProxyData value) { + _checkAndUpdate(); _data['proxyDetails'] = value._toJson(); - _updated(); } - const HttpProfileRequestData._( + HttpProfileRequestData._( this._data, this._updated, ); + + void _checkAndUpdate() { + if (_isClosed) { + throw StateError('HttpProfileResponseData has been closed, no further ' + 'updates are allowed'); + } + _updated(); + } + + /// Signal that the request, including the entire request body, has been + /// sent. + /// + /// [bodySink] will be closed and the fields of [HttpProfileRequestData] will + /// no longer be changeable. + /// + /// [endTime] is the time when the request was fully sent. It defaults to the + /// current time. + void close([DateTime? endTime]) { + _checkAndUpdate(); + _isClosed = true; + bodySink.close(); + _data['requestEndTimestamp'] = + (endTime ?? DateTime.now()).microsecondsSinceEpoch; + } + + /// Signal that sending the request has failed with an error. + /// + /// [bodySink] will be closed and the fields of [HttpProfileRequestData] will + /// no longer be changeable. + /// + /// [value] is a textual description of the error e.g. 'host does not exist'. + /// + /// [endTime] is the time when the error occurred. It defaults to the current + /// time. + void closeWithError(String value, [DateTime? endTime]) { + _checkAndUpdate(); + _isClosed = true; + bodySink.close(); + _data['error'] = value; + _data['requestEndTimestamp'] = + (endTime ?? DateTime.now()).microsecondsSinceEpoch; + } } /// Describes details about a response to an HTTP request. final class HttpProfileResponseData { + bool _isClosed = false; final Map _data; - final void Function() _updated; + final StreamController> _body = StreamController>(); /// Records a redirect that the connection went through. void addRedirect(HttpProfileRedirectData redirect) { + _checkAndUpdate(); (_data['redirects'] as List>).add(redirect._toJson()); - _updated(); } - /// The cookies set by the server (from the 'set-cookie' header). - /// - /// Usage example: - /// - /// ```dart - /// profile.responseData.cookies = [ - /// 'sessionId=abc123', - /// 'id=def456; Max-Age=2592000; Domain=example.com', - /// ]; - /// ``` - set cookies(List value) { - _data['cookies'] = [...value]; - _updated(); - } + /// The body of the request. + StreamSink> get bodySink => _body.sink; /// Information about the networking connection used in the HTTP response. /// @@ -219,10 +290,8 @@ 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 value) { + _checkAndUpdate(); for (final v in value.values) { if (!(v is String || v is int)) { throw ArgumentError( @@ -231,11 +300,21 @@ final class HttpProfileResponseData { } } _data['connectionInfo'] = {...value}; - _updated(); } - set headersValueList(Map>? value) { - _updated(); + /// The reponse headers where duplicate headers are represented using a list + /// of values. + /// + /// For example: + /// + /// ```dart + /// // Foo: Bar + /// // Foo: Baz + /// + /// profile?.requestData.headersListValues({'Foo', ['Bar', 'Baz']}); + /// ``` + set headersListValues(Map>? value) { + _checkAndUpdate(); if (value == null) { _data.remove('headers'); return; @@ -243,18 +322,24 @@ final class HttpProfileResponseData { _data['headers'] = {...value}; } - /// XXX should this include cookies or not? It's not obvious why we seperate - /// cookies from other headers. - set headers(Map? value) { - _updated(); + /// The response headers where duplicate headers are represented using + /// comma-seperated list of values. + /// + /// For example: + /// + /// ```dart + /// // Foo: Bar + /// // Foo: Baz + /// + /// profile?.responseData.headersCommaValues({'Foo', 'Bar, Baz']}); + /// ``` + set headersCommaValues(Map? value) { + _checkAndUpdate(); if (value == null) { _data.remove('headers'); return; } - _data['headers'] = { - for (var entry in value.entries) - entry.key: entry.value.split(RegExp(r'\s*,\s*')) - }; + _data['headers'] = _splitHeaderValues(value); } // The compression state of the response. @@ -263,71 +348,94 @@ final class HttpProfileResponseData { // received across the wire and whether callers will receive compressed or // uncompressed bytes when they listen to the response body byte stream. set compressionState(HttpClientResponseCompressionState value) { + _checkAndUpdate(); _data['compressionState'] = value.name; - _updated(); } + // The reason phrase associated with the response e.g. "OK". set reasonPhrase(String? value) { + _checkAndUpdate(); if (value == null) { _data.remove('reasonPhrase'); } else { _data['reasonPhrase'] = value; } - _updated(); } /// Whether the status code was one of the normal redirect codes. set isRedirect(bool value) { + _checkAndUpdate(); _data['isRedirect'] = value; - _updated(); } /// The persistent connection state returned by the server. set persistentConnection(bool value) { + _checkAndUpdate(); _data['persistentConnection'] = value; - _updated(); } /// The content length of the response body, in bytes. set contentLength(int value) { + _checkAndUpdate(); _data['contentLength'] = value; - _updated(); } set statusCode(int value) { + _checkAndUpdate(); _data['statusCode'] = value; - _updated(); } /// The time at which the initial response was received. set startTime(DateTime value) { + _checkAndUpdate(); _data['startTime'] = value.microsecondsSinceEpoch; - _updated(); } - /// 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; + HttpProfileResponseData._( + this._data, + this._updated, + ) { + _data['redirects'] = >[]; + } + + void _checkAndUpdate() { + if (_isClosed) { + throw StateError('HttpProfileResponseData has been closed, no further ' + 'updates are allowed'); + } _updated(); } - /// The error associated with a failed request. + /// Signal that the response, including the entire response body, has been + /// received. /// - /// This doesn't seem to be just set for failures. For HttpClient, - /// finishResponseWithError('Connection was upgraded') - set error(String value) { - _data['error'] = value; - _updated(); + /// [bodySink] will be closed and the fields of [HttpProfileResponseData] will + /// no longer be changeable. + /// + /// [endTime] is the time when the response was fully received. It defaults + /// to the current time. + void close([DateTime? endTime]) { + _checkAndUpdate(); + _isClosed = true; + bodySink.close(); + _data['endTime'] = (endTime ?? DateTime.now()).microsecondsSinceEpoch; } - HttpProfileResponseData._( - this._data, - this._updated, - ) { - _data['redirects'] = >[]; + /// Signal that receiving the response has failed with an error. + /// + /// [bodySink] will be closed and the fields of [HttpProfileResponseData] will + /// no longer be changeable. + /// + /// [value] is a textual description of the error e.g. 'host does not exist'. + /// + /// [endTime] is the time when the error occurred. It defaults to the current + /// time. + void closeWithError(String value, [DateTime? endTime]) { + _checkAndUpdate(); + _isClosed = true; + bodySink.close(); + _data['error'] = value; + _data['endTime'] = (endTime ?? DateTime.now()).microsecondsSinceEpoch; } } @@ -366,53 +474,22 @@ final class HttpClientRequestProfile { _updated(); } - /// 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(); - } - /// Details about the request. late final HttpProfileRequestData requestData; - final StreamController> _requestBody = - StreamController>(); - - /// The body of the request. - StreamSink> get requestBodySink { - _updated(); - return _requestBody.sink; - } - /// Details about the response. late final HttpProfileResponseData responseData; - final StreamController> _responseBody = - StreamController>(); - - /// The body of the response. - StreamSink> get responseBodySink { - _updated(); - return _responseBody.sink; - } - void _updated() => _data['_lastUpdateTime'] = DateTime.now().microsecondsSinceEpoch; HttpClientRequestProfile._({ - required DateTime requestStartTimestamp, + required DateTime requestStartTime, required String requestMethod, required String requestUri, }) { _data['isolateId'] = Service.getIsolateId(Isolate.current)!; - _data['requestStartTimestamp'] = - requestStartTimestamp.microsecondsSinceEpoch; + _data['requestStartTimestamp'] = requestStartTime.microsecondsSinceEpoch; _data['requestMethod'] = requestMethod; _data['requestUri'] = requestUri; _data['events'] = >[]; @@ -422,8 +499,8 @@ final class HttpClientRequestProfile { _data['responseData'] = {}; responseData = HttpProfileResponseData._( _data['responseData'] as Map, _updated); - _data['_requestBodyStream'] = _requestBody.stream; - _data['_responseBodyStream'] = _responseBody.stream; + _data['_requestBodyStream'] = requestData._body.stream; + _data['_responseBodyStream'] = responseData._body.stream; // This entry is needed to support the updatedSince parameter of // ext.dart.io.getHttpProfile. _updated(); @@ -433,7 +510,7 @@ final class HttpClientRequestProfile { /// otherwise returns `null`. static HttpClientRequestProfile? profile({ /// The time at which the request was initiated. - required DateTime requestStartTimestamp, + required DateTime requestStartTime, /// The HTTP request method associated with the request. required String requestMethod, @@ -447,7 +524,7 @@ final class HttpClientRequestProfile { return null; } final requestProfile = HttpClientRequestProfile._( - requestStartTimestamp: requestStartTimestamp, + requestStartTime: requestStartTime, requestMethod: requestMethod, requestUri: requestUri, ); diff --git a/pkgs/http_profile/test/populating_profiles_test.dart b/pkgs/http_profile/test/populating_profiles_test.dart index a1cbbb4b30..6a69c149bb 100644 --- a/pkgs/http_profile/test/populating_profiles_test.dart +++ b/pkgs/http_profile/test/populating_profiles_test.dart @@ -18,7 +18,7 @@ void main() { HttpClientRequestProfile.profilingEnabled = true; profile = HttpClientRequestProfile.profile( - requestStartTimestamp: DateTime.parse('2024-03-21'), + requestStartTime: DateTime.parse('2024-03-21'), requestMethod: 'GET', requestUri: 'https://www.example.com', )!; @@ -136,7 +136,7 @@ void main() { final requestData = backingMap['requestData'] as Map; expect(requestData['headers'], isNull); - profile.requestData.headers = { + profile.requestData.headersCommaValues = { 'content-length': ['0'], }; @@ -242,7 +242,7 @@ void main() { final responseData = backingMap['responseData'] as Map; expect(responseData['headers'], isNull); - profile.responseData.headers = { + profile.responseData.headersCommaValues = { 'connection': ['keep-alive'], 'cache-control': ['max-age=43200'], 'content-type': ['application/json', 'charset=utf-8'], diff --git a/pkgs/http_profile/test/profiling_enabled_test.dart b/pkgs/http_profile/test/profiling_enabled_test.dart index 3062c79719..7d9b63410f 100644 --- a/pkgs/http_profile/test/profiling_enabled_test.dart +++ b/pkgs/http_profile/test/profiling_enabled_test.dart @@ -13,7 +13,7 @@ void main() { expect(HttpClient.enableTimelineLogging, true); expect( HttpClientRequestProfile.profile( - requestStartTimestamp: DateTime.parse('2024-03-21'), + requestStartTime: DateTime.parse('2024-03-21'), requestMethod: 'GET', requestUri: 'https://www.example.com', ), @@ -26,7 +26,7 @@ void main() { expect(HttpClient.enableTimelineLogging, false); expect( HttpClientRequestProfile.profile( - requestStartTimestamp: DateTime.parse('2024-03-21'), + requestStartTime: DateTime.parse('2024-03-21'), requestMethod: 'GET', requestUri: 'https://www.example.com', ), From 4af722d8c041c52652488d08a8595af60bcdede3 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 21 Mar 2024 13:35:36 -0700 Subject: [PATCH 09/26] Update --- 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 315955e999..396234f116 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -311,7 +311,7 @@ class CupertinoClient extends BaseClient { urlRequest.httpBodyStream = splitter.split(); unawaited( profile.requestData.bodySink.addStream(splitter.split()).then((e) { - profile?.requestData.close(); + profile.requestData.close(); })); } } diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index b22109fae0..c52320d98e 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -4,7 +4,7 @@ description: >- A macOS/iOS Flutter plugin that provides access to the Foundation URL Loading System. repository: https://github.com/dart-lang/http/tree/master/pkgs/cupertino_http - +publish_to: none environment: sdk: ^3.2.0 flutter: '>=3.16.0' # If changed, update test matrix. From 57e367fd1642e0844effa7cf16df47d674948d33 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 21 Mar 2024 13:48:58 -0700 Subject: [PATCH 10/26] Changelog/Pubspec --- pkgs/cupertino_http/CHANGELOG.md | 4 +++- pkgs/cupertino_http/pubspec.yaml | 12 ++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkgs/cupertino_http/CHANGELOG.md b/pkgs/cupertino_http/CHANGELOG.md index 34a36c59a4..1329b74990 100644 --- a/pkgs/cupertino_http/CHANGELOG.md +++ b/pkgs/cupertino_http/CHANGELOG.md @@ -1,4 +1,6 @@ -## 1.3.1-wip +## 1.5.0-wip + +* Add integration to the DevTools "Network" tab. ## 1.3.0 diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index f41d8a64b4..c336a70b48 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -1,15 +1,14 @@ name: cupertino_http -version: 1.4.0-wip -publish_to: none # Do not merge with this here! - +version: 1.5.0-wip description: >- A macOS/iOS Flutter plugin that provides access to the Foundation URL Loading System. repository: https://github.com/dart-lang/http/tree/master/pkgs/cupertino_http publish_to: none + environment: - sdk: ^3.3.0 - flutter: '>=3.19.0' # If changed, update test matrix. + sdk: ^3.2.0 # XXX Wrong! + flutter: '>=3.16.0' # XXX Wrong! dependencies: async: ^2.5.0 @@ -17,13 +16,10 @@ dependencies: flutter: sdk: flutter http: ^1.2.0 -<<<<<<< HEAD http_profile: path: ../http_profile -======= web_socket: path: ../web_socket ->>>>>>> upstream/master dev_dependencies: dart_flutter_team_lints: ^2.0.0 From 618f47b5ead5fc68dfa364a269211b78077e9cd6 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 21 Mar 2024 13:50:31 -0700 Subject: [PATCH 11/26] Update CHANGELOG.md --- pkgs/cupertino_http/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/cupertino_http/CHANGELOG.md b/pkgs/cupertino_http/CHANGELOG.md index 1329b74990..6845b67881 100644 --- a/pkgs/cupertino_http/CHANGELOG.md +++ b/pkgs/cupertino_http/CHANGELOG.md @@ -1,6 +1,7 @@ ## 1.5.0-wip -* Add integration to the DevTools "Network" tab. +* Add integration to the + [DevTools "Network" tab](https://docs.flutter.dev/tools/devtools/network). ## 1.3.0 From 82590ddee516c753795bc9033d01c42ced8eb45b Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 21 Mar 2024 13:52:27 -0700 Subject: [PATCH 12/26] Fix lints --- pkgs/cupertino_http/lib/src/cupertino_client.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index 396234f116..1e35bac1d2 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -289,8 +289,6 @@ class CupertinoClient extends BaseClient { ..headersCommaValues = 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; @@ -299,7 +297,7 @@ class CupertinoClient extends BaseClient { // `httpBodyStream` requires a lot of expensive setup and data passing. urlRequest.httpBody = Data.fromList(request.bodyBytes); profile?.requestData.bodySink.add(request.bodyBytes); - profile?.requestData.close(); + unawaited(profile?.requestData.close()); } 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 - @@ -329,7 +327,7 @@ class CupertinoClient extends BaseClient { try { result = await taskTracker.responseCompleter.future; } catch (e) { - profile?.responseData.closeWithError(e.toString()); + unawaited(profile?.responseData.closeWithError(e.toString())); rethrow; } From 22c184f7d064283afd6275d9da0d7ef6dd43448d Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 21 Mar 2024 13:53:01 -0700 Subject: [PATCH 13/26] Update cupertino_client.dart --- pkgs/cupertino_http/lib/src/cupertino_client.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index 1e35bac1d2..c29cb0a8ce 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -280,7 +280,7 @@ class CupertinoClient extends BaseClient { requestUri: request.url.toString()); profile?.requestData ?..connectionInfo = { - 'package': 'package:cupertino_http', // XXX Set for http_impl.dart + 'package': 'package:cupertino_http', 'client': 'CupertinoClient', 'configuration': _urlSession!.configuration.toString(), } From 779f9ac956e08a83e3a2a1d1fb719a6dd8841e17 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Fri, 22 Mar 2024 15:02:49 -0700 Subject: [PATCH 14/26] Add tests --- .../integration_test/client_profile_test.dart | 266 ++++++++++++++++++ .../example/integration_test/main.dart | 2 + pkgs/cupertino_http/example/pubspec.yaml | 2 + pkgs/cupertino_http/lib/cupertino_http.dart | 2 +- .../lib/src/cupertino_client.dart | 79 ++++-- 5 files changed, 326 insertions(+), 25 deletions(-) create mode 100644 pkgs/cupertino_http/example/integration_test/client_profile_test.dart diff --git a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart new file mode 100644 index 0000000000..e28a96a323 --- /dev/null +++ b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart @@ -0,0 +1,266 @@ +// Copyright (c) 2024, 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:cupertino_http/src/cupertino_client.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart'; +import 'package:http_profile/http_profile.dart'; +import 'package:integration_test/integration_test.dart'; + +void main() { + IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + + group('profile', () { + HttpClientRequestProfile.profilingEnabled = true; + + group('non-streamed POST', () { + late HttpServer successServer; + late Uri successServerUri; + late HttpClientRequestProfile profile; + + setUpAll(() async { + successServer = (await HttpServer.bind('localhost', 0)) + ..listen((request) async { + await request.drain(); + request.response.headers.set('Content-Type', 'text/plain'); + request.response.headers.set('Content-Length', '11'); + request.response.write('Hello World'); + await request.response.close(); + }); + successServerUri = Uri.http('localhost:${successServer.port}'); + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + await client.post(successServerUri, + headers: {'Content-Type': 'text/plain'}, body: 'Hi'); + await Future.delayed(const Duration(seconds: 1)); + profile = client.profile!; + }); + tearDownAll(() { + successServer.close(); + }); + + test('profile attributes', () { + expect(profile.events, isEmpty); + expect(profile.requestMethod, 'POST'); + expect(profile.requestUri, successServerUri.toString()); + }); + + test('request attributes', () { + expect(profile.requestData.bodyBytes, 'Hi'.codeUnits); + expect(profile.requestData.connectionInfo, + containsPair('package', 'package:cupertino_http')); + expect(profile.requestData.contentLength, 2); + expect(profile.requestData.endTime, isNotNull); + expect(profile.requestData.error, isNull); + // XXX Should 'content-length' also be in the headers? It is not for the + // request but it is for the response. + expect(profile.requestData.headers, + containsPair('Content-Type', ['text/plain; charset=utf-8'])); + expect(profile.requestData.persistentConnection, isNull); + expect(profile.requestData.proxyDetails, isNull); + expect(profile.requestData.startTime, isNotNull); + }); + + test('response attributes', () { + expect(profile.responseData.bodyBytes, 'Hello World'.codeUnits); + expect(profile.responseData.compressionState, isNull); + // XXX how are request and response connectionInfo supposed to be + // different? + expect(profile.responseData.connectionInfo, isNull); + expect(profile.responseData.contentLength, 11); + expect(profile.responseData.endTime, isNotNull); + expect(profile.responseData.error, isNull); + expect(profile.responseData.headers, + containsPair('content-type', ['text/plain'])); + expect(profile.responseData.headers, + containsPair('content-length', ['11'])); + expect(profile.responseData.isRedirect, false); + expect(profile.responseData.persistentConnection, isNull); + expect(profile.responseData.reasonPhrase, 'OK'); + expect(profile.responseData.redirects, isEmpty); + expect(profile.responseData.startTime, isNotNull); + expect(profile.responseData.statusCode, 200); + }); + }); + + group('streaming POST request', () { + late HttpServer successServer; + late Uri successServerUri; + late HttpClientRequestProfile profile; + + setUpAll(() async { + successServer = (await HttpServer.bind('localhost', 0)) + ..listen((request) async { + await request.drain(); + request.response.headers.set('Content-Type', 'text/plain'); + request.response.headers.set('Content-Length', '11'); + request.response.write('Hello World'); + await request.response.close(); + }); + successServerUri = Uri.http('localhost:${successServer.port}'); + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + final request = StreamedRequest('POST', successServerUri); + final stream = () async* { + for (var i = 0; i < 1000; ++i) { + await Future.delayed(const Duration()); + // The request has started but not finished. + expect(client.profile!.requestData.startTime, isNotNull); + expect(client.profile!.requestData.endTime, isNull); + expect(client.profile!.responseData.startTime, isNull); + expect(client.profile!.responseData.endTime, isNull); + yield 'Hello'.codeUnits; + } + }(); + unawaited( + request.sink.addStream(stream).then((_) => request.sink.close())); + + await client.send(request); + profile = client.profile!; + }); + tearDownAll(() { + successServer.close(); + }); + + test('request attributes', () async { + expect(profile.requestData.bodyBytes, ('Hello' * 1000).codeUnits); + expect(profile.requestData.contentLength, isNull); + expect(profile.requestData.endTime, isNotNull); + expect(profile.requestData.startTime, isNotNull); + }); + }); + + group('failed POST request', () { + late HttpClientRequestProfile profile; + + setUpAll(() async { + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + try { + await client.post(Uri.http('thisisnotahost'), + headers: {'Content-Type': 'text/plain'}, body: 'Hi'); + fail('expected exception'); + } on ClientException { + // Expected exception. + } + profile = client.profile!; + }); + + test('profile attributes', () { + expect(profile.events, isEmpty); + expect(profile.requestMethod, 'POST'); + expect(profile.requestUri, 'http://thisisnotahost'); + }); + + test('request attributes', () { + expect(profile.requestData.bodyBytes, 'Hi'.codeUnits); + expect(profile.requestData.connectionInfo, + containsPair('package', 'package:cupertino_http')); + expect(profile.requestData.contentLength, 2); + expect(profile.requestData.endTime, isNotNull); + expect(profile.requestData.error, startsWith('ClientException:')); + // XXX Should 'content-length' also be in the headers? It is not for the + // request but it is for the response. + expect(profile.requestData.headers, + containsPair('Content-Type', ['text/plain; charset=utf-8'])); + expect(profile.requestData.persistentConnection, isNull); + expect(profile.requestData.proxyDetails, isNull); + expect(profile.requestData.startTime, isNotNull); + }); + + test('response attributes', () { + // XXX is this the correct expectation? If we have a failure at request + // time then `responseData` should logically contain nothing because + // no response was received. But does `requestData.closeWithError` + // satisfy whatever the I/O service expects? + expect(profile.responseData.bodyBytes, isEmpty); + expect(profile.responseData.compressionState, isNull); + expect(profile.responseData.connectionInfo, isNull); + expect(profile.responseData.contentLength, isNull); + expect(profile.responseData.endTime, isNull); + expect(profile.responseData.error, isNull); + expect(profile.responseData.headers, isNull); + expect(profile.responseData.isRedirect, isNull); + expect(profile.responseData.persistentConnection, isNull); + expect(profile.responseData.reasonPhrase, isNull); + expect(profile.responseData.redirects, isEmpty); + expect(profile.responseData.startTime, isNull); + expect(profile.responseData.statusCode, isNull); + }); + }); + + group('failed POST response', () { + late HttpServer successServer; + late Uri successServerUri; + late HttpClientRequestProfile profile; + + setUpAll(() async { + successServer = (await HttpServer.bind('localhost', 0)) + ..listen((request) async { + await request.drain(); + request.response.headers.set('Content-Type', 'text/plain'); + request.response.headers.set('Content-Length', '11'); + final socket = await request.response.detachSocket(); + await socket.close(); + }); + successServerUri = Uri.http('localhost:${successServer.port}'); + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + + try { + await client.post(successServerUri, + headers: {'Content-Type': 'text/plain'}, body: 'Hi'); + fail('expected exception'); + } on ClientException { + // Expected exception. + } + await Future.delayed(const Duration(seconds: 1)); + profile = client.profile!; + }); + tearDownAll(() { + successServer.close(); + }); + + test('profile attributes', () { + expect(profile.events, isEmpty); + expect(profile.requestMethod, 'POST'); + expect(profile.requestUri, successServerUri.toString()); + }); + + test('request attributes', () { + expect(profile.requestData.bodyBytes, 'Hi'.codeUnits); + expect(profile.requestData.connectionInfo, + containsPair('package', 'package:cupertino_http')); + expect(profile.requestData.contentLength, 2); + expect(profile.requestData.endTime, isNotNull); + expect(profile.requestData.error, isNull); + // XXX Should 'content-length' also be in the headers? It is not for the + // request but it is for the response. + expect(profile.requestData.headers, + containsPair('Content-Type', ['text/plain; charset=utf-8'])); + expect(profile.requestData.persistentConnection, isNull); + expect(profile.requestData.proxyDetails, isNull); + expect(profile.requestData.startTime, isNotNull); + }); + + test('response attributes', () { + expect(profile.responseData.bodyBytes, isEmpty); + expect(profile.responseData.compressionState, isNull); + expect(profile.responseData.connectionInfo, isNull); + expect(profile.responseData.contentLength, 11); + expect(profile.responseData.endTime, isNotNull); + expect(profile.responseData.error, startsWith('ClientException:')); + expect(profile.responseData.headers, + containsPair('content-type', ['text/plain'])); + expect(profile.responseData.headers, + containsPair('content-length', ['11'])); + expect(profile.responseData.isRedirect, false); + expect(profile.responseData.persistentConnection, isNull); + expect(profile.responseData.reasonPhrase, 'OK'); + expect(profile.responseData.redirects, isEmpty); + expect(profile.responseData.startTime, isNotNull); + expect(profile.responseData.statusCode, 200); + }); + }); + }); +} diff --git a/pkgs/cupertino_http/example/integration_test/main.dart b/pkgs/cupertino_http/example/integration_test/main.dart index 0d4d5e16d9..a632b0c40a 100644 --- a/pkgs/cupertino_http/example/integration_test/main.dart +++ b/pkgs/cupertino_http/example/integration_test/main.dart @@ -5,6 +5,7 @@ import 'package:integration_test/integration_test.dart'; import 'client_conformance_test.dart' as client_conformance_test; +import 'client_profile_test.dart' as profile_test; import 'client_test.dart' as client_test; import 'data_test.dart' as data_test; import 'error_test.dart' as error_test; @@ -30,6 +31,7 @@ void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); client_conformance_test.main(); + profile_test.main(); client_test.main(); data_test.main(); error_test.main(); diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index f394051994..c89ca4bf03 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -28,6 +28,8 @@ dev_dependencies: sdk: flutter http_client_conformance_tests: path: ../../http_client_conformance_tests/ + http_profile: + path: ../../http_profile/ integration_test: sdk: flutter test: ^1.21.1 diff --git a/pkgs/cupertino_http/lib/cupertino_http.dart b/pkgs/cupertino_http/lib/cupertino_http.dart index 68691b8ab4..55d62d3c15 100644 --- a/pkgs/cupertino_http/lib/cupertino_http.dart +++ b/pkgs/cupertino_http/lib/cupertino_http.dart @@ -87,5 +87,5 @@ import 'package:http/http.dart'; import 'src/cupertino_client.dart'; export 'src/cupertino_api.dart'; -export 'src/cupertino_client.dart'; +export 'src/cupertino_client.dart' show CupertinoClient; export 'src/cupertino_web_socket.dart'; diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index c29cb0a8ce..e70f9b5ecc 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -45,7 +45,6 @@ class _TaskTracker { void close() { responseController.close(); - profile?.responseData.close(); } } @@ -77,7 +76,7 @@ class CupertinoClient extends BaseClient { URLSession? _urlSession; - CupertinoClient._(URLSession urlSession) : _urlSession = urlSession; + CupertinoClient._(this._urlSession); String? _findReasonPhrase(int statusCode) { switch (statusCode) { @@ -174,14 +173,28 @@ class CupertinoClient extends BaseClient { if (error != null) { final exception = ClientException( error.localizedDescription ?? 'Unknown', taskTracker.request.url); + if (taskTracker.profile != null && + taskTracker.profile!.requestData.endTime == null) { + // Error occurred during the request. + taskTracker.profile!.requestData.closeWithError(exception.toString()); + } else { + // Error occurred during the response. + taskTracker.profile?.responseData.closeWithError(exception.toString()); + } if (taskTracker.responseCompleter.isCompleted) { taskTracker.responseController.addError(exception); } else { taskTracker.responseCompleter.completeError(exception); } - } else if (!taskTracker.responseCompleter.isCompleted) { - taskTracker.responseCompleter.completeError( - StateError('task completed without an error or response')); + } else { + assert(taskTracker.profile == null || + taskTracker.profile!.requestData.endTime != null); + + taskTracker.profile?.responseData.close(); + if (!taskTracker.responseCompleter.isCompleted) { + taskTracker.responseCompleter.completeError( + StateError('task completed without an error or response')); + } } taskTracker.close(); _tasks.remove(task); @@ -213,6 +226,8 @@ class CupertinoClient extends BaseClient { URLSession session, URLSessionTask task, URLResponse response) { final taskTracker = _tracker(task); taskTracker.responseCompleter.complete(response); + taskTracker.profile!.requestData.close(); + return URLSessionResponseDisposition.urlSessionResponseAllow; } @@ -252,6 +267,12 @@ class CupertinoClient extends BaseClient { return (await queue.hasNext, queue.rest); } + HttpClientRequestProfile? _createProfile(BaseRequest request) => + HttpClientRequestProfile.profile( + requestStartTime: DateTime.now(), + requestMethod: request.method, + requestUri: request.url.toString()); + @override Future send(BaseRequest request) async { // The expected success case flow (without redirects) is: @@ -274,10 +295,7 @@ class CupertinoClient extends BaseClient { final stream = request.finalize(); - final profile = HttpClientRequestProfile.profile( - requestStartTime: DateTime.now(), - requestMethod: request.method, - requestUri: request.url.toString()); + final profile = _createProfile(request); profile?.requestData ?..connectionInfo = { 'package': 'package:cupertino_http', @@ -297,7 +315,6 @@ class CupertinoClient extends BaseClient { // `httpBodyStream` requires a lot of expensive setup and data passing. urlRequest.httpBody = Data.fromList(request.bodyBytes); profile?.requestData.bodySink.add(request.bodyBytes); - unawaited(profile?.requestData.close()); } 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 - @@ -307,10 +324,7 @@ class CupertinoClient extends BaseClient { } else { final splitter = StreamSplitter(s); urlRequest.httpBodyStream = splitter.split(); - unawaited( - profile.requestData.bodySink.addStream(splitter.split()).then((e) { - profile.requestData.close(); - })); + unawaited(profile.requestData.bodySink.addStream(splitter.split())); } } @@ -324,12 +338,7 @@ class CupertinoClient extends BaseClient { final maxRedirects = request.followRedirects ? request.maxRedirects : 0; late URLResponse result; - try { - result = await taskTracker.responseCompleter.future; - } catch (e) { - unawaited(profile?.responseData.closeWithError(e.toString())); - rethrow; - } + result = await taskTracker.responseCompleter.future; final response = result as HTTPURLResponse; @@ -348,9 +357,13 @@ class CupertinoClient extends BaseClient { ); } + final contentLength = response.expectedContentLength == -1 + ? null + : response.expectedContentLength; final isRedirect = !request.followRedirects && taskTracker.numRedirects > 0; profile?.responseData - ?..headersCommaValues = responseHeaders + ?..contentLength = contentLength + ..headersCommaValues = responseHeaders ..isRedirect = isRedirect ..reasonPhrase = _findReasonPhrase(response.statusCode) ..startTime = DateTime.now() @@ -360,9 +373,7 @@ class CupertinoClient extends BaseClient { taskTracker.responseController.stream, response.statusCode, url: taskTracker.lastUrl ?? request.url, - contentLength: response.expectedContentLength == -1 - ? null - : response.expectedContentLength, + contentLength: contentLength, reasonPhrase: _findReasonPhrase(response.statusCode), request: request, isRedirect: isRedirect, @@ -370,3 +381,23 @@ class CupertinoClient extends BaseClient { ); } } + +class CupertinoClientWithProfile extends CupertinoClient { + HttpClientRequestProfile? profile; + + @override + HttpClientRequestProfile? _createProfile(BaseRequest request) => + profile = super._createProfile(request); + + CupertinoClientWithProfile._(super._urlSession) : super._(); + + factory CupertinoClientWithProfile.defaultSessionConfiguration() { + final config = URLSessionConfiguration.defaultSessionConfiguration(); + final session = URLSession.sessionWithConfiguration(config, + onComplete: CupertinoClient._onComplete, + onData: CupertinoClient._onData, + onRedirect: CupertinoClient._onRedirect, + onResponse: CupertinoClient._onResponse); + return CupertinoClientWithProfile._(session); + } +} From 9f717d145e9a392162a49f2478022b80c92ddfb2 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Fri, 22 Mar 2024 15:36:10 -0700 Subject: [PATCH 15/26] Update client_profile_test.dart --- .../example/integration_test/client_profile_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart index e28a96a323..4c64241dcd 100644 --- a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart +++ b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart @@ -35,7 +35,6 @@ void main() { final client = CupertinoClientWithProfile.defaultSessionConfiguration(); await client.post(successServerUri, headers: {'Content-Type': 'text/plain'}, body: 'Hi'); - await Future.delayed(const Duration(seconds: 1)); profile = client.profile!; }); tearDownAll(() { @@ -214,7 +213,6 @@ void main() { } on ClientException { // Expected exception. } - await Future.delayed(const Duration(seconds: 1)); profile = client.profile!; }); tearDownAll(() { From 671b7811209de66544cca48a1c1ca062fd7a6b49 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Mon, 25 Mar 2024 16:20:14 -0700 Subject: [PATCH 16/26] Set content-length request header --- .../integration_test/client_profile_test.dart | 15 +++++++-------- pkgs/cupertino_http/lib/src/cupertino_client.dart | 12 ++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart index 4c64241dcd..a03a49efae 100644 --- a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart +++ b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart @@ -54,8 +54,8 @@ void main() { expect(profile.requestData.contentLength, 2); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.error, isNull); - // XXX Should 'content-length' also be in the headers? It is not for the - // request but it is for the response. + expect( + profile.requestData.headers, containsPair('Content-Length', ['2'])); expect(profile.requestData.headers, containsPair('Content-Type', ['text/plain; charset=utf-8'])); expect(profile.requestData.persistentConnection, isNull); @@ -66,8 +66,6 @@ void main() { test('response attributes', () { expect(profile.responseData.bodyBytes, 'Hello World'.codeUnits); expect(profile.responseData.compressionState, isNull); - // XXX how are request and response connectionInfo supposed to be - // different? expect(profile.responseData.connectionInfo, isNull); expect(profile.responseData.contentLength, 11); expect(profile.responseData.endTime, isNotNull); @@ -128,6 +126,7 @@ void main() { expect(profile.requestData.contentLength, isNull); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.startTime, isNotNull); + expect(profile.requestData.headers, isNot(contains('Content-Length'))); }); }); @@ -159,8 +158,8 @@ void main() { expect(profile.requestData.contentLength, 2); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.error, startsWith('ClientException:')); - // XXX Should 'content-length' also be in the headers? It is not for the - // request but it is for the response. + expect( + profile.requestData.headers, containsPair('Content-Length', ['2'])); expect(profile.requestData.headers, containsPair('Content-Type', ['text/plain; charset=utf-8'])); expect(profile.requestData.persistentConnection, isNull); @@ -232,8 +231,8 @@ void main() { expect(profile.requestData.contentLength, 2); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.error, isNull); - // XXX Should 'content-length' also be in the headers? It is not for the - // request but it is for the response. + expect( + profile.requestData.headers, containsPair('Content-Length', ['2'])); expect(profile.requestData.headers, containsPair('Content-Type', ['text/plain; charset=utf-8'])); expect(profile.requestData.persistentConnection, isNull); diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index e70f9b5ecc..bd88820e5b 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -315,6 +315,12 @@ class CupertinoClient extends BaseClient { // `httpBodyStream` requires a lot of expensive setup and data passing. urlRequest.httpBody = Data.fromList(request.bodyBytes); profile?.requestData.bodySink.add(request.bodyBytes); + if (profile != null) { + final headers = + Map>.from(profile.requestData.headers!); + headers['Content-Length'] = ['${request.contentLength}']; + profile.requestData.headersListValues = headers; + } } 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 - @@ -325,6 +331,12 @@ class CupertinoClient extends BaseClient { final splitter = StreamSplitter(s); urlRequest.httpBodyStream = splitter.split(); unawaited(profile.requestData.bodySink.addStream(splitter.split())); + if (request.contentLength != null) { + final headers = + Map>.from(profile.requestData.headers!); + headers['Content-Length'] = ['${request.contentLength}']; + profile.requestData.headersListValues = headers; + } } } From 173238b3e4e1ffd0fedca2a8c9c06dc5bba8310a Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 27 Mar 2024 12:56:11 -0700 Subject: [PATCH 17/26] Update profile --- .../integration_test/client_profile_test.dart | 15 ++++++--------- pkgs/cupertino_http/lib/src/cupertino_client.dart | 12 ++++++------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart index a03a49efae..67385591c2 100644 --- a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart +++ b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart @@ -45,12 +45,12 @@ void main() { expect(profile.events, isEmpty); expect(profile.requestMethod, 'POST'); expect(profile.requestUri, successServerUri.toString()); + expect(profile.connectionInfo, + containsPair('package', 'package:cupertino_http')); }); test('request attributes', () { expect(profile.requestData.bodyBytes, 'Hi'.codeUnits); - expect(profile.requestData.connectionInfo, - containsPair('package', 'package:cupertino_http')); expect(profile.requestData.contentLength, 2); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.error, isNull); @@ -66,7 +66,6 @@ void main() { test('response attributes', () { expect(profile.responseData.bodyBytes, 'Hello World'.codeUnits); expect(profile.responseData.compressionState, isNull); - expect(profile.responseData.connectionInfo, isNull); expect(profile.responseData.contentLength, 11); expect(profile.responseData.endTime, isNotNull); expect(profile.responseData.error, isNull); @@ -149,12 +148,12 @@ void main() { expect(profile.events, isEmpty); expect(profile.requestMethod, 'POST'); expect(profile.requestUri, 'http://thisisnotahost'); + expect(profile.connectionInfo, + containsPair('package', 'package:cupertino_http')); }); test('request attributes', () { expect(profile.requestData.bodyBytes, 'Hi'.codeUnits); - expect(profile.requestData.connectionInfo, - containsPair('package', 'package:cupertino_http')); expect(profile.requestData.contentLength, 2); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.error, startsWith('ClientException:')); @@ -174,7 +173,6 @@ void main() { // satisfy whatever the I/O service expects? expect(profile.responseData.bodyBytes, isEmpty); expect(profile.responseData.compressionState, isNull); - expect(profile.responseData.connectionInfo, isNull); expect(profile.responseData.contentLength, isNull); expect(profile.responseData.endTime, isNull); expect(profile.responseData.error, isNull); @@ -222,12 +220,12 @@ void main() { expect(profile.events, isEmpty); expect(profile.requestMethod, 'POST'); expect(profile.requestUri, successServerUri.toString()); + expect(profile.connectionInfo, + containsPair('package', 'package:cupertino_http')); }); test('request attributes', () { expect(profile.requestData.bodyBytes, 'Hi'.codeUnits); - expect(profile.requestData.connectionInfo, - containsPair('package', 'package:cupertino_http')); expect(profile.requestData.contentLength, 2); expect(profile.requestData.endTime, isNotNull); expect(profile.requestData.error, isNull); @@ -243,7 +241,6 @@ void main() { test('response attributes', () { expect(profile.responseData.bodyBytes, isEmpty); expect(profile.responseData.compressionState, isNull); - expect(profile.responseData.connectionInfo, isNull); expect(profile.responseData.contentLength, 11); expect(profile.responseData.endTime, isNotNull); expect(profile.responseData.error, startsWith('ClientException:')); diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index bd88820e5b..d1ca81df35 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -296,13 +296,13 @@ class CupertinoClient extends BaseClient { final stream = request.finalize(); final profile = _createProfile(request); + profile?.connectionInfo = { + 'package': 'package:cupertino_http', + 'client': 'CupertinoClient', + 'configuration': _urlSession!.configuration.toString(), + }; profile?.requestData - ?..connectionInfo = { - 'package': 'package:cupertino_http', - 'client': 'CupertinoClient', - 'configuration': _urlSession!.configuration.toString(), - } - ..contentLength = request.contentLength + ?..contentLength = request.contentLength ..followRedirects = request.followRedirects ..headersCommaValues = request.headers ..maxRedirects = request.maxRedirects; From 969c0f483f08555ff27f3be1f891148f2af07e7d Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 27 Mar 2024 16:34:47 -0700 Subject: [PATCH 18/26] Add tests --- .../client_conformance_test.dart | 32 ++++++++++++++++--- .../lib/src/cupertino_client.dart | 2 +- 2 files changed, 28 insertions(+), 6 deletions(-) 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 3007123a98..7c936da464 100644 --- a/pkgs/cupertino_http/example/integration_test/client_conformance_test.dart +++ b/pkgs/cupertino_http/example/integration_test/client_conformance_test.dart @@ -5,17 +5,39 @@ import 'package:cupertino_http/cupertino_http.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http_client_conformance_tests/http_client_conformance_tests.dart'; +import 'package:http_profile/http_profile.dart'; import 'package:integration_test/integration_test.dart'; void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); group('defaultSessionConfiguration', () { - testAll( - CupertinoClient.defaultSessionConfiguration, - canReceiveSetCookieHeaders: true, - canSendCookieHeaders: true, - ); + group('profile enabled', () { + final profile = HttpClientRequestProfile.profilingEnabled; + HttpClientRequestProfile.profilingEnabled = true; + try { + testAll( + CupertinoClient.defaultSessionConfiguration, + canReceiveSetCookieHeaders: true, + canSendCookieHeaders: true, + ); + } finally { + HttpClientRequestProfile.profilingEnabled = profile; + } + }); + group('profile disabled', () { + final profile = HttpClientRequestProfile.profilingEnabled; + HttpClientRequestProfile.profilingEnabled = false; + try { + testAll( + CupertinoClient.defaultSessionConfiguration, + canReceiveSetCookieHeaders: true, + canSendCookieHeaders: true, + ); + } finally { + HttpClientRequestProfile.profilingEnabled = profile; + } + }); }); group('fromSessionConfiguration', () { final config = URLSessionConfiguration.ephemeralSessionConfiguration(); diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index d1ca81df35..1860290e91 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -226,7 +226,7 @@ class CupertinoClient extends BaseClient { URLSession session, URLSessionTask task, URLResponse response) { final taskTracker = _tracker(task); taskTracker.responseCompleter.complete(response); - taskTracker.profile!.requestData.close(); + unawaited(taskTracker.profile?.requestData.close()); return URLSessionResponseDisposition.urlSessionResponseAllow; } From 03e8ffc7f9a10231cc0707a0d6da5211c78bc695 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 28 Mar 2024 11:08:35 -0700 Subject: [PATCH 19/26] Add redirect tests --- .../integration_test/client_profile_test.dart | 85 +++++++++++++++++-- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart index 67385591c2..d823370618 100644 --- a/pkgs/cupertino_http/example/integration_test/client_profile_test.dart +++ b/pkgs/cupertino_http/example/integration_test/client_profile_test.dart @@ -15,7 +15,15 @@ void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); group('profile', () { - HttpClientRequestProfile.profilingEnabled = true; + final profilingEnabled = HttpClientRequestProfile.profilingEnabled; + + setUpAll(() { + HttpClientRequestProfile.profilingEnabled = true; + }); + + tearDownAll(() { + HttpClientRequestProfile.profilingEnabled = profilingEnabled; + }); group('non-streamed POST', () { late HttpServer successServer; @@ -167,10 +175,6 @@ void main() { }); test('response attributes', () { - // XXX is this the correct expectation? If we have a failure at request - // time then `responseData` should logically contain nothing because - // no response was received. But does `requestData.closeWithError` - // satisfy whatever the I/O service expects? expect(profile.responseData.bodyBytes, isEmpty); expect(profile.responseData.compressionState, isNull); expect(profile.responseData.contentLength, isNull); @@ -256,5 +260,76 @@ void main() { expect(profile.responseData.statusCode, 200); }); }); + + group('redirects', () { + late HttpServer successServer; + late Uri successServerUri; + late HttpClientRequestProfile profile; + + setUpAll(() async { + successServer = (await HttpServer.bind('localhost', 0)) + ..listen((request) async { + if (request.requestedUri.pathSegments.isEmpty) { + unawaited(request.response.close()); + } else { + final n = int.parse(request.requestedUri.pathSegments.last); + final nextPath = n - 1 == 0 ? '' : '${n - 1}'; + unawaited(request.response + .redirect(successServerUri.replace(path: '/$nextPath'))); + } + }); + successServerUri = Uri.http('localhost:${successServer.port}'); + }); + tearDownAll(() { + successServer.close(); + }); + + test('no redirects', () async { + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + await client.get(successServerUri); + profile = client.profile!; + + expect(profile.responseData.redirects, isEmpty); + }); + + test('follow redirects', () async { + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + await client.send(Request('GET', successServerUri.replace(path: '/3')) + ..followRedirects = true + ..maxRedirects = 4); + profile = client.profile!; + + expect(profile.requestData.followRedirects, true); + expect(profile.requestData.maxRedirects, 4); + expect(profile.responseData.isRedirect, false); + + expect(profile.responseData.redirects, [ + HttpProfileRedirectData( + statusCode: 302, + method: 'GET', + location: successServerUri.replace(path: '/2').toString()), + HttpProfileRedirectData( + statusCode: 302, + method: 'GET', + location: successServerUri.replace(path: '/1').toString()), + HttpProfileRedirectData( + statusCode: 302, + method: 'GET', + location: successServerUri.replace(path: '/').toString(), + ) + ]); + }); + + test('no follow redirects', () async { + final client = CupertinoClientWithProfile.defaultSessionConfiguration(); + await client.send(Request('GET', successServerUri.replace(path: '/3')) + ..followRedirects = false); + profile = client.profile!; + + expect(profile.requestData.followRedirects, false); + expect(profile.responseData.isRedirect, true); + expect(profile.responseData.redirects, isEmpty); + }); + }); }); } From 14b4e3201468e6814a1dce8a4bba0158a4c4fdad Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 28 Mar 2024 13:16:13 -0700 Subject: [PATCH 20/26] Lots of reverts --- pkgs/cronet_http/.idea/workspace.xml | 21 ------------------- .../example/devtools_options.yaml | 3 --- .../ios/Flutter/AppFrameworkInfo.plist | 2 +- pkgs/cupertino_http/example/ios/Podfile | 2 +- pkgs/cupertino_http/example/ios/Podfile.lock | 8 +++---- .../ios/Runner.xcodeproj/project.pbxproj | 9 ++++---- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- pkgs/cupertino_http/example/macos/Podfile | 2 +- .../cupertino_http/example/macos/Podfile.lock | 6 +++--- .../macos/Runner.xcodeproj/project.pbxproj | 8 +++---- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- 11 files changed, 20 insertions(+), 45 deletions(-) delete mode 100644 pkgs/cronet_http/.idea/workspace.xml delete mode 100644 pkgs/cupertino_http/example/devtools_options.yaml diff --git a/pkgs/cronet_http/.idea/workspace.xml b/pkgs/cronet_http/.idea/workspace.xml deleted file mode 100644 index e18b8f2975..0000000000 --- a/pkgs/cronet_http/.idea/workspace.xml +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - - - - - - - \ No newline at end of file diff --git a/pkgs/cupertino_http/example/devtools_options.yaml b/pkgs/cupertino_http/example/devtools_options.yaml deleted file mode 100644 index fa0b357c4f..0000000000 --- a/pkgs/cupertino_http/example/devtools_options.yaml +++ /dev/null @@ -1,3 +0,0 @@ -description: This file stores settings for Dart & Flutter DevTools. -documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states -extensions: diff --git a/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist b/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist index 7c56964006..9625e105df 100644 --- a/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist +++ b/pkgs/cupertino_http/example/ios/Flutter/AppFrameworkInfo.plist @@ -21,6 +21,6 @@ CFBundleVersion 1.0 MinimumOSVersion - 12.0 + 11.0 diff --git a/pkgs/cupertino_http/example/ios/Podfile b/pkgs/cupertino_http/example/ios/Podfile index 279576f388..88359b225f 100644 --- a/pkgs/cupertino_http/example/ios/Podfile +++ b/pkgs/cupertino_http/example/ios/Podfile @@ -1,5 +1,5 @@ # Uncomment this line to define a global platform for your project -# platform :ios, '12.0' +# platform :ios, '11.0' # CocoaPods analytics sends network stats synchronously affecting flutter build latency. ENV['COCOAPODS_DISABLE_STATS'] = 'true' diff --git a/pkgs/cupertino_http/example/ios/Podfile.lock b/pkgs/cupertino_http/example/ios/Podfile.lock index 79e5d5abc6..709672b9d7 100644 --- a/pkgs/cupertino_http/example/ios/Podfile.lock +++ b/pkgs/cupertino_http/example/ios/Podfile.lock @@ -20,9 +20,9 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: cupertino_http: 5f8b1161107fe6c8d94a0c618735a033d93fa7db - Flutter: e0871f40cf51350855a761d2e70bf5af5b9b5de7 - integration_test: ce0a3ffa1de96d1a89ca0ac26fca7ea18a749ef4 + Flutter: f04841e97a9d0b0a8025694d0796dd46242b2854 + integration_test: a1e7d09bd98eca2fc37aefd79d4f41ad37bdbbe5 -PODFILE CHECKSUM: c4c93c5f6502fe2754f48404d3594bf779584011 +PODFILE CHECKSUM: ef19549a9bc3046e7bb7d2fab4d021637c0c58a3 -COCOAPODS: 1.11.3 +COCOAPODS: 1.11.2 diff --git a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj index 64fe2b898d..3f233bb8b9 100644 --- a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj +++ b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/project.pbxproj @@ -156,7 +156,7 @@ 97C146E61CF9000F007C117D /* Project object */ = { isa = PBXProject; attributes = { - LastUpgradeCheck = 1510; + LastUpgradeCheck = 1300; ORGANIZATIONNAME = ""; TargetAttributes = { 97C146ED1CF9000F007C117D = { @@ -205,7 +205,6 @@ files = ( ); inputPaths = ( - "${TARGET_BUILD_DIR}/${INFOPLIST_PATH}", ); name = "Thin Binary"; outputPaths = ( @@ -343,7 +342,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 12.0; + IPHONEOS_DEPLOYMENT_TARGET = 11.0; MTL_ENABLE_DEBUG_INFO = NO; SDKROOT = iphoneos; SUPPORTED_PLATFORMS = iphoneos; @@ -421,7 +420,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 12.0; + IPHONEOS_DEPLOYMENT_TARGET = 11.0; MTL_ENABLE_DEBUG_INFO = YES; ONLY_ACTIVE_ARCH = YES; SDKROOT = iphoneos; @@ -470,7 +469,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 12.0; + IPHONEOS_DEPLOYMENT_TARGET = 11.0; MTL_ENABLE_DEBUG_INFO = NO; SDKROOT = iphoneos; SUPPORTED_PLATFORMS = iphoneos; diff --git a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme index 5e31d3d342..c87d15a335 100644 --- a/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme +++ b/pkgs/cupertino_http/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme @@ -1,6 +1,6 @@ Date: Thu, 28 Mar 2024 13:17:00 -0700 Subject: [PATCH 21/26] Update pubspec.yaml --- pkgs/http_profile/pubspec.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkgs/http_profile/pubspec.yaml b/pkgs/http_profile/pubspec.yaml index 39a2fb9e6b..93b55f296c 100644 --- a/pkgs/http_profile/pubspec.yaml +++ b/pkgs/http_profile/pubspec.yaml @@ -10,5 +10,8 @@ environment: # TODO(derekxu16): Change the following constraint to ^3.4.0 before publishing this package. sdk: ^3.4.0-154.0.dev +dependencies: + test: ^1.24.9 + dev_dependencies: dart_flutter_team_lints: ^2.1.1 From d3a48a3e925fd44781a882878ae002eab36cba4f Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 28 Mar 2024 13:41:22 -0700 Subject: [PATCH 22/26] Update cupertino_client.dart --- pkgs/cupertino_http/lib/src/cupertino_client.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index 1860290e91..c36e26975f 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -394,6 +394,7 @@ class CupertinoClient extends BaseClient { } } +/// A test-only class that makes the [HttpClientRequestProfile] data available. class CupertinoClientWithProfile extends CupertinoClient { HttpClientRequestProfile? profile; From 81deb023ae45fb67718be43a6040870b754154d7 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Fri, 29 Mar 2024 11:10:32 -0700 Subject: [PATCH 23/26] Update cupertino_client.dart --- .../lib/src/cupertino_client.dart | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index c36e26975f..f535ae3c8f 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -307,6 +307,13 @@ class CupertinoClient extends BaseClient { ..headersCommaValues = request.headers ..maxRedirects = request.maxRedirects; + if (profile != null && request.contentLength != null) { + profile.requestData.headersListValues = { + 'Content-Length': ['${request.contentLength}'], + ...profile.requestData.headers! + }; + } + final urlRequest = MutableURLRequest.fromUrl(request.url) ..httpMethod = request.method; @@ -315,12 +322,6 @@ class CupertinoClient extends BaseClient { // `httpBodyStream` requires a lot of expensive setup and data passing. urlRequest.httpBody = Data.fromList(request.bodyBytes); profile?.requestData.bodySink.add(request.bodyBytes); - if (profile != null) { - final headers = - Map>.from(profile.requestData.headers!); - headers['Content-Length'] = ['${request.contentLength}']; - profile.requestData.headersListValues = headers; - } } 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 - @@ -331,12 +332,6 @@ class CupertinoClient extends BaseClient { final splitter = StreamSplitter(s); urlRequest.httpBodyStream = splitter.split(); unawaited(profile.requestData.bodySink.addStream(splitter.split())); - if (request.contentLength != null) { - final headers = - Map>.from(profile.requestData.headers!); - headers['Content-Length'] = ['${request.contentLength}']; - profile.requestData.headersListValues = headers; - } } } From 26d048c4e33c5a783006dac79519d052564d33ff Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 9 May 2024 14:28:07 -0700 Subject: [PATCH 24/26] Fix versions --- .github/workflows/cupertino.yml | 2 +- pkgs/cupertino_http/pubspec.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cupertino.yml b/.github/workflows/cupertino.yml index 21fb23bb44..b790193875 100644 --- a/.github/workflows/cupertino.yml +++ b/.github/workflows/cupertino.yml @@ -31,7 +31,7 @@ jobs: matrix: # Test on the minimum supported flutter version and the latest # version. - flutter-version: ["3.19.0", "any"] + flutter-version: ["3.22.0", "any"] steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225 diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index 24aff6c850..83174227f4 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -7,8 +7,8 @@ repository: https://github.com/dart-lang/http/tree/master/pkgs/cupertino_http publish_to: none environment: - sdk: ^3.2.0 # XXX Wrong! - flutter: '>=3.16.0' # XXX Wrong! + sdk: ^3.4.0 + flutter: '>=3.22.0' dependencies: async: ^2.5.0 From f1ae74e9f6cfe591711cd782033ce455b198ff30 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Thu, 9 May 2024 14:34:27 -0700 Subject: [PATCH 25/26] Release prep --- pkgs/cupertino_http/example/pubspec.yaml | 3 +-- pkgs/cupertino_http/pubspec.yaml | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index c89ca4bf03..881013456c 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -28,8 +28,7 @@ dev_dependencies: sdk: flutter http_client_conformance_tests: path: ../../http_client_conformance_tests/ - http_profile: - path: ../../http_profile/ + http_profile: ^0.1.0 integration_test: sdk: flutter test: ^1.21.1 diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml index 83174227f4..ac6bf5b059 100644 --- a/pkgs/cupertino_http/pubspec.yaml +++ b/pkgs/cupertino_http/pubspec.yaml @@ -8,7 +8,7 @@ publish_to: none environment: sdk: ^3.4.0 - flutter: '>=3.22.0' + flutter: '>=3.22.0' # If changed, update test matrix. dependencies: async: ^2.5.0 @@ -16,8 +16,7 @@ dependencies: flutter: sdk: flutter http: ^1.2.0 - http_profile: - path: ../http_profile + http_profile: ^0.1.0 web_socket: ^0.1.0 dev_dependencies: From f95600392f207cd644b21e4089a6afefa22b79a6 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 22 May 2024 18:21:39 -0700 Subject: [PATCH 26/26] Review fixes --- pkgs/cupertino_http/CHANGELOG.md | 4 ++-- pkgs/cupertino_http/example/pubspec.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/cupertino_http/CHANGELOG.md b/pkgs/cupertino_http/CHANGELOG.md index 4be9e639b2..090475c3d6 100644 --- a/pkgs/cupertino_http/CHANGELOG.md +++ b/pkgs/cupertino_http/CHANGELOG.md @@ -1,7 +1,7 @@ -## 1.5.0-wip +## 1.5.0 * Add integration to the - [DevTools "Network" tab](https://docs.flutter.dev/tools/devtools/network). + [DevTools Network View](https://docs.flutter.dev/tools/devtools/network). * Upgrade to `package:ffigen` 11.0.0. * Bring `WebSocket` behavior in line with the documentation by throwing `WebSocketConnectionClosed` rather than `StateError` when attempting to send diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index 881013456c..ba79752e48 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -6,8 +6,8 @@ publish_to: 'none' version: 1.0.0+1 environment: - sdk: ^3.2.0 - flutter: '>=3.16.0' + sdk: ^3.4.0 + flutter: '>=3.22.0' dependencies: cupertino_http: