search

max-mapper / concat-stream

writable stream that concatenates strings or data and calls a callback with the result
MIT License
573 stars 64 forks source link

Use standard, error-first callbacks #37

Open zenflow opened 9 years ago

zenflow commented 9 years ago

Drawn from #15. Let's discuss in a separate issue.

jonathanong on Dec 1, 2013

callback(err, data) - it's a callback, not an event listener, so imo it should have err as the first argument. however, err should always be null since concat-stream should never have any errors (i get #6 (comment)) unless we decide to throw errors when there are crazy typing issues. we can do crazy stuff like check listener.length but i'm not a fan of that either.

substack on Dec 23, 2013

cb(err, data) is annoying if there isn't ever an error. Why not just omit that parameter like it presently is?

jeromew on Jan 13, 2014

[...] don't understand "concat-stream should never have any errors" because shouln't concat-stream handle the errors of the underlying streams to report via cb(err, data) ?

zenflow on June 29, 2015 (just now)

If you pipe a readable stream to this or any other writable stream (with Readable.protototype.pipe), errors will not be piped downstream along with the data. When using this library, you must handle upstream errors yourself Besides any upstream errors, there are no other errors to expect.

kumavis commented 9 years ago

I agree

zenflow commented 9 years ago

@kumavis what in particular do you agree with?

kumavis commented 9 years ago

good question. I was anticipating that upstream errors would flow through, and therefore there should be an error handler.

Can this stream close/end/finish? if it can, and then more data is pushed into it, shouldn't it error? The notion that this never errors seems misguided.

but im blowing smoke b/c i dont particularly remember ever actually used this module

piranna commented 8 years ago

I think pipeline errors should be dispatched, too.