Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
100 changes: 90 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,58 @@ class DartIOHttpRequestData extends NetworkRequest {
DateTime? get _endTime =>
_hasError ? _request.endTime : _request.response?.endTime;

bool _matchesCancellationMarker(String? value) {
if (value == null) return false;
final normalized = value.toLowerCase();

/// Markers used for substring matching against request / response errors
/// and request event names to classify cancelled requests.
///
/// Derived from observed cancellation wording in HTTP profiler payloads,
/// keeping specific terms to reduce false positives.
const _cancellationMarkers = [
'canceled',
'cancelled',
'aborted',
];

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

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 +203,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 +274,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 +328,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 +357,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
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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)
- Improved 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](https://github.com/flutter/devtools/pull/9683)

## Logging 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);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is clearer than the old code, thanks!

}
}

// 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