Powerful timeouts

[kornelski]

Currently there's a single timeout option that sets a deadline for the entire request to complete.

This is suboptimal for large files and slow connections, because it doesn't distinguish between the server not responding at all, and the file downloading reliably, but slowly.

In reality there are multiple stages of the request that can time out:

• DNS timeout
• TCP/IP connection timeout
• File upload
• Waiting for HTTP response headers
• Waiting for each chunk of HTTP response body

I'm thinking of hooking into more of these steps to allow having shorter timeouts as appropriate, e.g. DNS and TCP/IP connection never take more than a few seconds, even if the whole request needs minutes to download.

What should be the API for this? Should we try to interpret existing .timeout(2000) API in some clever way?

[focusaurus]

👎 for clever API interpretations and overloading. My first thought would be leave .timeout(2000) as an alias for the entire request completing including the full body, and introduce APIs like .resolveTimeout(500), .connectTimeout(1000), .headerTimeout(3000), .chunkTimeout(4000).

[kornelski]

By overloading, do you mean .timeout(Number) vs .timeout({connect:…, chunk:…})?

[kornelski]

Also, should we have any defaults?

[focusaurus]

Yes, overloading meaning a single API function call that has multiple meanings or behaviors depending on arguments provided. I think in general that's an antipattern and having distinct API functions that do 1 clear thing is preferable, even if it means the API surface is bigger (it's just being honest about the actual API surface).

I don't mind .timeout({connect: 42, chunk: 97}) that much though. It's not terrible. It just requires good validation and I think it's easier to document and more obvious when you typo .conectTimeout(42) and get an obvious exception vs calling .timeout({conect: 42}) and maybe having to debug why the timeout seems to not be taking effect as expected. If the code detects unexpected option names and throws an exception immediately, it's close enough I think.

I think we should have no defaults and do what the runtimes/node/oses do by default.

[kornelski]

The MVP of response timeout seems to work well enough, so I'll leave it at that.