destroy request on error to avoid hanging sockets#1141
destroy request on error to avoid hanging sockets#1141benzekrimaha wants to merge 5 commits intodevelopment/1.3from
Conversation
On timeout or other errors the socket was left open; destroy the request so the socket is properly closed.
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
Issue: HD-4352
maeldonn
left a comment
There was a problem hiding this comment.
Could you add a test that reproduces the original problem (socket left open on error) and verifies it is now fixed?
Yeah if you are able to add a little test that counts connections or something it's nice, but might be hard I don't know |
ce87e45 to
9cc19c9
Compare
Stub http.request to emit a synthetic error after response; assert destroy() is invoked with the expected err.code. Issue: HD-4352
9cc19c9 to
7ab390a
Compare
maeldonn
left a comment
There was a problem hiding this comment.
If @SylvainSenechal comment is right, we're missing the most important test.
9502b41 to
d80775c
Compare
Assert destroy call counts in post-response and stream-error paths to guard against double-destroy regressions. Issue: HD-4352
1df5dd9 to
6d878a0
Compare
maeldonn
left a comment
There was a problem hiding this comment.
LGTM. Just two small comments/questions.
Unify destroy-capture stubs with optional post-response error injection to reduce duplication and keep tests clearer. Issue: HD-4352
On timeout or other errors the socket was left open; we now destroy the request so the socket is properly closed.
Issue: HD-4352