-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
doc: add steam pipelining note on Http usage #41796
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
doc: add steam pipelining note on Http usage #41796
Conversation
fb8c5a9 to
7f235dc
Compare
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
mmarchini
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.
nit: replace steam with stream in commit message
7f235dc to
fd7932d
Compare
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
| failure, this can cause event listener leaks and swallowed errors. | ||
|
|
||
| `stream.pipeline()` closes all the streams when an error is raised. | ||
| The `IncomingRequest` usage with `pipeline` could lead to an unexpected behavior |
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.
What is IncomingRequest? Did you mean IncomingMessage? In the example below res is a ServerResponse or OutgoingMessage.
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.
Actually, I meant an Incoming Request in general. The idea is to show that a socket/request usage with pipeline could lead to a error
|
Landed in 1c7a74e |
PR-URL: #41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #41796 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
Address: #26311
Reference: #41137
cc/@ronag @mcollina