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

Default fetch timeout too short #1373

Open
kyrylkov opened this issue Apr 26, 2022 · 16 comments
Open

Default fetch timeout too short #1373

kyrylkov opened this issue Apr 26, 2022 · 16 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@kyrylkov
Copy link
Contributor

By default fetch times out after 30 seconds. This causes issues like #1248

Chrome has 300 second time out.

Please set default unidici fetch timeout to 300 seconds by default.

@kyrylkov kyrylkov added the enhancement New feature or request label Apr 26, 2022
@ronag ronag added the good first issue Good for newcomers label Apr 26, 2022
@kyrylkov
Copy link
Contributor Author

kyrylkov commented Apr 26, 2022

@ronag Would it make sense to change default request timeout undici-wide to 120 seconds (presumably like in Node.js) or 300 seconds (like in Chrome)?

@mcollina
Copy link
Member

What's the current timeout?

@kyrylkov
Copy link
Contributor Author

30 seconds

@mcollina
Copy link
Member

Sure thing, let's bring it at least to 120 to match Node.js.

@kyrylkov
Copy link
Contributor Author

#1384

@kibertoad
Copy link
Contributor

Now that #1386 was merged, should this be closed?

@kyrylkov
Copy link
Contributor Author

kyrylkov commented Feb 10, 2023

The default time out is only 30 seconds again? I don't see 5 minutes anymore set in lib/fetch/index.js.

@mcollina

@kyrylkov kyrylkov reopened this Feb 10, 2023
@ronag
Copy link
Member

ronag commented Feb 10, 2023

The default time out is only 30 seconds again? I don't see 5 minutes anymore set in lib/fetch/index.js.

@mcollina

That caused other problems... See #1870

@ronag
Copy link
Member

ronag commented Feb 10, 2023

I would suggest we bump the undici defaults then...

@kyrylkov
Copy link
Contributor Author

kyrylkov commented Feb 10, 2023

The problem is that we're using undici fetch bundled with Node.js. Is there even a way to override this in such case?

Setting to Chrome default 5 minutes would be great.

In the mean time, can we use undici package to override like this?

const { fetch, Agent } = require('undici');

fetch(webservicesUrl, {
  dispatcher: new Agent({
    bodyTimeout: 10 * 60e3, // 10 minutes
  }
});

@targos
Copy link
Member

targos commented Feb 10, 2023

You can make Node.js fetch use a custom dispatcher with:

globalThis[Symbol.for('undici.globalDispatcher.1')] = dispatcher;

@kyrylkov
Copy link
Contributor Author

kyrylkov commented Feb 10, 2023

@targos I'm sorry, that's above my level of knowledge. So how would I override the bodyTimeout with custom dispatcher and fetch bundled with Node.js?

@kyrylkov
Copy link
Contributor Author

@ronag #1924

@pether-b
Copy link

pether-b commented Mar 3, 2023

I'm also interested in learning how to override the headersTimeout when using Node's fetch(). I just tried

globalThis[Symbol.for("undici.globalDispatcher.1")] = new Agent({
  headersTimeout: 10000,
});

inspired by @targos ' suggestion above, and

http.globalAgent = new Agent({
  headersTimeout: 10000,
});

but none seemed to have any effect. I need a timeout longer than 300s but used 10s here just for testing. In both cases Agent is imported from undici as Node's Agent doesn't have the headersTimeout option available.

@cactysman
Copy link

This is really frustrating. For now I'm "patching" all of my occurrences of fetch with

import { fetch } from 'undici'

wherever I run requests that might validly take longer than 30 seconds to resolve.

@ralyodio
Copy link

This is becoming an issue with AI requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants