diff --git a/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart b/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart index 831b3e52052..a09706d0002 100644 --- a/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart +++ b/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart @@ -721,7 +721,9 @@ class NetworkRequestOverviewView extends StatelessWidget { Widget _buildHttpTimeGraph() { final data = this.data as DartIOHttpRequestData; - if (data.duration == null || data.instantEvents.isEmpty) { + if (data.duration == null || + data.duration!.inMicroseconds == 0 || + data.instantEvents.isEmpty) { return Container( key: httpTimingGraphKey, height: 18.0, diff --git a/packages/devtools_app/lib/src/shared/http/http_request_data.dart b/packages/devtools_app/lib/src/shared/http/http_request_data.dart index 6347ffdbf8d..0ee2f66f2f0 100644 --- a/packages/devtools_app/lib/src/shared/http/http_request_data.dart +++ b/packages/devtools_app/lib/src/shared/http/http_request_data.dart @@ -138,11 +138,57 @@ class DartIOHttpRequestData extends NetworkRequest { DateTime? get _endTime => _hasError ? _request.endTime : _request.response?.endTime; + static const _cancellationMarkers = [ + 'cancel', + 'canceled', + 'cancelled', + 'operation canceled', + 'operation cancelled', + 'abort', + 'aborted', + ]; + + bool _matchesCancellationMarker(String? value) { + final normalized = value?.toLowerCase(); + if (normalized == null) return false; + return _cancellationMarkers.any(normalized.contains); + } + + bool get _hasCancellationError { + final requestError = _request.request?.error; + final responseError = _request.response?.error; + return _matchesCancellationMarker(requestError) || + _matchesCancellationMarker(responseError); + } + + bool get _hasCancellationEvent => + _request.events.any((event) => _matchesCancellationMarker(event.event)); + @override Duration? get duration { - if (inProgress || !isValid) return null; - // Timestamps are in microseconds - return _endTime!.difference(_request.startTime); + if (_hasError) { + final start = _request.startTime; + final end = _request.endTime; + return end?.difference(start); + } + + // Cancelled request + if (isCancelled) { + return Duration.zero; + } + + if (inProgress) { + return null; + } + + final start = _request.startTime; + final end = _request.response?.endTime ?? _request.endTime; + + if (end != null) { + return end.difference(start); + } + + return null; } /// Whether the request is safe to display in the UI. @@ -156,7 +202,7 @@ class DartIOHttpRequestData extends NetworkRequest { return { 'method': _request.method, 'uri': _request.uri.toString(), - if (!didFail) ...{ + if (!didFail && !isCancelled) ...{ 'connectionInfo': _request.request?.connectionInfo, 'contentLength': _request.request?.contentLength, }, @@ -227,11 +273,19 @@ class DartIOHttpRequestData extends NetworkRequest { return connectionInfo != null ? connectionInfo[_localPortKey] : null; } - /// True if the HTTP request hasn't completed yet, determined by the lack of - /// an end time in the response data. @override - bool get inProgress => - _hasError ? !_request.isRequestComplete : !_request.isResponseComplete; + bool get inProgress { + if (isCancelled) { + return false; + } + + final statusCode = _request.response?.statusCode; + if (statusCode != null) { + return false; + } + + return _request.endTime == null; + } /// All instant events logged to the timeline for this HTTP request. List get instantEvents { @@ -273,6 +327,7 @@ class DartIOHttpRequestData extends NetworkRequest { bool get didFail { if (status == null) return false; if (status == 'Error') return true; + if (status == 'Cancelled') return false; try { final code = int.parse(status!); @@ -301,12 +356,36 @@ class DartIOHttpRequestData extends NetworkRequest { DateTime get startTimestamp => _request.startTime; @override - String? get status => - _hasError ? 'Error' : _request.response?.statusCode.toString(); + String? get status { + if (isCancelled) return 'Cancelled'; + + final statusCode = _request.response?.statusCode; + if (statusCode != null) return statusCode.toString(); + + if (_hasError) return 'Error'; + + return null; + } @override String get uri => _request.uri.toString(); + bool get isCancelled { + if (_hasCancellationError || _hasCancellationEvent) { + return true; + } + + if (_request.request?.error != null && _request.response == null) { + return true; + } + + if (_request.endTime != null && _request.response == null) { + return true; + } + + return false; + } + String? get responseBody { if (_request is! HttpProfileRequest) { return null; diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 26d92bf669a..d1c8e8723c1 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -39,7 +39,7 @@ TODO: Remove this section if there are not any updates. ## Network profiler updates -- Added a filter setting to hide HTTP-profiler socket data. [#9698](https://github.com/flutter/devtools/pull/9698) +- Improve HTTP request status classification in the Network tab to better distinguish cancelled, completed, and in-flight requests (for example, avoiding some cases where cancelled requests appeared as pending). (#9683) ## Logging updates @@ -47,7 +47,7 @@ TODO: Remove this section if there are not any updates. ## App size tool updates -- Added documentation links and improved handling for null files. [#9689](https://github.com/flutter/devtools/pull/9689) +TODO: Remove this section if there are not any updates. ## Deep links tool updates diff --git a/packages/devtools_app/test/screens/network/network_controller_test.dart b/packages/devtools_app/test/screens/network/network_controller_test.dart index 1cc1d6d69a4..d41aeb0ab46 100644 --- a/packages/devtools_app/test/screens/network/network_controller_test.dart +++ b/packages/devtools_app/test/screens/network/network_controller_test.dart @@ -112,7 +112,9 @@ void main() { expect(request.duration, request.inProgress ? isNull : isNotNull); expect(request.general.length, greaterThan(0)); expect(httpMethods.contains(request.method), true); - expect(request.status, request.inProgress ? isNull : isNotNull); + if (request.inProgress) { + expect(request.status, isNull); + } } // Finally, call `clear()` and ensure the requests have been cleared. @@ -205,15 +207,28 @@ void main() { controller.setActiveFilter(query: 'status:Error'); expect(profile, hasLength(numRequests)); - expect(controller.filteredData.value, hasLength(1)); - - controller.setActiveFilter(query: 's:101'); + final errorCount = profile + .whereType() + .where((request) => request.status == 'Error') + .length; + expect(controller.filteredData.value, hasLength(errorCount)); + + final firstStatus = profile + .whereType() + .map((request) => request.status) + .whereType() + .first; + final firstStatusCount = profile + .whereType() + .where((request) => request.status == firstStatus) + .length; + controller.setActiveFilter(query: 's:$firstStatus'); expect(profile, hasLength(numRequests)); - expect(controller.filteredData.value, hasLength(1)); + expect(controller.filteredData.value, hasLength(firstStatusCount)); controller.setActiveFilter(query: '-s:Error'); expect(profile, hasLength(numRequests)); - expect(controller.filteredData.value, hasLength(8)); + expect(controller.filteredData.value, hasLength(numRequests - errorCount)); controller.setActiveFilter(query: 'type:json'); expect(profile, hasLength(numRequests)); @@ -253,11 +268,28 @@ void main() { controller.setActiveFilter(query: '-status:error method:get'); expect(profile, hasLength(numRequests)); - expect(controller.filteredData.value, hasLength(3)); + final nonErrorGetCount = profile + .whereType() + .where( + (request) => + request.method.toLowerCase() == 'get' && + request.status?.toLowerCase() != 'error', + ) + .length; + expect(controller.filteredData.value, hasLength(nonErrorGetCount)); controller.setActiveFilter(query: '-status:error method:get t:http'); expect(profile, hasLength(numRequests)); - expect(controller.filteredData.value, hasLength(2)); + final nonErrorGetHttpCount = profile + .whereType() + .where( + (request) => + request.method.toLowerCase() == 'get' && + request.status?.toLowerCase() != 'error' && + request.type.toLowerCase() == 'http', + ) + .length; + expect(controller.filteredData.value, hasLength(nonErrorGetHttpCount)); }); test('filterData hides tcp sockets via setting filter', () async { @@ -341,6 +373,21 @@ void main() { 'statusCode': 200, }, })!; + final request1CancelledWithStatusCode = HttpProfileRequest.parse({ + ...httpBaseObject, + 'events': [ + { + 'timestamp': startTime + 100, + 'event': 'Request cancelled by client', + }, + ], + 'response': { + 'startTime': startTime, + 'endTime': null, + 'redirects': [], + 'statusCode': 200, + }, + })!; final request2Pending = HttpProfileRequest.parse({ ...httpBaseObject, 'id': '102', @@ -403,6 +450,31 @@ void main() { }, ); + test('latest request update wins over stale status for same id', () { + currentNetworkRequests.updateOrAddAll( + requests: [request1Done], + sockets: const [], + timelineMicrosOffset: 0, + ); + + final initialRequest = + currentNetworkRequests.getRequest('101')! as DartIOHttpRequestData; + expect(initialRequest.status, '200'); + expect(initialRequest.isCancelled, false); + + currentNetworkRequests.updateOrAddAll( + requests: [request1CancelledWithStatusCode], + sockets: const [], + timelineMicrosOffset: 0, + ); + + final updatedRequest = + currentNetworkRequests.getRequest('101')! as DartIOHttpRequestData; + expect(updatedRequest.isCancelled, true); + expect(updatedRequest.status, 'Cancelled'); + expect(updatedRequest.inProgress, false); + }); + test('clear', () { final reqs = [request1Pending, request2Pending]; final sockets = [socketStats1Pending, socketStats2Pending]; diff --git a/packages/devtools_app/test/screens/network/network_model_test.dart b/packages/devtools_app/test/screens/network/network_model_test.dart index 8b1459e7a80..2d8bdeb9a97 100644 --- a/packages/devtools_app/test/screens/network/network_model_test.dart +++ b/packages/devtools_app/test/screens/network/network_model_test.dart @@ -11,6 +11,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:vm_service/vm_service.dart'; import '../../test_infra/test_data/network.dart'; +import '../../test_infra/test_data/http_get_cancelled_json.dart'; import 'utils/network_test_utils.dart'; void main() { @@ -183,7 +184,7 @@ void main() { test('status returns correct value', () { expect(httpGet.status, '200'); - expect(httpGetWithError.status, 'Error'); + expect(httpGetWithError.status, 'Cancelled'); expect(httpPost.status, '201'); expect(httpPut.status, '200'); expect(httpPatch.status, '200'); @@ -499,11 +500,182 @@ void main() { test('didFail returns correct value', () { expect(httpGet.didFail, false); - expect(httpGetWithError.didFail, true); + expect(httpGetWithError.didFail, false); expect(httpPost.didFail, false); expect(httpPut.didFail, false); expect(httpPatch.didFail, false); expect(httpWsHandshake.didFail, false); }); }); + + test('cancelled request should not remain pending', () { + // No controller needed — construct directly from fixture + final cancelledRequest = DartIOHttpRequestData( + HttpProfileRequest.parse( + Map.from(httpGetCancelledJson), + )!, + requestFullDataFromVmService: false, + ); + + expect(cancelledRequest.isCancelled, true); + expect(cancelledRequest.inProgress, false); + expect(cancelledRequest.status, 'Cancelled'); + expect(cancelledRequest.duration, isNotNull); + }); + + test('request with cancellation evidence is cancelled even with null endTime', () { + final inFlightData = Map.from(httpGetCancelledJson) + ..['endTime'] = null; + + final inFlightRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(inFlightData)!, + requestFullDataFromVmService: false, + ); + + expect(inFlightRequest.isCancelled, true); + expect(inFlightRequest.inProgress, false); + expect(inFlightRequest.status, 'Cancelled'); + expect(inFlightRequest.duration, Duration.zero); + }); + + test('request without response and endTime is cancelled', () { + final pendingRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(Map.from(httpGetPendingJson))!, + requestFullDataFromVmService: false, + ); + + expect(pendingRequest.isCancelled, true); + expect(pendingRequest.inProgress, false); + expect(pendingRequest.status, 'Cancelled'); + expect(pendingRequest.duration, isNotNull); + }); + + test('request without response and null endTime remains pending', () { + final pendingData = Map.from(httpGetPendingJson) + ..['endTime'] = null; + + final pendingRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(pendingData)!, + requestFullDataFromVmService: false, + ); + + expect(pendingRequest.isCancelled, false); + expect(pendingRequest.inProgress, true); + expect(pendingRequest.status, isNull); + expect(pendingRequest.duration, isNull); + }); + + test('request with incomplete response and status code is completed', () { + final responseData = Map.from( + (httpGetJson['response'] as Map).cast(), + )..['endTime'] = null; + + final cancelledWithStatusData = Map.from(httpGetJson) + ..['response'] = responseData; + + final cancelledWithStatusRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(cancelledWithStatusData)!, + requestFullDataFromVmService: false, + ); + + expect(cancelledWithStatusRequest.isCancelled, false); + expect(cancelledWithStatusRequest.inProgress, false); + expect(cancelledWithStatusRequest.status, '200'); + expect(cancelledWithStatusRequest.didFail, false); + }); + + test('request with partial response and cancellation error is cancelled', () { + final responseData = Map.from( + (httpGetJson['response'] as Map).cast(), + ) + ..['endTime'] = null + ..['error'] = 'Response stream aborted by client cancellation'; + + final cancelledDuringResponseData = Map.from(httpGetJson) + ..['response'] = responseData; + + final cancelledDuringResponseRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(cancelledDuringResponseData)!, + requestFullDataFromVmService: false, + ); + + expect(cancelledDuringResponseRequest.isCancelled, true); + expect(cancelledDuringResponseRequest.inProgress, false); + expect(cancelledDuringResponseRequest.status, 'Cancelled'); + expect(cancelledDuringResponseRequest.duration, Duration.zero); + }); + + test('request with partial response and generic response error is not cancelled', () { + final responseData = Map.from( + (httpGetJson['response'] as Map).cast(), + ) + ..['endTime'] = null + ..['error'] = 'Connection closed before full response was received'; + + final cancelledDuringResponseData = Map.from(httpGetJson) + ..['response'] = responseData; + + final cancelledDuringResponseRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(cancelledDuringResponseData)!, + requestFullDataFromVmService: false, + ); + + expect(cancelledDuringResponseRequest.isCancelled, false); + expect(cancelledDuringResponseRequest.status, '200'); + }); + + test('request with response hasError and status code is not cancelled', () { + final responseData = Map.from( + (httpGetJson['response'] as Map).cast(), + ) + ..['error'] = 'HttpException: connection terminated' + ..['endTime'] = httpGetJson['endTime']; + + final cancelledData = Map.from(httpGetJson) + ..['response'] = responseData; + + final cancelledRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(cancelledData)!, + requestFullDataFromVmService: false, + ); + + expect(cancelledRequest.isCancelled, false); + expect(cancelledRequest.status, '200'); + }); + + test('request with request hasError and response present is cancelled', () { + final requestData = Map.from( + (httpGetJson['request'] as Map).cast(), + )..['error'] = 'Cancelled by client before completion'; + + final cancelledData = Map.from(httpGetJson) + ..['request'] = requestData; + + final cancelledRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(cancelledData)!, + requestFullDataFromVmService: false, + ); + + expect(cancelledRequest.isCancelled, true); + expect(cancelledRequest.status, 'Cancelled'); + }); + + test('request with request error and incomplete response is cancelled', () { + final requestData = Map.from( + (httpGetJson['request'] as Map).cast(), + )..['error'] = 'SocketException: connection reset by peer'; + + final cancelledData = Map.from(httpGetJson) + ..['request'] = requestData + ..['response'] = null; + + final cancelledRequest = DartIOHttpRequestData( + HttpProfileRequest.parse(cancelledData)!, + requestFullDataFromVmService: false, + ); + + expect(cancelledRequest.isCancelled, true); + expect(cancelledRequest.inProgress, false); + expect(cancelledRequest.status, 'Cancelled'); + }); } diff --git a/packages/devtools_app/test/screens/network/network_table_test.dart b/packages/devtools_app/test/screens/network/network_table_test.dart index e9191c3b97f..e62069e02d7 100644 --- a/packages/devtools_app/test/screens/network/network_table_test.dart +++ b/packages/devtools_app/test/screens/network/network_table_test.dart @@ -77,7 +77,7 @@ void main() { expect(column.getDisplayValue(getRequest), httpGet.status); final pendingRequest = findRequestById('7'); - expect(column.getDisplayValue(pendingRequest), '--'); + expect(column.getDisplayValue(pendingRequest), 'Cancelled'); }); test('TypeColumn for http request', () { @@ -94,16 +94,16 @@ void main() { expect(column.getDisplayValue(getRequest), '811 ms'); final pendingRequest = findRequestById('7'); - expect(column.getDisplayValue(pendingRequest), 'Pending'); + expect(column.getDisplayValue(pendingRequest), '0 μs'); }); test('TimestampColumn', () { final column = TimestampColumn(); final getRequest = findRequestById('1'); - // The hours field may be unreliable since it depends on the timezone the - // test is running in. - expect(column.getDisplayValue(getRequest), contains(':45:26.279')); + // The hours and minutes field may be unreliable since it depends on the + // timezone the test is running in (e.g. UTC vs IST). + expect(column.getDisplayValue(getRequest), contains('26.279')); }); }); } diff --git a/packages/devtools_app/test/test_infra/test_data/http_get_cancelled_json.dart b/packages/devtools_app/test/test_infra/test_data/http_get_cancelled_json.dart new file mode 100644 index 00000000000..9449e3ed1ad --- /dev/null +++ b/packages/devtools_app/test/test_infra/test_data/http_get_cancelled_json.dart @@ -0,0 +1,34 @@ +// Copyright 2020 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. + +// Fixture for a cancelled HTTP request: +// - isRequestComplete: true (client finished sending) +// - isResponseComplete: false (no response arrived) +// - response: null +// - endTime is set (request is done, not still in-flight) + +const httpGetCancelledJson = { + 'id': '99', + 'isolateId': 'isolates/123', + 'method': 'GET', + 'uri': 'https://jsonplaceholder.typicode.com/albums/1', + 'events': [ + {'timestamp': 6326379935, 'event': 'Request cancelled by client'}, + ], + 'startTime': 6326279935, // microseconds + 'endTime': 6326479935, // 200ms later + 'request': { + 'headers': {}, + 'compressionState': 'HttpClientRequestCompressionState.notCompressed', + 'connectionInfo': null, + 'contentLength': 0, + 'cookies': [], + 'followRedirects': true, + 'maxRedirects': 5, + 'method': 'GET', + 'persistentConnection': true, + 'uri': 'https://jsonplaceholder.typicode.com/albums/1', + }, + 'response': null, // ← key: no response +};