[whatwg/fetch] Add a `timeout` option (#951)

I'd like to propose adding a `timeout` option to the Fetch API.

**Prior issue:** I know this was in #20, but that discussion was long and winding. One of the blockers was aborting requests, which was a big rabbit hole, but is now solved with `AbortController`! So I'd like to open this fresh issue to discuss the importance and ergonomics of timeouts. 

Please bare with me! I just want a blank slate to argue the case…

## Timeouts are critical!

Before jumping to implementation, it's important to understand how critical timeouts are. Since TCP/HTTP is inherently unpredictable, you can't actually guarantee that any call to `fetch` will ever resolve or reject. It can very easily just hang forever waiting for a response. You have no guarantee of a response.

**Any HTTP request without a timeout might hang your code forever.**

That should scare you if you're trying to write stable code.

The solution for this uncertainty is timeouts. As long as you have a timeout, you've returned to a state of certainty. The request might fail, sure, but at least you're now guaranteed not to be left in an infinitely hanging state.

I think Python's [`requests` documentation](https://requests.readthedocs.io/en/latest/user/quickstart/#timeouts) sums up the severity nicely (emphasis mine):

> You can tell Requests to stop waiting for a response after a given number of seconds with the `timeout` parameter. **Nearly all production code should use this parameter in nearly all requests. Failure to do so can cause your program to hang indefinitely.**

I think it underscores just how widespread the use case is. If your code is striving to be stable and bug-free, then every call to `fetch` should have an associated timeout.

But to make that goal possible, specifying a timeout needs to be **easy**. Right now it's not.

**Prior concern:** In #20, people brought up other types of timeouts, like "idle timeouts", which ensure that at least _some_ data is received for a request every interval of time. These can be useful, but they are definitely not the 90% case. **And importantly, they don't actually eliminate the infinite hanging uncertainty.** Other types of timeouts can either be implemented in userland or taken up in a separate issue if there's a need.

## `AbortController` is great, but not enough.

The `AbortController` and `AbortSignal` features are great! And they seem really well designed for use across different APIs and for low-level use cases. I have no qualms with them at all, and I think they are perfect to use to make a `timeout` property available.

But I don't think that just adding them has solved "timeouts" for Fetch. Because they aren't ergonomic enough to offer a good solution for the common case.

Right now you'd need to do...

```js
const controller = new AbortController()
const signal = controller.signal
const fetchPromise = fetch('https://example.com', { signal })
const timeoutId = setTimeout(() => controller.abort(), 5000)
const response = await fetchPromise
const body = await response.json()
clearTimeout(timeoutId)
```

This is *a lot* of code. 

It's way to much code for a use case that is so common. (Remember, this is something that should be done for almost *every* call to `fetch`.)

And it's easy to get wrong! 

Most people want to wait for the entire body to be received (notice how the timeout is cleared after `response.json()`), but most examples of using `AbortController` in the wild right now *do not* properly handle this case, leaving them in an uncertain state.

Not only that, but prior to `AbortController` people would use libraries like `p-timeout` and almost certainly added unexpected bugs because it is common to see people recommend things like:

```js
const res = pTimeout(fetch('https://example.com'), 5000)
const body = await res.json()
```

That example also has the potential to hang forever!

## What's the ideal UX?

People are currently using either no timeouts, or incorrectly written timeouts that still leave their code in a potentially infinitely hanging state. And these subtle bugs are only getting more common as more and more people switch to using the `isomorphic-fetch` or `node-fetch` packages.

I think this is something that `fetch` should solve. And to do that, we really need something as simple as:

```js
fetch('https://example.com', {
  timeout: 5000
})
```

And ideally that should "just work" as expected. Which means that if people are reading the body (and they often are), the timeout should apply to reading the body too.

**Prior concern:** In #20, people brought up that because `fetch` breaks responses down into "headers" and "body" across two promises, it's unclear what a `timeout` property should apply to. I think this is actually not a problem, and there's a good solution.

## A potential solution…

For the `timeout` option to match user expectations, it needs to incorporate reading the full body. This is just how people think about HTTP responses—it's the 90% use case. They will absolutely expect that `timeout` encompasses time to read the entire response they're working with.

However! Keep reading before you assume things…

The decision does not have to be "either the timeout only applies to the headers, or it only applies to the body". It can just as easily apply to either just the headers, or both the headers and the body, in a way that ensures it always meets user expectations.

This is similar to how Go handles their default `Timeout` parameter:

> Timeout specifies a time limit for requests made by this `Client`. The timeout includes connection time, any redirects, and reading the response body. The timer remains running after `Get`, `Head`, `Post`, or `Do` return and will interrupt reading of the `Response.Body`. [Source](https://golang.org/pkg/net/http/#Client)

This is smart, because it allows `timeout` to adapt to how the user's code handles the response. If you read the body, the timeout covers it. If you don't you don't need to worry either.

Here's what that looks like in terms of where errors are thrown…

```js
const res = await fetch('https://example.com', { timeout: 5000 })
// Right now different types of network failures are thrown here.
//
// With a timeout property, timeout failures will also be thrown and 
// thrown here *if* the headers of the request have not been received 
// before the timeout elapses.
```
```js
const json = await res.json()
// Right now network failures *and* JSON parsing failures are thrown here.
// 
// With a timeout property, timeout failures will also be thrown and 
// thrown here *if* the headers of the request were received, but the
// body of the request was not also received within the timeout.
```

To summarize what's going on there in English...

The `timeout` property can be passed into `fetch` calls, which specifies a *total* duration for the read operations on a response. If only the headers are read, the timeout only applies to the headers. If both the headers and body are read the timeout applies to the headers and body.

## A real example.

So you can do:

```js
const example = () => {
  const res = await fetch('https://example.com', { timeout: 10000 })
  const json = await res.json()
  return json
}
```

Which guarantees you received that full JSON body in 10 seconds.

Or, if you don't care about the body...

```js
const example = () => {
  const res = await fetch('https://example.com', { timeout: 1000 })
  return res.ok
}
```

Which guarantees you receive the headers in 1 second.

This aligns well with user expectations and use cases. As far as a user is concerned, they can set a `timeout` property, and be covered from network uncertainty regardless of how much of the response they read. By setting `timeout`, they are saved from the infinitely hanging bug.

## In summary…

- Any call to `fetch` without a timeout is not guaranteed to resolve **or** reject.
- This is has been brought up in many, many issues in `fetch` polyfills.
- The existing ways to add timeouts to a `fetch` aren't ergonomic enough.
- This leads to subtle bugs and widespread incorrect example code.
- There's a good solution for a `timeout` option that meets user expectations.
- This solution eliminates network uncertainty, that's the goal.

Thanks for reading!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/issues/951

Received on Saturday, 12 October 2019 23:13:23 UTC