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); + } +}