Uh oh!
There was an error while loading. Please reload this page.
[v13.x backport] stream: don't destroy final readable stream in pipeline - #32111
[v13.x backport] stream: don't destroy final readable stream in pipeline#32111ronag wants to merge 1 commit into
Conversation
Note that the base PR (#32110) has NOT landed yet. I'm just preparing this in order to fast track. I'm not sure what the procedure here is and whether this can land first. |
2ece1ad to
0acdc6bCompare
This comment has been minimized.
This comment has been minimized.
0acdc6b to
276bb79Compare
This comment has been minimized.
This comment has been minimized.
276bb79 to
db54b58Comparedb54b58 to
e91dc3bCompareIf the last stream in a pipeline is still usable/readable don't destroy it to allow further composition. Fixes: nodejs#32105 Backport-PR-URL: nodejs#32111
e91dc3b to
07228c6Compare
This comment has been minimized.
This comment has been minimized.
ronag
commented
Mar 5, 2020
nodejs-github-bot
commented
Mar 6, 2020
szmarczak
commented
Mar 6, 2020
There may be an edge case where the source is an IncomingMessage, and in that case the connection will be destroyed after the response has gracefully ended. This will cause to create new connection every time you make a new request. But that should be fixed in https://github.com/nodejs/node/blob/master/lib/_http_incoming.js#L121-L124 As it should destroy the socket only if the This PR is good to merge IMO. |
ronag
commented
Mar 6, 2020
ronag
commented
Mar 8, 2020
@nodejs/streams PTAL, this PR (and #32110) needs another approval and should preferably land as soon as possible as it is affecting the ecosystem. |
If the last stream in a pipeline is still usable/readable
don't destroy it to allow further composition.
Fixes: #32105
Refs: #32110
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes