Skip to content

Commit 4e75679

Browse files
committed
Fix timings.end being undefined when stream is destroyed before completion
Fixes #2282
1 parent fb86418 commit 4e75679

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

source/core/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,20 @@ export default class Request extends Duplex implements RequestEvents<Request> {
545545
this._request.destroy();
546546
}
547547

548+
// Workaround: http-timer only sets timings.end when the response emits 'end'.
549+
// When a stream is destroyed before completion, the 'end' event may not fire,
550+
// leaving timings.end undefined. This should ideally be fixed in http-timer
551+
// by listening to the 'close' event, but we handle it here for now.
552+
// Only set timings.end if there was no error or abort (to maintain semantic correctness).
553+
const timings = (this._request as ClientRequestWithTimings)?.timings;
554+
if (timings && is.undefined(timings.end) && !is.undefined(timings.response) && is.undefined(timings.error) && is.undefined(timings.abort)) {
555+
timings.end = Date.now();
556+
if (is.undefined(timings.phases.total)) {
557+
timings.phases.download = timings.end - timings.response;
558+
timings.phases.total = timings.end - timings.start;
559+
}
560+
}
561+
548562
if (error !== null && !is.undefined(error) && !(error instanceof RequestError)) {
549563
error = new RequestError(error.message, error, this);
550564
}

test/timings.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,34 @@ test('http/2 timings', withHttpsServer(), async (t, server, got) => {
5454
t.true(phases.download! >= 0);
5555
t.true(phases.total! >= 0);
5656
});
57+
58+
test('timings.end is set when stream is destroyed before completion', withServer, async (t, server, got) => {
59+
server.get('/', (_request, response) => {
60+
response.write('chunk1');
61+
// Don't end the response, so it stays open
62+
});
63+
64+
await new Promise<void>((resolve, reject) => {
65+
const stream = got.stream('');
66+
67+
stream.on('data', () => {
68+
stream.destroy(new Error('Manual destroy'));
69+
});
70+
71+
stream.on('error', (error: Error) => {
72+
t.is(error.message, 'Manual destroy');
73+
t.truthy(stream.timings);
74+
t.truthy(stream.timings!.response);
75+
t.truthy(stream.timings!.end);
76+
t.true(stream.timings!.end! >= stream.timings!.response!);
77+
t.truthy(stream.timings!.phases.total);
78+
resolve();
79+
});
80+
81+
stream.on('end', () => {
82+
reject(new Error('Stream should not end normally'));
83+
});
84+
});
85+
86+
t.pass();
87+
});

0 commit comments

Comments
 (0)