http: destroy timeout socket by Agent#23752
Conversation
e6b1b70 to
bb3eef5
Compare
|
@BridgeAR fixed follow your suggestions. |
89690c4 to
b3e5daa
Compare
There was a problem hiding this comment.
this is a break change, now timeout request will emit ERR_HTTP_SOCKET_TIMEOUT error if you don't handle the timeout event yourself.
b3e5daa to
9e8265e
Compare
|
@nodejs/http This could use some reviews. |
|
@nodejs/http bump.... 🔉 |
|
@nodejs/http @nodejs/http2 @nodejs/streams @ronag this could use some reviews. It should probably be reviewed in general idea wise before we ask for a rebase, since it took so long to look at this again. |
mcollina
left a comment
There was a problem hiding this comment.
This require a doc update, as our documentation for req.setTimeout states that
Emitted when the underlying socket times out from inactivity. This only notifies that the socket has been idle. The request must be aborted manually.
doc/api/errors.md
Outdated
There was a problem hiding this comment.
I think this should be ERR_HTTP_SOCKET_TIMEOUT, as this is triggered by http.
9e8265e to
52cb92d
Compare
Agent must destroy timeout socket when there is no any timeout handler. Avoid socket hang on forever when the server don't send any response back.
52cb92d to
be95679
Compare
| (reason phrase). | ||
|
|
||
| <a id="ERR_HTTP_SOCKET_TIMEOUT"></a> | ||
| ### ERR_HTTP_SOCKET_TIMEOUT |
There was a problem hiding this comment.
@mcollina change to ERR_HTTP_SOCKET_TIMEOUT and add update on request.setTimeout().
| agent.removeSocket(s, options); | ||
| debug('CLIENT active socket destroy'); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think just doing:
s.destroy(new ERR_HTTP_SOCKET_TIMEOUT());should be enough now in master
|
@fengmk2 My opinion here as matured a bit given recent changes. I would be more positive to this PR. Would you still be interested in sorting out the conflicts and comments? |
|
This seems to have been come stale. I'm closing it in favor of #33177 in order to try and land this change. |
Agent must destroy timeout socket when there is no any timeout
handler. Avoid socket hang on forever when the server don't send
any response back.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes