doc: emphasize "timeout" and "request.setTimeout()" difference #52576
Conversation
|
Review requested:
|
nodejs-github-bot
added
doc
| @@ -176,7 +176,7 @@ changes: | |||
| while the `'lifo'` scheduling will keep it as low as possible. | |||
| **Default:** `'lifo'`. | |||
| * `timeout` {number} Socket timeout in milliseconds. | |||
| This will set the timeout when the socket is created. | |||
| This will immediately set the timeout when the socket is created. | |||
There was a problem hiding this comment.
immediately is a really important adverb here.
doc/api/http.md
Outdated
| Calls [`socket.setTimeout()`][] on the socket assigned to this request | ||
| after that socket emits the `connect` event. | ||
|
|
||
| Unlike the `timeout` option on `http.Agent` and `http.request()`, the |
There was a problem hiding this comment.
Clearly describe the difference.
| @@ -3769,8 +3773,9 @@ changes: | |||
| request. | |||
| * `socketPath` {string} Unix domain socket. Cannot be used if one of `host` | |||
| or `port` is specified, as those specify a TCP Socket. | |||
| * `timeout` {number}: A number specifying the socket timeout in milliseconds. | |||
| This will set the timeout before the socket is connected. | |||
| * `timeout` {number}: Socket timeout in milliseconds. | |||
There was a problem hiding this comment.
Make the timeout option description consistent with the same option in http.Agent() constructor docs on the same page.
| * `timeout` {number}: A number specifying the socket timeout in milliseconds. | ||
| This will set the timeout before the socket is connected. | ||
| * `timeout` {number}: Socket timeout in milliseconds. | ||
| This is forwarded to the [`http.Agent`][], which will set the timeout |
There was a problem hiding this comment.
Explicitly mention this option is forwarded to http.Agent as-is. It's important to know.
There was a problem hiding this comment.
The request might not use an Agent at all.
There was a problem hiding this comment.
That's a good point. Wouldn't it use the defaultAgent in that case?
I believe the agent is always present, even if not provided explicitly. Correct me if I'm wrong (something has to handle the sockets!)
There was a problem hiding this comment.
No agent is used when the createConnection option is specified or when the .agent option is set to false
There was a problem hiding this comment.
The agent: false is tricky. I think that does mean to use a default agent. Setting agent: null, afaik, is for explicitly opting out from any agent.
But you are right, we shouldn't rely on the http.Agent always being present. Any objection if I rephrase it with "if present"?
I also wonder, what will happen with the timeout if the agent is not present?
There was a problem hiding this comment.
Any objection if I rephrase it with "if present"?
No objections, but I think the original wording is more general/correct.
I also wonder, what will happen with the
timeoutif the agent is not present?
The options are passed to net.createConnection() and the timeout option will work as expected. See https://nodejs.org/api/net.html#netcreateconnectionoptions-connectlistener and
Lines 233 to 235 in 3790d52
There was a problem hiding this comment.
I reverted the original description but kept the first line consistent between http.Agent and http.request.
kettanaito
changed the title
| * `timeout` {number}: A number specifying the socket timeout in milliseconds. | ||
| This will set the timeout before the socket is connected. | ||
| * `timeout` {number}: Socket timeout in milliseconds. | ||
| This is forwarded to the [`http.Agent`][], which will set the timeout |
There was a problem hiding this comment.
The request might not use an Agent at all.
Co-authored-by: Luigi Pinca <[email protected]>
This change brings emphasis on the difference between these two ways of setting a socket timeout on an HTTP request:
The two are completely different, and the current state of the docs failed to highlight that difference when I was debugging an issue.
timeoutoption onhttp.request()is forwarded to thehttp.Agentand applies timeout immediately when the socket is created.request.setTimeout()method is not forwarded to thehttp.Agent, and applies timeout after the socket has connected.