Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert to real stream #42

Open
piranna opened this issue Jan 5, 2016 · 4 comments
Open

Convert to real stream #42

piranna opened this issue Jan 5, 2016 · 4 comments

Comments

@piranna
Copy link

piranna commented Jan 5, 2016

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.

@max-mapper
Copy link
Owner

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
})

@piranna
Copy link
Author

piranna commented Jan 7, 2016

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 :-)

@max-mapper
Copy link
Owner

@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-
Copy link

aks- commented Mar 29, 2018

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants