Skip to content

Commit 5de8e82

Browse files
rishika0212rishika0212
committed
Refine HTTP cancellation/status handling and network timing graph null-safety; update related network tests
1 parent 8a50293 commit 5de8e82

6 files changed

Lines changed: 83 additions & 103 deletions

File tree

‎packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -707,8 +707,13 @@ class NetworkRequestOverviewView extends StatelessWidget {
707707
];
708708
}
709709

710-
Widget _buildTimingRow(Color color, String label, Duration duration) {
711-
final flex = (duration.inMicroseconds / data.duration!.inMicroseconds * 100)
710+
Widget _buildTimingRow(
711+
Color color,
712+
String label,
713+
Duration duration,
714+
Duration totalDuration,
715+
) {
716+
final flex = (duration.inMicroseconds / totalDuration.inMicroseconds * 100)
712717
.round();
713718
return Flexible(
714719
flex: flex,
@@ -721,8 +726,9 @@ class NetworkRequestOverviewView extends StatelessWidget {
721726

722727
Widget _buildHttpTimeGraph() {
723728
final data = this.data as DartIOHttpRequestData;
724-
if (data.duration == null ||
725-
data.duration!.inMicroseconds == 0 ||
729+
final requestDuration = data.duration;
730+
if (requestDuration == null ||
731+
requestDuration.inMicroseconds == 0 ||
726732
data.instantEvents.isEmpty) {
727733
return Container(
728734
key: httpTimingGraphKey,
@@ -745,14 +751,18 @@ class NetworkRequestOverviewView extends StatelessWidget {
745751
final timingWidgets = <Widget>[];
746752
for (final instant in data.instantEvents) {
747753
final duration = instant.timeRange.duration;
748-
timingWidgets.add(_buildTimingRow(nextColor(), instant.name, duration));
754+
timingWidgets.add(
755+
_buildTimingRow(nextColor(), instant.name, duration, requestDuration),
756+
);
749757
}
750758
final duration = Duration(
751759
microseconds:
752760
data.endTimestamp!.microsecondsSinceEpoch -
753761
data.instantEvents.last.timestamp.microsecondsSinceEpoch,
754762
);
755-
timingWidgets.add(_buildTimingRow(nextColor(), 'Response', duration));
763+
timingWidgets.add(
764+
_buildTimingRow(nextColor(), 'Response', duration, requestDuration),
765+
);
756766
return Row(key: httpTimingGraphKey, children: timingWidgets);
757767
}
758768

‎packages/devtools_app/lib/src/shared/http/http_request_data.dart‎

Lines changed: 16 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -135,18 +135,19 @@ class DartIOHttpRequestData extends NetworkRequest {
135135

136136
bool get _hasError => _request.request?.hasError ?? false;
137137

138-
DateTime? get _endTime =>
139-
_hasError ? _request.endTime : _request.response?.endTime;
138+
DateTime? get _endTime => (_hasError || _isCancelled)
139+
? _request.endTime
140+
: _request.response?.endTime;
140141

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

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

152153
return _cancellationMarkers.any(normalized.contains);
@@ -164,28 +165,10 @@ class DartIOHttpRequestData extends NetworkRequest {
164165

165166
@override
166167
Duration? get duration {
167-
if (_hasError) {
168-
final start = _request.startTime;
169-
final end = _request.endTime;
170-
return end?.difference(start);
171-
}
172-
173-
if (isCancelled) {
174-
return Duration.zero;
175-
}
176-
177-
if (inProgress) {
168+
if (inProgress || !isValid) {
178169
return null;
179170
}
180-
181-
final start = _request.startTime;
182-
final end = _request.response?.endTime ?? _request.endTime;
183-
184-
if (end != null) {
185-
return end.difference(start);
186-
}
187-
188-
return null;
171+
return _endTime?.difference(_request.startTime);
189172
}
190173

191174
/// Whether the request is safe to display in the UI.
@@ -199,7 +182,7 @@ class DartIOHttpRequestData extends NetworkRequest {
199182
return {
200183
'method': _request.method,
201184
'uri': _request.uri.toString(),
202-
if (!didFail && !isCancelled) ...{
185+
if (!didFail && !_isCancelled) ...{
203186
'connectionInfo': _request.request?.connectionInfo,
204187
'contentLength': _request.request?.contentLength,
205188
},
@@ -272,15 +255,7 @@ class DartIOHttpRequestData extends NetworkRequest {
272255

273256
@override
274257
bool get inProgress {
275-
if (isCancelled) {
276-
return false;
277-
}
278-
279-
final statusCode = _request.response?.statusCode;
280-
if (statusCode != null) {
281-
return false;
282-
}
283-
258+
if (_isCancelled) return false;
284259
return _request.endTime == null;
285260
}
286261

@@ -354,34 +329,20 @@ class DartIOHttpRequestData extends NetworkRequest {
354329

355330
@override
356331
String? get status {
357-
if (isCancelled) return 'Cancelled';
332+
if (_isCancelled) return 'Cancelled';
333+
334+
if (_hasError) return 'Error';
358335

359336
final statusCode = _request.response?.statusCode;
360337
if (statusCode != null) return statusCode.toString();
361338

362-
if (_hasError) return 'Error';
363-
364339
return null;
365340
}
366341

367342
@override
368343
String get uri => _request.uri.toString();
369344

370-
bool get isCancelled {
371-
if (_hasCancellationError || _hasCancellationEvent) {
372-
return true;
373-
}
374-
375-
if (_request.request?.error != null && _request.response == null) {
376-
return true;
377-
}
378-
379-
if (_request.endTime != null && _request.response == null) {
380-
return true;
381-
}
382-
383-
return false;
384-
}
345+
bool get _isCancelled => _hasCancellationError || _hasCancellationEvent;
385346

386347
String? get responseBody {
387348
if (_request is! HttpProfileRequest) {

‎packages/devtools_app/lib/src/shared/primitives/custom_pointer_scroll_view.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ abstract class CustomPointerScrollView extends BoxScrollView {
3131
super.physics,
3232
super.shrinkWrap,
3333
super.padding,
34-
super.scrollCacheExtent,
34+
super.cacheExtent,
3535
super.semanticChildCount,
3636
super.dragStartBehavior,
3737
this.customPointerSignalHandler,

‎packages/devtools_app/test/screens/network/network_controller_test.dart‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ void main() {
109109
expect(requests.length, numRequests);
110110
final httpRequests = requests.whereType<DartIOHttpRequestData>().toList();
111111
for (final request in httpRequests) {
112-
expect(request.duration, request.inProgress ? isNull : isNotNull);
112+
expect(
113+
request.duration,
114+
request.inProgress || request.endTimestamp == null
115+
? isNull
116+
: isNotNull,
117+
);
113118
expect(request.general.length, greaterThan(0));
114119
expect(httpMethods.contains(request.method), true);
115120
if (request.inProgress) {
@@ -463,7 +468,7 @@ void main() {
463468
final initialRequest =
464469
currentNetworkRequests.getRequest('101')! as DartIOHttpRequestData;
465470
expect(initialRequest.status, '200');
466-
expect(initialRequest.isCancelled, false);
471+
expect(initialRequest.status, isNot('Cancelled'));
467472

468473
currentNetworkRequests.updateOrAddAll(
469474
requests: [request1CancelledWithStatusCode],
@@ -473,7 +478,6 @@ void main() {
473478

474479
final updatedRequest =
475480
currentNetworkRequests.getRequest('101')! as DartIOHttpRequestData;
476-
expect(updatedRequest.isCancelled, true);
477481
expect(updatedRequest.status, 'Cancelled');
478482
expect(updatedRequest.inProgress, false);
479483
});

‎packages/devtools_app/test/screens/network/network_model_test.dart‎

Lines changed: 42 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ void main() {
184184

185185
test('status returns correct value', () {
186186
expect(httpGet.status, '200');
187-
expect(httpGetWithError.status, 'Cancelled');
187+
expect(httpGetWithError.status, 'Error');
188188
expect(httpPost.status, '201');
189189
expect(httpPut.status, '200');
190190
expect(httpPatch.status, '200');
@@ -500,7 +500,7 @@ void main() {
500500

501501
test('didFail returns correct value', () {
502502
expect(httpGet.didFail, false);
503-
expect(httpGetWithError.didFail, false);
503+
expect(httpGetWithError.didFail, true);
504504
expect(httpPost.didFail, false);
505505
expect(httpPut.didFail, false);
506506
expect(httpPatch.didFail, false);
@@ -515,7 +515,7 @@ void main() {
515515
requestFullDataFromVmService: false,
516516
);
517517

518-
expect(cancelledRequest.isCancelled, true);
518+
expect(cancelledRequest.status, 'Cancelled');
519519
expect(cancelledRequest.inProgress, false);
520520
expect(cancelledRequest.status, 'Cancelled');
521521
expect(cancelledRequest.duration, isNotNull);
@@ -532,24 +532,26 @@ void main() {
532532
requestFullDataFromVmService: false,
533533
);
534534

535-
expect(inFlightRequest.isCancelled, true);
535+
expect(inFlightRequest.status, 'Cancelled');
536536
expect(inFlightRequest.inProgress, false);
537537
expect(inFlightRequest.status, 'Cancelled');
538-
expect(inFlightRequest.duration, Duration.zero);
538+
expect(inFlightRequest.duration, isNull);
539539
},
540540
);
541541

542-
test('request without response and endTime is cancelled', () {
543-
final pendingRequest = DartIOHttpRequestData(
544-
HttpProfileRequest.parse(Map<String, dynamic>.of(httpGetPendingJson))!,
545-
requestFullDataFromVmService: false,
546-
);
542+
test(
543+
'request without response and endTime is complete but not cancelled',
544+
() {
545+
final pendingRequest = DartIOHttpRequestData(
546+
HttpProfileRequest.parse(Map<String, dynamic>.of(httpGetPendingJson))!,
547+
requestFullDataFromVmService: false,
548+
);
547549

548-
expect(pendingRequest.isCancelled, true);
549-
expect(pendingRequest.inProgress, false);
550-
expect(pendingRequest.status, 'Cancelled');
551-
expect(pendingRequest.duration, isNotNull);
552-
});
550+
expect(pendingRequest.status, isNull);
551+
expect(pendingRequest.inProgress, false);
552+
expect(pendingRequest.duration, isNull);
553+
},
554+
);
553555

554556
test('request without response and null endTime remains pending', () {
555557
final pendingData = Map<String, dynamic>.of(httpGetPendingJson)
@@ -560,7 +562,7 @@ void main() {
560562
requestFullDataFromVmService: false,
561563
);
562564

563-
expect(pendingRequest.isCancelled, false);
565+
expect(pendingRequest.status, isNot('Cancelled'));
564566
expect(pendingRequest.inProgress, true);
565567
expect(pendingRequest.status, isNull);
566568
expect(pendingRequest.duration, isNull);
@@ -579,7 +581,7 @@ void main() {
579581
requestFullDataFromVmService: false,
580582
);
581583

582-
expect(cancelledWithStatusRequest.isCancelled, false);
584+
expect(cancelledWithStatusRequest.status, isNot('Cancelled'));
583585
expect(cancelledWithStatusRequest.inProgress, false);
584586
expect(cancelledWithStatusRequest.status, '200');
585587
expect(cancelledWithStatusRequest.didFail, false);
@@ -599,10 +601,10 @@ void main() {
599601
requestFullDataFromVmService: false,
600602
);
601603

602-
expect(cancelledDuringResponseRequest.isCancelled, true);
604+
expect(cancelledDuringResponseRequest.status, 'Cancelled');
603605
expect(cancelledDuringResponseRequest.inProgress, false);
604606
expect(cancelledDuringResponseRequest.status, 'Cancelled');
605-
expect(cancelledDuringResponseRequest.duration, Duration.zero);
607+
expect(cancelledDuringResponseRequest.duration, isNotNull);
606608
});
607609

608610
test(
@@ -623,7 +625,7 @@ void main() {
623625
requestFullDataFromVmService: false,
624626
);
625627

626-
expect(cancelledDuringResponseRequest.isCancelled, false);
628+
expect(cancelledDuringResponseRequest.status, isNot('Cancelled'));
627629
expect(cancelledDuringResponseRequest.status, '200');
628630
},
629631
);
@@ -642,7 +644,7 @@ void main() {
642644
requestFullDataFromVmService: false,
643645
);
644646

645-
expect(cancelledRequest.isCancelled, false);
647+
expect(cancelledRequest.status, isNot('Cancelled'));
646648
expect(cancelledRequest.status, '200');
647649
});
648650

@@ -659,26 +661,29 @@ void main() {
659661
requestFullDataFromVmService: false,
660662
);
661663

662-
expect(cancelledRequest.isCancelled, true);
664+
expect(cancelledRequest.status, 'Cancelled');
663665
expect(cancelledRequest.status, 'Cancelled');
664666
});
665667

666-
test('request with request error and incomplete response is cancelled', () {
667-
final requestData = Map<String, dynamic>.of(
668-
httpGetJson['request'] as Map<String, Object?>,
669-
)..['error'] = 'SocketException: connection reset by peer';
668+
test(
669+
'request with non-cancellation error and no response has error status',
670+
() {
671+
final requestData = Map<String, dynamic>.of(
672+
httpGetJson['request'] as Map<String, Object?>,
673+
)..['error'] = 'SocketException: connection reset by peer';
670674

671-
final cancelledData = Map<String, dynamic>.of(httpGetJson)
672-
..['request'] = requestData
673-
..['response'] = null;
675+
final errorData = Map<String, dynamic>.of(httpGetJson)
676+
..['request'] = requestData
677+
..['response'] = null;
674678

675-
final cancelledRequest = DartIOHttpRequestData(
676-
HttpProfileRequest.parse(cancelledData)!,
677-
requestFullDataFromVmService: false,
678-
);
679+
final errorRequest = DartIOHttpRequestData(
680+
HttpProfileRequest.parse(errorData)!,
681+
requestFullDataFromVmService: false,
682+
);
679683

680-
expect(cancelledRequest.isCancelled, true);
681-
expect(cancelledRequest.inProgress, false);
682-
expect(cancelledRequest.status, 'Cancelled');
683-
});
684+
expect(errorRequest.status, 'Error');
685+
expect(errorRequest.status, isNot('Cancelled'));
686+
expect(errorRequest.inProgress, false);
687+
},
688+
);
684689
}

‎packages/devtools_app/test/test_infra/test_data/http_get_cancelled_json.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,5 @@ const httpGetCancelledJson = <String, dynamic>{
3030
'persistentConnection': true,
3131
'uri': 'https://jsonplaceholder.typicode.com/albums/1',
3232
},
33-
'response': null, // ← key: no response
33+
'response': null,
3434
};

0 commit comments

Comments
 (0)