Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
99 changes: 89 additions & 10 deletions packages/devtools_app/lib/src/shared/http/http_request_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are just searching in a String for these values, we either don't need the longer ones ("operation canceled") or we don't need the shorter ones ("canceled").

I think it is find to just keep the shorter ones.

Are these values derived from some general libraries, like the dio package and dart:io?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they’re derived from cancellation wording observed in common Dart HTTP stacks, specifically our dart:io, package:http, and dio companion flows and the resulting VM profile payload strings.

'operation cancelled',
'abort',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove the shortest two in this list, "cancel" and "abort." Unless there are examples from dart:io, package:http, or package:dio, where a cancelled error/event contains "cancel" or "abort" but not "cancelled"/"canceled" or "aborted."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions, I’ve updated things accordingly.

I kept just canceled, cancelled, and aborted, and removed the longer phrases like operation canceled / operation cancelled since we’re already doing substring matching. I also dropped the more generic terms like cancel and abort to avoid false positives.

These markers are based on the cancellation wording we’ve seen in related flows (like dart:io request.abort(), package:http client.close(), and dio cancelToken.cancel(...)) along with the profiler payload text.

One thing to note: in companion cancel mode, we can still get a 200 since the status gets set early so in those cases, classification relies on explicit cancellation text in the error/event payloads.

'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.
Expand All @@ -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,
},
Expand Down Expand Up @@ -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<DartIOHttpInstantEvent> get instantEvents {
Expand Down Expand Up @@ -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!);
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ 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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<DartIOHttpRequestData>()
.where((request) => request.status == 'Error')
.length;
expect(controller.filteredData.value, hasLength(errorCount));

final firstStatus = profile
.whereType<DartIOHttpRequestData>()
.map((request) => request.status)
.whereType<String>()
.first;
final firstStatusCount = profile
.whereType<DartIOHttpRequestData>()
.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));
Expand Down Expand Up @@ -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<DartIOHttpRequestData>()
.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<DartIOHttpRequestData>()
.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 {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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];
Expand Down
Loading
Loading