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

Body timeout not working #3297

Closed
caldrjir opened this issue May 24, 2024 · 5 comments · Fixed by #3324
Closed

Body timeout not working #3297

caldrjir opened this issue May 24, 2024 · 5 comments · Fixed by #3324
Labels
bug Something isn't working

Comments

@caldrjir
Copy link

Bug Description

Our request below should fail on 'BodyTimeoutError', instead it seems to be an infinite timeout.

It happens mostly with bodyTimeout params higher than 500ms.

There is an example URL, in the code bellow, but it also happens with others.

Reproducible By

Run this code:

const fs = require('node:fs');
const { pipeline } = require('node:stream/promises');
const { request } = require('undici');

async function test() {
    const {body} = await request('http://79.8.16.194/cgi-bin/faststream.jpg?stream=full', {
        throwOnError: true,
        bodyTimeout: 1000,
    });
    await pipeline(body, fs.createWriteStream('./tmp/body-timeout-test'));
}

test();

Expected Behavior

Expecting BodyTimeoutError after 1000ms

BodyTimeoutError: Body Timeout Error
    at Timeout.onParserTimeout [as _onTimeout] (/YOUR_FOLDER_PATH/node_modules/undici/lib/dispatcher/client-h1.js:626:28)
    at listOnTimeout (node:internal/timers:575:11)
    at process.processTimers (node:internal/timers:514:7) {
  code: 'UND_ERR_BODY_TIMEOUT'
}

Environment

  • Node v21.2.0
  • NPM v10.5.1
  • macOS - Ventura 13.5.2
@caldrjir caldrjir added the bug Something isn't working label May 24, 2024
@mcollina
Copy link
Member

Can you please include a server as well in your example? Otherwise reproducing it is network dependent.

@sandratatarevicova
Copy link

See the example below.

const fs = require('node:fs');
const { pipeline } = require('node:stream/promises');
const http = require('node:http');
const { request } = require('undici');

const server = http.createServer((req, res) => {
    res.writeHead(200);
    setTimeout(() => {
        res.end();
        console.log('Request closed after 10 seconds');
    }, 10000);
    const writeZeroes = () => {
        res.write(Buffer.alloc(1024, 0));
        setImmediate(writeZeroes);
    };
    writeZeroes();
});

server.listen();
const serverUrl = `http://localhost:${server.address().port}`;
console.log(`Server listening on ${serverUrl}`);

async function test() {
    const { body } = await request(serverUrl, {
        throwOnError: true,
        bodyTimeout: 1000,
    });
    console.log('Reading body...');
    await pipeline(body, fs.createWriteStream('/tmp/body-timeout-test'));
    console.log('Reading body finished');
}

test();

Output:

Server listening on http://localhost:63483
Reading body...
Request closed after 10 seconds
Reading body finished

Reading the response body should timeout after 1 second, but it does not.

@metcoder95
Copy link
Member

metcoder95 commented May 28, 2024

If I'm not mistaken, the bodyTimeout is only applied on idle situations between receiving body's data.
As you keep writing to the socket in an interval, and for instance the client keeps receiving data from the server, the timeout is reset on every new chunk, delaying its effect.

e.g. if I alter your example like this:

const fs = require('node:fs')
const { pipeline } = require('node:stream/promises')
const http = require('node:http')
const { request } = require('undici')

const server = http.createServer((req, res) => {
  res.writeHead(200)
  setTimeout(() => {
    res.end()
    console.log('Request closed after 10 seconds')
  }, 10000).unref()
  const writeZeroes = () => {
    res.write(Buffer.alloc(1024, 0))
    setInterval(writeZeroes, 2000)
  }
  writeZeroes()
})

server.listen(0, () => {
  const serverUrl = `http://localhost:${server.address().port}`
  console.log(`Server listening on ${serverUrl}`)

  async function test () {
    console.log('Sending request...')
    const { body } = await request(serverUrl, {
      throwOnError: true,
      bodyTimeout: 1000
    })
    console.log('Reading body...')
    await pipeline(body, fs.createWriteStream('/tmp/body-timeout-test'))
    console.log('Reading body finished')
  }

  test()
})

Then the timeout will effectively apply.

You can use AbortSignal.timeout to enforce a specific timeout independent of the bodyTimeout one

@sandratatarevicova
Copy link

Thank you, we will try the AbortSignal, but I think the documentation is a bit misleading:

  • 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 300 seconds.

Based on this, I would expect the request to be aborted after the timeout.

@metcoder95
Copy link
Member

See what you mean, a PR to support us improving the docs is always welcomed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants