Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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 @@ -50,11 +50,8 @@ ExpansionTile _buildTile(
class HttpRequestHeadersView extends StatelessWidget {
const HttpRequestHeadersView(this.data, {super.key});

@visibleForTesting
static const generalKey = Key('General');
@visibleForTesting
static const requestHeadersKey = Key('Request Headers');
@visibleForTesting
static const responseHeadersKey = Key('Response Headers');

final DartIOHttpRequestData data;
Expand Down Expand Up @@ -460,9 +457,7 @@ bool _isJsonDecodable(String source) {
class HttpRequestCookiesView extends StatelessWidget {
const HttpRequestCookiesView(this.data, {super.key});

@visibleForTesting
static const requestCookiesKey = Key('Request Cookies');
@visibleForTesting
static const responseCookiesKey = Key('Response Cookies');

final DartIOHttpRequestData data;
Expand Down Expand Up @@ -600,9 +595,7 @@ class NetworkRequestOverviewView extends StatelessWidget {

static const _keyWidth = 110.0;
static const _timingGraphHeight = 18.0;
@visibleForTesting
static const httpTimingGraphKey = Key('Http Timing Graph Key');
@visibleForTesting
static const socketTimingGraphKey = Key('Socket Timing Graph Key');

final NetworkRequest data;
Expand Down Expand Up @@ -721,7 +714,9 @@ class NetworkRequestOverviewView extends StatelessWidget {

Widget _buildHttpTimeGraph() {
final data = this.data as DartIOHttpRequestData;
if (data.duration == null || data.instantEvents.isEmpty) {
if (data.duration == null ||
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[nit] pull data.duration into a named variable (e.g. requestDuration) so that we don't need the ! null assertion operator (data.duration!)

data.duration!.inMicroseconds == 0 ||
data.instantEvents.isEmpty) {
return Container(
key: httpTimingGraphKey,
height: 18.0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,27 @@ class DartIOHttpRequestData extends NetworkRequest {

@override
Duration? get duration {
if (inProgress || !isValid) return null;
// Timestamps are in microseconds
return _endTime!.difference(_request.startTime);
if (_hasError) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are these changes for? It looks like the _endTime getter already takes into account whether or not there is an error so I don't think we need to change any of this logic.

final start = _request.startTime;
final end = _request.endTime;
return end?.difference(start);
}

// Cancelled request
if (_request.isRequestComplete &&
!_request.isResponseComplete &&
_request.response == null) {
return Duration.zero;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it could still be beneficial to include timing information even if the request was canceled - this matches what Chrome DevTools network panel does.

}

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 Down Expand Up @@ -227,11 +245,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 {
// Detect cancelled requests (request finished but response never arrived)
if (_request.isRequestComplete &&
!_request.isResponseComplete &&
_request.response == null) {
return false;
}

return !(_hasError
? _request.isRequestComplete
: _request.isResponseComplete);
}

/// All instant events logged to the timeline for this HTTP request.
List<DartIOHttpInstantEvent> get instantEvents {
Expand Down Expand Up @@ -301,8 +327,17 @@ class DartIOHttpRequestData extends NetworkRequest {
DateTime get startTimestamp => _request.startTime;

@override
String? get status =>
_hasError ? 'Error' : _request.response?.statusCode.toString();
String? get status {
if (_hasError) return 'Error';

if (_request.isRequestComplete &&
!_request.isResponseComplete &&
_request.response == null) {
return 'Cancelled';
}

return _request.response?.statusCode.toString();
}

@override
String get uri => _request.uri.toString();
Expand Down
3 changes: 1 addition & 2 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ TODO: Remove this section if there are not any updates.

## Network profiler updates

- Fix crash in the Network tab when viewing binary multipart request or
response bodies. [#9680](https://github.com/flutter/devtools/pull/9680)
- Fix cancelled HTTP requests incorrectly showing as "Pending" in the Network tab (#9683)

## Logging updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,5 +505,16 @@ void main() {
expect(httpPatch.didFail, false);
expect(httpWsHandshake.didFail, false);
});

test('cancelled request should not remain pending', () {
final cancelledRequest = DartIOHttpRequestData(
HttpProfileRequest.parse(httpGetPendingJson)!,
requestFullDataFromVmService: false,
);

expect(cancelledRequest.inProgress, false);
expect(cancelledRequest.status, 'Cancelled');
expect(cancelledRequest.duration, Duration.zero);
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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', () {
Expand All @@ -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'));
});
});
}