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

pipeline error logic #23

Closed
mnichols opened this Issue Sep 30, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@mnichols

mnichols commented Sep 30, 2012

The docs clearly say that using 'pipeline' will emit errors from all the streams in the pipeline. In addition, though, the error is emitted for the 'pipeline' being created which means the error is re-emitted by the main pipeline. So this test indicates every error is duplicated.:

    describe('demonstrateerror', function(done) {
      return it('should error once', function() {
        var errors, pipe, thrower, thru;
        errors = 0;
        thrower = es.through(function(data) {
          return this.emit('error', new Error(data));
        });
        thru = es.through(function(data) {
          return this.emit('data', data);
        });
        pipe = es.pipeline(thru, thrower);
        pipe.on('error', function(err) {
          errors++;
          console.log('error count', errors);
          if (errors.length > 1) {
            return done();
          }
        });
        return pipe.write('meh');
      });
    });

My question is why the 'pipeline' isn't considered to be more transparent? Duplicating errors causes issues on handlers since error handling code will inevitably be fired at least twice for each error and is IMO counterintuitive since the error handler now has to be ensure to only fire once (yes, I could use 'once'). Am I misunderstanding the use of this or is this issue valid?

@dominictarr

This comment has been minimized.

Owner

dominictarr commented Sep 30, 2012

Thanks, this is a bug. I've confirmed this, added a test, and fixed it in event-stream@3.0.6

The problem was the duplexer (which pipeline is based on) already reemits the first and last error,
so I was mistakenly adding two listeners to the head and tail of the stream...

@mnichols

This comment has been minimized.

mnichols commented Oct 1, 2012

Thanks Dominic.
You've taught me quite a lot about using Streams. Thank you for your
contributions.

Mike

On Sun, Sep 30, 2012 at 5:42 PM, Dominic Tarr notifications@github.comwrote:

Thanks, this is a bug. I've confirmed this, added a test, and fixed it in
event-stream@3.0.6

The problem was the duplexer (which pipeline is based on) already reemits
the first and last error,
so I was mistakenly adding two listeners to the head and tail of the
stream...


Reply to this email directly or view it on GitHubhttps://github.com//issues/23#issuecomment-9018826.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment