Timeout option collision

[floatdrop]

Well, it should have happened. Since we passing options to http(s).request methods – we have timeout option collision.

For now we move our option out from object – 52ddf95

In future we should use timeout option from Http module instead.

[alextes]

Your timed-out module currently supports a connection timeout which Node doesn't. Can you elaborate on this issue a little? Are you okay dropping it and just using the native timeout?

[sindresorhus]

@floatdrop ⬆️

[floatdrop]

@alextes timeout option in Node is for connection timeout. For whole body timeout there is p-timeout and for response timeout it will take a few lines more, but we never used in production this case, so I considering it not popular.

[floatdrop]

On second glance – timeout option is for socket timeout in Node, but there is no option for connection timeout.

I propose add separate option connectionTimeout.

[sindresorhus]

Is it really necessary to have that distinction? When will you ever need to set the connection timeout manually? What about a "whole body timeout"? That what I would care about. A timeout for the total request.

[floatdrop]

When will you ever need to set the connection timeout manually?

@sindresorhus quite often, actually. Connection timeout is very handy, when we have network outages between data-centres and need to retry request on reachable backend.

Is it really necessary to have that distinction?

They are quite different things and often confuse beginners. I'm good with setting timeout option for whole request timeout, but keep socketTimeout and connectionTimeout tuneable.

[sindresorhus]

They are quite different things and often confuse beginners. I'm good with setting timeout option for whole request timeout, but keep socketTimeout and connectionTimeout tuneable.

👍 Alright. Let's do that.