max-mapper/concat-stream

Convert to real stream

piranna opened this issue · 4 comments

The doc says

Since concat-stream is not itself a stream it does not emit errors

Making concat-stream would make it trivial to get the errors down from the pipeline. It could emit a data event when it's ready.

Using the builtin .pipe doesn't propagate error events, so you would still need to either handle .on('error' yourself in the downstream streams or use something like require('pump') that does it for you.

My personal philosophy of streams is that they should be used for streaming efficient pieces of data, so I'm 👎 on a stream that uses the data event to emit a huge concatenated buffer.

Maybe a good module idea would be to combine pump and concat-stream into a module that had an API like this:

var pumpcat = require('pumpcat')
pumpcat(readableStream, throughStream, function done (err, data) {
   // err is from `pump`
   // `data` is from concat-stream
})

Using the builtin .pipe doesn't propagate error events, so you would still need to either handle .on('error' yourself in the downstream streams or use something like require('pump') that does it for you.

Oh, I believed errors where propagated over all the pipeline, so it was only needed to listed to them on the last element. Is not this the case? :-(

My personal philosophy of streams is that they should be used for streaming efficient pieces of data, so I'm 👎 on a stream that uses the data event to emit a huge concatenated buffer.

This point has some perspectives, and they are not incompatible :-) I find myself acceptable to emit a single big data event when dealing with pure-stream app architectures. Another valid style would be to return a Promise instead of using a callback, and I think both three would be allowed to be used at the same time, just apply the callback on the promise and dispatch this last one on the single data event :-)

Maybe a good module idea would be to combine pump and concat-stream into a module that had an API like this:

var pumpcat = require('pumpcat')
pumpcat(readableStream, throughStream, function done (err, data) {
// err is from pump
// data is from concat-stream
})

I'm not too much into thought module (I like to use directly the stream API), but yes, I like the module API you've proposed :-)

@piranna

Oh, I believed errors where propagated over all the pipeline, so it was only needed to listed to them on the last element. Is not this the case? :-(

yes unfortunately you have to handle errors on each stream in the pipeline yourself. however if you use something like pump it implements error propagation for you so you can handle it with a single callback

aks- commented

@maxogden hm, i had to do pump + bl quite a few times, and also bumped into this thread. here it is - https://github.com/aks-/pumpcat