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

How to handle errors without err callback argument #6

Closed
joliss opened this issue May 24, 2013 · 5 comments
Closed

How to handle errors without err callback argument #6

joliss opened this issue May 24, 2013 · 5 comments

Comments

@joliss
Copy link

joliss commented May 24, 2013

The following code:

var concat = require('concat-stream')
var through = require('through')

var stream = through().pause()
stream.pipe(concat(function(data) {}))
stream.emit('error')

throws

stream.js:94
      throw er; // Unhandled stream error in pipe.
            ^
undefined

Before d530532, I believe there would have been an err argument to the callback.

It seems a bit surprising to me to have no err argument to the callback. Is there a nice way to handle errors now? Or should we bring the err argument back maybe?

@max-mapper
Copy link
Owner

Cc @substack (I will respond in more detail later)

Sent from my iPhone

On May 25, 2013, at 12:18 AM, Jo Liss notifications@github.com wrote:

The following code:

var concat = require('concat-stream')
var through = require('through')

var stream = through().pause()
stream.pipe(concat(function(data) {}))
stream.emit('error')
throws

stream.js:94
throw er; // Unhandled stream error in pipe.
^
undefined
Before d530532, I believe there would have been an err argument to the callback.

It seems a bit surprising to me to have no err argument to the callback. Is there a nice way to handle errors now? Or should we bring the err argument back maybe?


Reply to this email directly or view it on GitHub.

@jkroso
Copy link

jkroso commented Jun 23, 2013

i'd also like to hear the story behind it

@max-mapper
Copy link
Owner

sorry about the lag on responding!

so originally I had an err in the callback but @substack convinced me that it was out of scope for this module. For example, add this line to @joliss' example:

stream.on('error', handleError)

all concat-stream does is combine data events, and it itself isn't a stream, so the error event shouldn't propagate. Instead the error handling can just happen in the upstream stream.

gonna close this for now but feel free to re-open if this description isn't logical

@jkroso
Copy link

jkroso commented Jun 23, 2013

I can't say I'm convinced, though handling errors separately isn't hard work I'd be inclined to stick with the older version because its more normal to me

@joliss
Copy link
Author

joliss commented Jun 23, 2013

Hm, I'd be worried that people just forget to attach error handlers. Anyways, I'll defer to your judgment.

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