Skip to content

Commit

Permalink
Fix hanging promise on HTTP/2 timeout
Browse files Browse the repository at this point in the history
Fixes #1492
  • Loading branch information
szmarczak committed Apr 11, 2021
1 parent 45b89f5 commit a59fac4
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"cacheable-request": "^7.0.1",
"decompress-response": "^6.0.0",
"get-stream": "^6.0.0",
"http2-wrapper": "^2.0.1",
"http2-wrapper": "^2.0.3",
"lowercase-keys": "^2.0.0",
"p-cancelable": "^2.0.0",
"responselike": "^2.0.0"
Expand Down
5 changes: 5 additions & 0 deletions source/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,11 @@ export default class Request extends Duplex implements RequestEvents<Request> {

timer(request);

if (this.options.http2) {
// Unset stream timeout, as the `timeout` option was used only for connection timeout.
request.setTimeout(0);
}

this._cancelTimeouts = timedOut(request, timeout, url as URL);

const responseEventName = options.cache ? 'cacheableResponse' : 'response';
Expand Down
13 changes: 12 additions & 1 deletion source/core/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,16 @@ const cloneInternals = (internals: typeof defaultInternals): typeof defaultInter
return result;
};

const getHttp2TimeoutOption = (internals: typeof defaultInternals): number | undefined => {
const delays = [internals.timeout.socket, internals.timeout.connect, internals.timeout.lookup, internals.timeout.request, internals.timeout.secureConnect].filter(delay => typeof delay === 'number') as number[];

if (delays.length > 0) {
return Math.min(...delays);
}

return undefined;
};

const descriptor = {
_unixOptions: {
value: undefined,
Expand Down Expand Up @@ -2119,7 +2129,8 @@ export default class Options {
maxHeaderSize: internals.maxHeaderSize,
localAddress: internals.localAddress,
headers: internals.headers,
createConnection: internals.createConnection
createConnection: internals.createConnection,
timeout: internals.http2 ? getHttp2TimeoutOption(internals) : undefined
};
}

Expand Down
14 changes: 14 additions & 0 deletions test/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -763,3 +763,17 @@ test('timeouts are emitted ASAP', async t => {

t.true(error.timings.phases.total! < (timeout + marginOfError));
});

test('http2 timeout', async t => {
await t.throwsAsync(got('https://123.123.123.123', {
timeout: {
request: 1
},
http2: true,
retry: {
calculateDelay: ({computedValue}) => computedValue ? 1 : 0
}
}), {
code: 'ETIMEDOUT'
});
});

0 comments on commit a59fac4

Please sign in to comment.