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

Proposal for .pipe() - forward error emitted from src to dest in stream #6113

Closed
benpptung opened this issue Apr 8, 2016 · 2 comments
Closed
Labels
feature request Issues that request new features to be added to Node.js. stream Issues and PRs related to the stream subsystem.

Comments

@benpptung
Copy link

  • v4.4.1:
  • Darwin Bens-Mac-mini.local 13.4.0 Darwin Kernel Version 13.4.0: Wed Mar 18 16:20:14 PDT 2015; root:xnu-2422.115.14~1/RELEASE_X86_64 x86_64:
  • Subsystem:

While dealing with a weird uncaughtException, emitted by a stream. It inspires me this idea, maybe .pipe() can do one more thing to bind an error listener on the src and emit ( forward ) the error to dest automatically.

The benefit is: We can easily handle the error on the destination, and no worry that error might be emitted on somewhere we forgot or have no access to it.

In my case, for testing purpose, I emit error on the query object created by node-mysql, and keep to get uncaughtException, I checked all the EventEmitters on the scope, and don't know why it happened. Till the end, I understand, node-mysql forward the error from the query object to the stream, and my database api(my another module not written in the current module) pipe that stream to a Transform stream and return that transform stream to me. I forgot the stream is not the stream originally created by node-mysql, and to avoid potential uncaughtException, I decide to add error listener to the original stream.

It makes the codes ugly, previously. it is

return query.stream().pipe(NormalizeData());

and now, it becomes

var s = query.stream();
var n = NormalizeData():
s.on('error', err=> n.emit('error', err));
return s.pipe(n);

It's just an idea, please feel free to close it, if it is no sense :)

@r-52 r-52 added stream Issues and PRs related to the stream subsystem. feature request Issues that request new features to be added to Node.js. labels Apr 8, 2016
@Fishrock123
Copy link
Contributor

Mostly a duplicate of #1521 :)

@benpptung
Copy link
Author

@Fishrock123 thank you for the sharing.
I've read that thread, and it inspired me to pull the error from the upstream using the pipe and unpipe events, so I can get rid of the ugly codes mentioned previously. Maybe, nodejs can add some stream error handling practices on website?

Hope nodejs will add the forward error feature, it'd be better if it's a default behavior, since streaming might not be about data only, IMO, it might also about some upstream key events, e.g. error, warn, info. If all the stream can forward error, it would be easier to avoid uncaughtException. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants