Pass stream as parameter to onFinish handler

[jgrund]

I have a Node stream that can emit an error and can also emit content afterwards.

Example:

import {
  PassThrough
} from 'stream';

const passthrough = new stream.PassThrough();
passthrough.pipe(process.stdout);

passthrough.emit(new Error('boom'));

passthrough.write('foo');  // This gets written to process.stdout, even though an error was emitted.

I'd like to handle the error and continue. I can do so if we update pipeReadable to call onFinish with stream as a third parameter.

This way I can override error handling to write a new StreamError without ending.

[vqvu]

I wish Node would clarify whether or not error is a special event. It would save us a lot of headache in interop. As it is, sometimes it's special and sometimes it's not. src.pipe(dest) will unpipe if dest emits an error but not if src emits an error (as in your example).

I agree that there should be a way to work around the default Highland behavior in this case, especially since we used to support it. However, I want to do it in a more principled way than just passing in stream, since that may encourage people to do whatever they want with it. It also doesn't help people who don't want to depend on internal APIs like StreamError. Let me think about this for a bit.

For curiosity, are the Node streams you're using something from the Node standard library or is it from an external (to Node) library?

[jgrund]

I'm fine with restricting the way to handle as long as there is a way to say: "handle this error and continue".

I am using a PassThrough stream as in the example above. I have an error condition that I emit to the stream and continue afterwards.

I could treat the Error as a normal token; but that would add a third type when I'd like to preserve the semantics.

[vqvu]

I see. Well, I think the least bad best way to do this is to have the onFinish return an object that says "do not end on errors". See #534.

[jgrund]

Looks good to me, thanks. Can we forward port this to the 3.0.0 branch as well?

[vqvu]

Yes. I will do that when I merge the PR.

[vqvu]

3.0.0-beta.3 released.