Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add read timeout #1518

Merged
merged 6 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 4 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,10 @@ This also accepts an `object` with the following fields to constrain the duratio
- `socket` starts when the socket is connected. See [request.setTimeout](https://nodejs.org/api/http.html#http_request_settimeout_timeout_callback).
- `response` starts when the request has been written to the socket and ends when the response headers are received.
- `send` starts when the socket is connected and ends with the request has been written to the socket.
- `request` starts when the request is initiated and ends when the response's end event fires.
- `request` starts when the request is initiated and ends when the response's `end` event fires.
- `read` starts when the `response` event is emitted and ends when the response's `end` event fires.
szmarczak marked this conversation as resolved.
Show resolved Hide resolved

**Note:** The `read` timeout is blocked by https://github.com/nodejs/node/issues/35923

###### retry

Expand Down
25 changes: 16 additions & 9 deletions source/core/utils/timed-out.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ interface TimedOutOptions {

export interface Delays {
lookup?: number;
socket?: number;
connect?: number;
secureConnect?: number;
socket?: number;
response?: number;
send?: number;
response?: number;
read?: number;

szmarczak marked this conversation as resolved.
Show resolved Hide resolved
request?: number;
}

Expand Down Expand Up @@ -89,14 +91,12 @@ export default (request: ClientRequest, delays: Delays, options: TimedOutOptions
}
});

request.once('close', cancelTimeouts);

once(request, 'response', (response: IncomingMessage): void => {
once(response, 'end', cancelTimeouts);
});

if (typeof delays.request !== 'undefined') {
addTimeout(delays.request, timeoutHandler, 'request');
const cancelTimeout = addTimeout(delays.request, timeoutHandler, 'request');

once(request, 'response', (response: IncomingMessage): void => {
once(response, 'end', cancelTimeout);
});
}

if (typeof delays.socket !== 'undefined') {
Expand Down Expand Up @@ -168,6 +168,13 @@ export default (request: ClientRequest, delays: Delays, options: TimedOutOptions
});
}

if (typeof delays.read !== 'undefined') {
once(request, 'response', (response: IncomingMessage): void => {
const cancelTimeout = addTimeout(delays.read!, timeoutHandler, 'read');
once(response, 'end', cancelTimeout);
});
}

return cancelTimeouts;
};

Expand Down
29 changes: 29 additions & 0 deletions test/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,35 @@ test.serial('no unhandled `Premature close` error', withServer, async (t, server
await delay(20);
});

// TODO: use fakeTimers here
test.serial.failing('`read` timeout - promise', withServer, async (t, server, got) => {
server.get('/', (_request, response) => {
response.write('o');
});

await t.throwsAsync(got({
timeout: {
read: 10
},
retry: 0
}), {message: 'Timeout awaiting \'read\' for 10ms'});
});

// TODO: use fakeTimers here
test.serial.failing('`read` timeout - stream', withServer, async (t, server, got) => {
server.get('/', (_request, response) => {
response.end('ok');
});

const stream = got.stream({
timeout: {
read: 10
}
});

await t.throwsAsync(pEvent(stream, 'end'), {message: 'Timeout awaiting \'read\' for 10ms'});
});

// TODO: use fakeTimers here
test.serial('cancelling the request removes timeouts', withServer, async (t, server, got) => {
server.get('/', (_request, response) => {
Expand Down