-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Stream.pipeline()
does not call its callback if the readable stream has ended since 19.5.0
#46595
Labels
stream
Issues and PRs related to the stream subsystem.
Comments
ehmicky
changed the title
Feb 10, 2023
Stream.pipeline()
does not call its callback if the readable stream has endedStream.pipeline()
does not call its callback if the readable stream has ended since 19.5.0
This was referenced Feb 10, 2023
Trying to look into it! |
debadree25
added a commit
to debadree25/node
that referenced
this issue
Feb 10, 2023
nodejs-github-bot
pushed a commit
that referenced
this issue
Feb 19, 2023
Fixes: #46595 PR-URL: #46600 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Thanks @debadree25! ❤️ |
debadree25
added a commit
to debadree25/node
that referenced
this issue
Feb 27, 2023
Fixes: nodejs#46595 PR-URL: nodejs#46600 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
debadree25
added a commit
to debadree25/node
that referenced
this issue
Feb 27, 2023
Fixes: nodejs#46595 PR-URL: nodejs#46600 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
debadree25
added a commit
to debadree25/node
that referenced
this issue
Feb 27, 2023
Fixes: nodejs#46595 PR-URL: nodejs#46600 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Mar 8, 2023
targos
pushed a commit
that referenced
this issue
Mar 13, 2023
Fixes: #46595 PR-URL: #46600 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This is now fixed in |
danielleadams
pushed a commit
that referenced
this issue
Apr 11, 2023
Fixes: #46595 PR-URL: #46600 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Version
19.5.0 and 18.14.0
Platform
Linux ehmicky-laptop 5.19.0-31-generic #32-Ubuntu SMP PREEMPT_DYNAMIC Fri Jan 20 15:20:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
stream
What steps will reproduce the bug?
With Node >=18.14.0 and >=19.5.0,
done
is not printed.With Node <18.4.0 and <19.5.0,
done
is printed.How often does it reproduce? Is there a required condition?
Only the Node.js minor version.
What is the expected behavior?
stream.pipeline()
should fire its callback if the readable stream has already ended.What do you see instead?
Whether the callback is fired or not depends on the Node.js minor version.
Additional information
It seems like the following PR might be responsible for this new behavior: #46226
Side note: this currently breaks:
get-stream
, which now hangs on streams that have already ended:get-stream
does not complete when the stream has already ended sindresorhus/get-stream#50get-stream
to wait for a child process' stdin/stdout/stderr to complete before completing itself: Promise never resolves if process output pipes are read first in node 18.14.0 sindresorhus/execa#516The text was updated successfully, but these errors were encountered: