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

stream pipeline kills process when writeStream is closed #52622

Open
DevT0ny opened this issue Apr 21, 2024 · 5 comments · May be fixed by #53241
Open

stream pipeline kills process when writeStream is closed #52622

DevT0ny opened this issue Apr 21, 2024 · 5 comments · May be fixed by #53241
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@DevT0ny
Copy link

DevT0ny commented Apr 21, 2024

Version

v21.7.1

Platform

Linux

Subsystem

No response

What steps will reproduce the bug?

To reproduce this follow this

echo "Hello from src file" > /tmp/src.txt

Now run this script

//@ts-check

const fs = require('node:fs')
const { pipeline } = require('node:stream/promises')
const { promisify } = require('node:util')

const main = async () => {
  const dst = '/tmp/dest.txt'
  const writeStream = fs.createWriteStream(dst, { flags: 'a' })

  const close = promisify(writeStream.close)
  await close.call(writeStream)

  console.log(writeStream.closed, writeStream.writableEnded)

  await pipeline(fs.createReadStream('/tmp/src.txt'), writeStream)
  const dstfile = await fs.promises.readFile(dst, { encoding: 'utf8' })
  console.log({ dstfile })
}

main()
  .then(() => {
    console.log('done')
  })
  .catch((err) => {
    console.log('error')
    console.error(err)
  })
  .finally(() => {
    console.log('over')
  })

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

No response

What is the expected behavior? Why is that the expected behavior?

pipeline function should throw error saying writeStream is closed.

What do you see instead?

prints true true then exits with code 0. true true is part of script and its working as intended.

Additional information

No response

@VoltrexKeyva VoltrexKeyva added the stream Issues and PRs related to the stream subsystem. label Apr 21, 2024
@benjamingr
Copy link
Member

@ronag wdyt?

@benjamingr
Copy link
Member

(also weren't we deprecating close?)

@ronag
Copy link
Member

ronag commented Apr 25, 2024

Yea. I think we should throw if pipelining to a destroyed or ended stream. However, that will be semver major.

@benjamingr
Copy link
Member

Yes definitely semver-major and I agree it's better behavior conceptually

@jakecastelli
Copy link
Contributor

Hi guys, I'd like to take some time to work on this issue, @ronag may I ask what should be the error code for this one if we decide to throw an error?

I think we have ERR_STREAM_CANNOT_PIPE that sounds really close

E('ERR_STREAM_CANNOT_PIPE', 'Cannot pipe, not readable', Error);

But I think this one probably should be something like Cannot pipe, the stream has already ended / destroyed / closed.

Being said should I add new error code(s) or there is an existing one that fits the purpose.

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

Successfully merging a pull request may close this issue.

5 participants