teambition / merge2

Merge multiple streams into one stream in sequence or parallel (~119M/month downloads).
MIT License
170 stars 14 forks source link

emit errors from streams #29

Closed f1nzer closed 4 years ago

f1nzer commented 4 years ago

Errors should be emitted to the merged stream, otherwise, all errors will be encountered as an unhandled error.

Using this approach it is possible to handle errors using stream.pipeline(merge2(stream1, stream2), outStream) for example.

zensh commented 4 years ago

Hi, @f1nzer : Readable.prototype.pipe method does not handle error event in Node.js. It says: "One important caveat is that if the Readable stream emits an error during processing, the Writable destination is not closed automatically. If an error occurs, it will be necessary to manually close each stream in order to prevent memory leaks". I think we should follow the Node.js' definition. But we can add a option pipeError for this feature.

f1nzer commented 4 years ago

Yeah, I think there are 2 cases how the merged stream should behave: 1) as it behaves now: it is just a pipe that doesn't care about source streams lifecycle 2) merged stream should behave like a new stream, that hides all these source streams: emits errors and maybe should allow us to destroy source streams when the merged one is destroyed.

Due to stream "flow" mode (because of pause/resume calls on source streams - maybe it's also should be optional) it's not trivial how to emit an error to the merged stream since error might occur even on its creation - before we got the merged one.

Anyway, pipeError option looks like a solution for this case, but maybe it might be later extended to the mode option (like what I explained above).