Should we always time out?

[glasserc]

At present, all requests have a timeout which defaults to 5 seconds. Why? I know the Python requests library says that all requests should have timeouts, but I think the situation here is different:

• "If no timeout is specified explicitly, requests do not time out." I don't think that's true for us; I think the underlying browser will always time out a connection, right?
• "timeout is not a time limit on the entire response download; rather, an exception is raised if the server has not issued a response for timeout seconds (more precisely, if no bytes have been received on the underlying socket for timeout seconds)." But we're explicitly setting a timeout on the entire response download, so we won't just kill dead connections, but ones that are continuing to receive data very slowly.

Indeed, I bet 5 seconds is much too short for many interesting syncing problems that have to happen over the public Internet. I know we have the occasional test failure in Firefox for Android (such as bug 1329069) for this reason, which is over a local connection.

The obvious fix here is to just increase the length of the timeout to something that seems reasonable, but that doesn't really solve the problem so much as move it around. Or I could turn off the timeout for certain requests, but first I'd like to better understand why we'd want a timeout in the first place and under what circumstances it might or might not be appropriate. Is it for interactive use, maybe? Why is it better to have a request eventually time out? Does that let the user react in some way? Or would an application benefit from it somehow?

[Natim]

Thank you for brigging this on.

We need timeout to be able to tell people if their request was successful or not.

For kinto-http.js 5 seconds seems a good amount of time because for instance when you are using the kinto-admin 5 seconds looks like ages when you have to wait for your request.

However with kinto.js 5 seconds looks a bit short especially on batch request that should be able to take 5 seconds per underlying requests. (5*25 ~ 2 minutes)

So IMHO:

1. we should put timeout as soon as we talk to a remote server just to handle the case where it is not responding.
2. For request users are waiting on in the UX it should be short (5 seconds)
3. For requests in the background (for offline first clients) that implies large synchronization it can be much longer (120 seconds)

In anycase we should never timeout while reading the response.

The baseline is always explicit the communication contract.

[glasserc]

So if I understood correctly, we time out requests to show that kinto-admin isn't broken, but rather that it's the server. This seems kind of limited to me. A user can't choose to cancel earlier than five seconds, nor can they choose not to cancel until much longer (if they know the request is going to take a long time). We're already returning a promise, so it's not like client code is blocked until we get a response. kinto-admin can let the user click a different link and fire off a different request any time, which will start up a new fetch. That's essentially the same as what we do right now anyhow, because we don't cancel or clean up the fetch in any way in our timeout case.[1] If a client wants to timeout responses, they can always choose to race the promise against a timeout themselves.[2]

Of course, even if a client wants to use this timeout functionality when using e.g. kinto.js's Collection#sync, they have to guess how many requests they think the sync is going to take, and put in a timeout that makes sense for each one of those requests!

No timeout we can implement will respect whether timeouts happen while trying to connect to the server, issuing the request, waiting for a response, or reading a response. See also whatwg/fetch#20.

Of course, I got into all of this because of the orange-hunter documentation, which recommends against Using magical timeouts to cause delays.

Based on this, I'd argue that we should not have timeouts in this library at all. I'm going to open a PR taking the timeouts out, and open an issue on kinto-admin to add them back (or replace them with a cancel button).

[1] The current issue tracking how to cancel fetches is whatwg/fetch#447, but see also whatwg/fetch#27 and w3c/ServiceWorker#625.
[2] I'd say they should use a cancellable promise but apparently there are no such things as cancellable promises and we can't have nice things because this is JS. See also https://www.reddit.com/r/javascript/comments/5j3tn6/tc39_cancellable_promises_proposal_has_been/.

[Natim]

A user can't choose to cancel earlier

As far as I know, at least today, a fetch command promise cannot be cancelled that why we are relying on timeouts.

[n1k0]

I'm going to open a PR taking the timeouts out, and open an issue on kinto-admin to add them back (or replace them with a cancel button).

I actually like this idea a lot. It's up to apps to make these decisions according to their target execution platforms, environments, and other constraints.

Still I think we should keep the ability for lib users to define a default request timeout. We should probably just remove the 5s value we use and not timing out at all by default.

[mhammond]

FWIW, Sync uses a 5 minute timeout on storage requests.

[leplatrem]

Done.