grncdr / merge-stream

Merge multiple streams into one interleaved stream
MIT License
214 stars 16 forks source link

Errors are not propagated #13

Closed UltCombo closed 9 years ago

UltCombo commented 9 years ago

If a source stream emits an error, I'd expect it to be propagated to the merged stream.

Test case:

var mergeStream = require('merge-stream'),
    ReadableStream = require('readable-stream');

var stream = new ReadableStream();

stream._read = function() {
    this.emit('error', new Error('bleh'));
};

mergeStream(stream)
    .on('error', function(err) {
        console.log('ok');
    })
    .resume();

This will throw an "unhandled error event" error as the error is not propagated from the source stream to the merged stream. Would it be possible to propagate these errors to the merged stream, so that I can treat them as in the sample above?

UltCombo commented 9 years ago

Never mind, .pipe()s usually don't forward errors so guess I'll just use a domain.

grncdr commented 9 years ago

Yes, this is the normal behaviour for consuming streams, 'error' events are considered special. That being said, it might be worth having an option or alternate function to also propogate errors, since this isn't necessarily the same as .pipe.

P.S. - instead of using a domain, why not just attach error listeners at the same place you are merging the streams?

UltCombo commented 9 years ago

P.S. - instead of using a domain, why not just attach error listeners at the same place you are merging the streams?

Basically, this is what I want to achieve:

readableStream
    .pipe(throughStream1) // if this stream emits an error, skip throughStream2 and writableStream
    .pipe(throughStream2) // if this stream emits an error, skip writableStream
    .pipe(writableStream)
    .on('error', function catchAll(err) {/*...*/});

However, this goes against how streams were designed, and domains achieve just what I want as the first unhandled error will break the stream and let me treat it.