Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: don't timeout while waiting for client to send request #1604

Merged
merged 2 commits into from Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api/Client.md
Expand Up @@ -18,7 +18,7 @@ Returns: `Client`
### Parameter: `ClientOptions`

* **bodyTimeout** `number | null` (optional) - Default: `30e3` - The timeout after which a request will time out, in milliseconds. Monitors time between receiving body data. Use `0` to disable it entirely. Defaults to 30 seconds.
* **headersTimeout** `number | null` (optional) - Default: `30e3` - The amount of time the parser will wait to receive the complete HTTP headers. Defaults to 30 seconds.
* **headersTimeout** `number | null` (optional) - Default: `30e3` - The amount of time the parser will wait to receive the complete HTTP headers while not sending the request. Defaults to 30 seconds.
* **keepAliveMaxTimeout** `number | null` (optional) - Default: `600e3` - The maximum allowed `keepAliveTimeout` when overridden by *keep-alive* hints from the server. Defaults to 10 minutes.
* **keepAliveTimeout** `number | null` (optional) - Default: `4e3` - The timeout after which a socket without active requests will time out. Monitors time between activity on a connected socket. This value may be overridden by *keep-alive* hints from the server. See [MDN: HTTP - Headers - Keep-Alive directives](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive#directives) for more details. Defaults to 4 seconds.
* **keepAliveTimeoutThreshold** `number | null` (optional) - Default: `1e3` - A number subtracted from server *keep-alive* hints when overriding `keepAliveTimeout` to account for timing inaccuracies caused by e.g. transport latency. Defaults to 1 second.
Expand Down
2 changes: 1 addition & 1 deletion docs/api/Dispatcher.md
Expand Up @@ -199,7 +199,7 @@ Returns: `Boolean` - `false` if dispatcher is busy and further dispatch calls wo
* **blocking** `boolean` (optional) - Default: `false` - Whether the response is expected to take a long time and would end up blocking the pipeline. When this is set to `true` further pipelining will be avoided on the same connection until headers have been received.
* **upgrade** `string | null` (optional) - Default: `null` - Upgrade the request. Should be used to specify the kind of upgrade i.e. `'Websocket'`.
* **bodyTimeout** `number | null` (optional) - The timeout after which a request will time out, in milliseconds. Monitors time between receiving body data. Use `0` to disable it entirely. Defaults to 30 seconds.
* **headersTimeout** `number | null` (optional) - The amount of time the parser will wait to receive the complete HTTP headers. Defaults to 30 seconds.
* **headersTimeout** `number | null` (optional) - The amount of time the parser will wait to receive the complete HTTP headers while not sending the request. Defaults to 30 seconds.
* **throwOnError** `boolean` (optional) - Default: `false` - Whether Undici should throw an error upon receiving a 4xx or 5xx response from the server.

#### Parameter: `DispatchHandler`
Expand Down
17 changes: 15 additions & 2 deletions lib/client.js
Expand Up @@ -889,8 +889,10 @@ function onParserTimeout (parser) {

/* istanbul ignore else */
if (timeoutType === TIMEOUT_HEADERS) {
assert(!parser.paused, 'cannot be paused while waiting for headers')
util.destroy(socket, new HeadersTimeoutError())
if (!socket[kWriting] || socket.writableNeedDrain || client[kRunning] > 1) {
assert(!parser.paused, 'cannot be paused while waiting for headers')
util.destroy(socket, new HeadersTimeoutError())
}
} else if (timeoutType === TIMEOUT_BODY) {
if (!parser.paused) {
util.destroy(socket, new BodyTimeoutError())
Expand Down Expand Up @@ -1641,7 +1643,18 @@ class AsyncWriter {
this.bytesWritten += len

const ret = socket.write(chunk)

request.onBodySent(chunk)

if (!ret) {
if (socket[kParser].timeout && socket[kParser].timeoutType === TIMEOUT_HEADERS) {
// istanbul ignore else: only for jest
if (socket[kParser].timeout.refresh) {
socket[kParser].timeout.refresh()
}
}
}

return ret
}

Expand Down
16 changes: 3 additions & 13 deletions test/request-timeout.js
Expand Up @@ -18,37 +18,27 @@ const {
test('request timeout', (t) => {
t.plan(1)

const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const server = createServer((req, res) => {
setTimeout(() => {
res.end('hello')
}, 100)
clock.tick(100)
}, 1000)
})
t.teardown(server.close.bind(server))

server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 50 })
const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 })
t.teardown(client.destroy.bind(client))

client.request({ path: '/', method: 'GET' }, (err, response) => {
t.type(err, errors.HeadersTimeoutError)
})

clock.tick(50)
})
})

test('request timeout with readable body', (t) => {
t.plan(1)

const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const server = createServer((req, res) => {
clock.tick(100)
})
t.teardown(server.close.bind(server))

Expand All @@ -57,7 +47,7 @@ test('request timeout with readable body', (t) => {
t.teardown(() => unlinkSync(tempfile))

server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 50 })
const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 1e3 })
t.teardown(client.destroy.bind(client))

const body = createReadStream(tempfile)
Expand Down
48 changes: 48 additions & 0 deletions test/request-timeout2.js
@@ -0,0 +1,48 @@
'use strict'

const { test } = require('tap')
const { Client } = require('..')
const { createServer } = require('http')
const { Readable } = require('stream')

test('request timeout with slow readable body', (t) => {
t.plan(1)

const server = createServer(async (req, res) => {
let str = ''
for await (const x of req) {
str += x
}
res.end(str)
})
t.teardown(server.close.bind(server))

server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 50 })
t.teardown(client.close.bind(client))

const body = new Readable({
read () {
if (this._reading) {
return
}
this._reading = true

this.push('asd')
setTimeout(() => {
this.push('asd')
this.push(null)
}, 2e3)
}
})
client.request({
path: '/',
method: 'POST',
headersTimeout: 1e3,
body
}, async (err, response) => {
t.error(err)
await response.body.dump()
})
})
})