[Networking] Ability to set OkHttpClient timeouts

[atticoos]

I noticed the default behavior of the OkHttpClient is no timeouts.

Since a lot of applications rely on responding to outcome of a connection, this can leave apps in a long waiting period if there's an issue with the connection where a timeout could otherwise kill it after, say 30 seconds.

It appears that a timeout can be configured at the request level by cloning the client and applying the timeout settings to it. However, everything is interfaced through fetch, which doesn't expose this type of configuration. The alternative would be to allow configuration of the singleton instance's timeouts.

While awaiting for a possible permanent solution, I may update these values in my build to use 30 seconds.

---

Theory: If too many connections hang without a timeout, would this mean they never close? If there's any sort of max client connections -- wouldn't this mean that there could become an infinite growing queue if too many connections are hung without a timeout?

Taking a look at OkHTTP's ConnectionPool the default number of connections is 5. Without a timeout, I believe this could lead to a clogged HTTP client that would end up hanging. I've experienced this in my app; it might be good to get some timeout in here.

[philikon]

We should probably just implement timeout and ontimeout on XHR (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout), which is exactly what #4832 is aiming at -- though currently that PR implements the timeout behaviour in JS rather than setting the properties on the underlying native library (IMHO it should do the latter.)

As far as the fetch API is concerned, it looks like there isn't an option for timeouts (though filed as an enhancement here: whatwg/fetch#179), so using XHR is probably going to be better for this use case. That said, our own fetch polyfill (https://github.com/facebook/react-native/blob/master/Libraries/Fetch/fetch.js) could probably extend the fetch API with timeout functionality.

[atticoos]

It would be ideal to have things handled natively, but as we've both noticed, we don't have a clear interface to work with since we're behind fetch.

Nice to see #4832's attempt with the timeout on the JS side, I wasn't aware of this PR. Thanks for linking & explaining.

[philikon]

we don't have a clear interface to work with since we're behind fetch.

What does that mean? You don't have to use fetch, you know. You can use just XMLHttpRequest. A bit clunkier, but gives you more flexibility. RN's fetch implementation is just a wrapper around XHR anyway.

[fabianeichinger]

It is possible (albeit maybe prone to break in future RN releases) to get the singleton OkHttpClient instance used by React via OkHttpClientProvider.getOkHttpClient() and reconfigure it before the first use-site as it is not immutable (but should be treated as such after first use).

But I'd also like to see per-request control over timeouts from JS.

[atticoos]

This should have been introduced in 0.19.0 via 1303e6d

Related to #4648.

[grabbou]

Going to close that one as it happens to be implemented in the commit linked by @ajwhite. If anyone needs ontimeout event (if it's not there yet), please consider filling in a new issue / product pains or ideally - try implementing it.