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

Most upvoted comments

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.