Add buffered data to error.stdout, error.stderr and error.all

[ehmicky]

Fixes #228. The issue was that errors on stdin/stdout/stderr were not reported the same way as other errors.

It also adds:

• error.stdout or error.stderr set with the buffered data when that stream failed
• error.stream set with the name of the failing stream
• better error.message on stream errors

Issues

If one stream errors, we do not want to wait for other streams to complete, for the same reason as #270, i.e. those streams might take a long time to complete (or never complete). However this creates the following issues:

1. we can only get the buffered data of the failing stream, not the other streams. get-stream does not seem to allow to retrieve the currently buffered data.
2. several streams might fail at the same time when they rely on the same underlying file/TTY. In that case, there's a race condition where error.stream will be set tot the first stream to report an error.
3. all will always fail right before stdout or stderr, taking precedence over them.

Possible solutions for each of those issues:

1. Make the other streams fail by emitting an error event on them. Not implemented yet.
2. Wait for one macrotask (setImmediate()) and check if other streams have failed. Not implemented yet.
3. Ignore errors on all. This PR currently implements this.

[ehmicky]

CI tests fixed. Before moving forward with README and TypeScript, I would need feedback about those 3 problems and possible solutions.

[sindresorhus]

error.stream set with the name of the failing stream

Is this really useful? What would someone use it for?

get-stream does not seem to allow to retrieve the currently buffered data.

We could potentially add this to get-stream if it simplifies the implementation here?

[ehmicky]

error.stream is not super useful, I will remove it from this PR. It's also used in error.message which shows which stream failed, but we could just not indicate which stream failed if it simplifies this PR.

I think once one stream fails, we don't want the other streams to keep going, i.e. we can close or destroy them right away. If we do this by emitting an error event on them, then await them, get-stream will reject their promises and set their bufferedData. That would require no need to change get-stream. What do you think?

[sindresorhus]

error.stream is not super useful, I will remove it from this PR. It's also used in error.message which shows which stream failed, but we could just not indicate which stream failed if it simplifies this PR.

👍

What do you think?

👍

[ehmicky]

Fixed. Summary of changes:

• on either process error or any stream error, make error.stdout, error.stderr and error.all contain the buffered stream data.
• remove error.stream and error.bufferedData (undocumented error properties)

It turns out calling stream.emit('error') (as suggested in the previous comment) is not needed. stream.destroy() is enough to make the buffered data available. This meant the following changes:

• stream.destroy() is called a little earlier, which is not an issue since it's not used after that point
• stream.destroy() does not need to be called when all streams successfully completed.

This PR enables some little refactoring, but I will do it in a follow-up PR to keep the diff simple.

No TypeScript types have been changed, but I've added documentation for the new buffered data behavior.

[ehmicky]

Fixed.