Improving timeout

[guybedford]

Timeout seems to handle the case of request / response delays, but once the response has started streaming, it offers no connection-level timeout during network interruption at that point.

The system defaults for fetch timeouts are so long, that the user experience is just terrible around this.

I think it's important that timeouts be incorporated into the core fetch itself, because wrapping a single entire fetch operation in a timeout doesn't take into account the fact that the request might just be going slowly, or be a large file transfer.

Ideally, the concept of timeout should be as simple as - if it has been greater than timeout since the last received packet, then abort and throw a timeout error.

That is to extend the timeout to not just the first request / response, but to instead treat every single packet response as resetting the timeout counter.

I understand work on timeout has been deferred to the AbortController, but as mentioned above wrapping the entire operation in an abort timeout is not adequate for the best user experience around this.

Would there be interest in a PR here? This is something I'd really like to see properly supported.

[bitinn]

Questions:

• Is there a case not covered by AbortController here?
• I mean, there is an argument to be made that AbortController isn't elegant, but why couldn't one create an easy-to-use timeout controller based on it?

I would support improvement to node-fetch without introducing more non-standard behaviors; but to be realistic, I don't curently have free time to review and merge PRs; so if someone is able to pick up the gauntlet of node-fetch 3.x, preferably with employer backing, I am happy to hand over the publishing right. (See #567)

[jimmywarting]

Create a helper function and do something like this:

const ctrl = new AbortController()
const abort = _.debounce(() => {
  ctrl.abort()
}, 1000)

const res = await fetch(url, { signal: ctrl.signal })
abort() // initiate abort countdown

for await (chunk of res.body) {
  abort() // resets the abort countdown
  // do something with chunk
}

this was only for download but you could be creative and do something for upload also by creating a body stream and resetting the abort timer in some transformer

[guybedford]

@jimmywarting yes that's exactly the type of behaviour.

In my case I want to expose the fetch API to a plugin system, without them having to "inject" this behaviour. So I'd effectively be wrapping the entire fetch API and doing a stream interception.

This seems to me like the way a core timeout should behave to begin with, so I can either write such a wrapper which I would always install directly instead of node-fetch (and the timeout in core can be deprecated), or I think we should consider this directly in node-fetch here as the default timeout behaviour.

[jimmywarting]

We can't change the default timeout to behave differently, it would be a breaking change for many
We want to follow the spec so that wrapper and other things can work well in both node and the browser

[guybedford]

Ok, since browser parity is the goal then deprecating timeout and having this be a wrapper sounds sensible. Will see how that work goes and share back here if there are any issues.