undici: Requests does not timeout when bodyTimeout & headersTimeout have values lower than 1 second
We have an HTTP proxy were we are using fastify and @fastify/reply-from. In order the fix some vulnerabilities for undici, we updated to the latest version and had some failed tests.
The issue is that some requests does not timeout when bodyTimeout & headersTimeout have values lower than 1 second. We identified that the issue is related to undici@5.18.0, when faster timers where introduced (especially 1 second value used fast timer timeout - https://github.com/nodejs/undici/blob/main/lib/timers.js#L44).
In order to test this, we added a new test in https://github.com/nodejs/undici/blob/main/lib/agent.js. We run tests using Node 18.14.1.
test('agent should timeout request (lower value timeouts)', t => {
t.plan(4)
const timeout = 1000
const wanted = 'payload'
const server = http.createServer((req, res) => {
t.equal('/', req.url)
t.equal('GET', req.method)
t.equal(`localhost:${server.address().port}`, req.headers.host)
res.setHeader('Content-Type', 'text/plain')
setTimeout(() => {
res.end(wanted)
}, timeout * 3)
})
t.teardown(server.close.bind(server))
server.listen(0, () => {
const dispatcher = new Agent({ bodyTimeout: timeout, headersTimeout: timeout })
request(`http://localhost:${server.address().port}`, { dispatcher })
.then(({ statusCode, headers, body }) => {
t.fail('Request should timeout')
})
.catch(err => {
t.type(err, errors.HeadersTimeoutError)
})
})
})
π SUMMARY RESULTS π
Suites: 87 passed, 1 skip, 88 of 88 completed
Asserts: 7382 passed, 8 skip, of 7390
Time: 29s
This test will pass because the timeout is 1000 ms.
If we change the timeout to 100 ms, this test will fail.
FAIL test/agent.js
β Request should timeout
test/agent.js
191 | request(`http://localhost:${server.address().port}`, { dispatcher })
192 | .then(({ statusCode, headers, body }) => {
> 193 | t.fail('Request should timeout')
| ----------^
194 | })
195 | .catch(err => {
196 | t.type(err, errors.HeadersTimeoutError)
test: test/agent.js agent should timeout request (lower value timeouts)
stack: |
test/agent.js:193:11
π SUMMARY RESULTS π
FAIL test/agent.js 1 failed of 133 4s
β Request should timeout
Suites: 1 failed, 86 passed, 1 skip, 88 of 88 completed
Asserts: 1 failed, 7381 passed, 8 skip, of 7390
Time: 30s
If we change https://github.com/nodejs/undici/blob/main/lib/timers.js#L44 timeout value to 100ms, the test will pass.
Iβm not sure what the correct fix should be and this is the reason why Iβve created an issue and not a PR. I hope itβs more clear to you and a fix is easy to implement!
Thank you in advance π
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 20 (10 by maintainers)
Commits related to this issue
- fix: use normal timers for delays < 1s Fixes: https://github.com/nodejs/undici/issues/1950 — committed to nodejs/undici by ronag a year ago
- fix: use normal timers for delays < 1s Fixes: https://github.com/nodejs/undici/issues/1950 — committed to nodejs/undici by ronag a year ago
- fix: use normal timers for delays < 1s (#1961) Fixes: https://github.com/nodejs/undici/issues/1950 — committed to nodejs/undici by ronag a year ago
- fix: use normal timers for delays < 1s (#1961) Fixes: https://github.com/nodejs/undici/issues/1950 — committed to metcoder95/undici by ronag a year ago
- fix: use normal timers for delays < 1s (#1961) Fixes: https://github.com/nodejs/undici/issues/1950 — committed to crysmags/undici by ronag a year ago
Thanks, the use case is helpful: now I understand the problem.
@ronag I think we might want to put the new fast timers behind an option, or possibly have a way to handle this situation without a regression
I think the easiest is to not use fast timers if the timeout is set to something less than 1 second.