http: emit 'close' when sockets are not constructed#33818
http: emit 'close' when sockets are not constructed#33818rexagod wants to merge 2 commits intonodejs:masterfrom
Conversation
`OutgoingMessage#socket` is `null` for such cases at all times. Hence, we emit a 'close' event, imitating `destroyImpl.destroy`, without the socket operations. Fixes: nodejs#33653
Co-authored-by: James M Snell <jasnell@gmail.com>
|
ping @mcollina |
| }); | ||
| } | ||
|
|
||
| if (this instanceof OutgoingMessage) { |
There was a problem hiding this comment.
I added this to check for instances where no sockets were constructed, as in msg = new OutgoingMessage (similar to the code snippet in the original issue #33653).
We should always emit a close, eventually... Though I don't think the implementation in this PR is correct. When we do get a socket Just so that I understand a bit better... the current code assumes that a socket will always was be assigned eventually... in which case is this false? The test is not a good example since it's never actually used like that. |
ronag
left a comment
ronag
left a comment
There was a problem hiding this comment.
I don't think this is correct, se above comment.
5 participants
OutgoingMessage#socketisnullfor such cases at all times.Hence, we emit a 'close' event, imitating
destroyImpl.destroy,without the socket operations.
Fixes: #33653
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes