Skip to content
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

Promise version of stream.finished does not give access to cleanup function #42938

Open
kwasimensah opened this issue May 2, 2022 · 1 comment
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@kwasimensah
Copy link

kwasimensah commented May 2, 2022

Version

16.13.0

Platform

Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_X86_64 x86_64

Subsystem

streams

What steps will reproduce the bug?

const readStream = fs.createReadStream(<a folder>)
const writeStream = fs.createWriteStream("foo.txt")

readStream.on("error", (error) => {
   throw new errlop('Caught the error', error)'
})

readStream.pipe(writeStream);
await stream.finished(writeStream);

How often does it reproduce? Is there a required condition?

Always reproduces

What is the expected behavior?

await stream.finished(writeStream) returns the cleanup function specified at https://nodejs.org/docs/latest-v16.x/api/stream.html#streamfinishedstream-options-callback so we can clean up the error call.

Or that error rejects the promise we're awaiting on and gets treated as part of the normal Promise error handling flow.

What do you see instead?

"Uncaught Error: Caught the error"

Additional information

https://github.com/nodejs/node/blob/master/lib/internal/streams/end-of-stream.js#L242 drops the cleanup function that eos returns.

I think I can work around this by manually making my own promises that resolve/reject based on callbacks firing

@kwasimensah
Copy link
Author

Actually, this fixes the error (stream.finished isn't actually important here). it seems it's standard for the error to not get passed through the pipe

readStream.on("error", (error) => {
   const newError = new errlop('Caught the error', error);
   writeStream.destroy(newError);
})

@VoltrexKeyva VoltrexKeyva added the stream Issues and PRs related to the stream subsystem. label May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants