Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
stream: avoid destroying http1 objects#32197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ronag commented Mar 11, 2020
I'm sorry about 8a2b62e causing these troubles. I've tried to resolve these follow up issues as quickly as possible. |
nodejs-github-bot commented Mar 11, 2020
mcollina commented Mar 11, 2020
@ronag are we sure we should not revert all of those fixes instead and redo a PR with all of them? |
mcollina left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ronag commented Mar 11, 2020
I would prefer not. Seems more difficult than fixing it. I see your point though. If you would prefer revert I would be ok with that. |
mcollina commented Mar 11, 2020
Let's see if something else pop up. |
5aeab8d to abd091cComparenodejs-github-bot commented Mar 11, 2020
Uh oh!
There was an error while loading. Please reload this page.
MylesBorins commented Mar 11, 2020
can we fast track this? I'd like to see it land in the upcoming 13.11.0 release @ronag looks like this needs a rebase |
http1 objects are coupled with their corresponding res/req and cannot be treated independently as normal streams. Add a special exception for this in the pipeline cleanup. Fixes: nodejs#32184
d83675f to 1415464Compareronag commented Mar 11, 2020 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
rebased @MylesBorins |
nodejs-github-bot commented Mar 11, 2020 • edited by MylesBorins
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by MylesBorins
Uh oh!
There was an error while loading. Please reload this page.
mcollina commented Mar 11, 2020
+1 to fast track |
MylesBorins commented Mar 11, 2020
@ronag FWIW this patch is not going to land cleanly on v13.x. You might want to get started on a backport now if you have time. very much trying to get this out in today's release. |
MylesBorins commented Mar 11, 2020
landed in f8f2089 |
http1 objects are coupled with their corresponding res/req and cannot be treated independently as normal streams. Add a special exception for this in the pipeline cleanup. Fixes: #32184 PR-URL: #32197 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
http1 objects are coupled with their corresponding res/req and cannot be treated independently as normal streams. Add a special exception for this in the pipeline cleanup. Fixes: #32184 Backport-PR-URL: #32212 PR-URL: #32197 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
http1 objects are coupled with their corresponding
res/req and cannot be treated independently as
normal streams. Add a special exception for this
in the pipeline cleanup.
Fixes: #32184
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes