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

The all option is broken when using stdout: 'ignore' or stderr: 'ignore' #702

Closed
ehmicky opened this issue Jan 19, 2024 · 0 comments · Fixed by #703
Closed

The all option is broken when using stdout: 'ignore' or stderr: 'ignore' #702

ehmicky opened this issue Jan 19, 2024 · 0 comments · Fixed by #703

Comments

@ehmicky
Copy link
Collaborator

ehmicky commented Jan 19, 2024

When using the following options:

  • all: true
  • Either stdout: 'ignore' or stderr: 'ignore' (not both)

Then:

  • result.all is correct
  • But both result.stdout and result.stderr are empty

The reason is the following. The all option creates childProcess.all, which is usually a PassThrough that reads from both childProcess.stdout and childProcess.stderr. However, with the above conditions, childProcess.all is a reference for either childProcess.stdout or childProcess.stderr instead. This means result.all and result.stdout|stderr read the exact same stream.

We use get-stream, which does readable.read() calls under the hood. When result.all is reading the stream, those chunks are gone, and result.stdout|stderr cannot read them anymore. In other words, result.all and result.stdout|stderr are not sharing the stream, they are competing for it.

On the other hand, when using a PassThrough, things work for the following reason: it relies on listening for data events, which does allow sharing the stream with multiple consumers (as opposed to readable.read()).

This bug was introduced by #646. Our existing tests were missing this specific case. So we need to revert that PR, which is done by #703.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant