Node: Why does emitting an error in a destination stream unpipe it?

[frontierpsycho]

I was wondering why this happens. It's also been documented by Ben Nadel, but I couldn't find it in the official docs.

Is there a reason why it's _always_ better to unpipe on error? If not, could it be made configurable? Should it at least be documented?

[mscdex]

/cc @nodejs/streams

[calvinmetcalf]

I believe the idea is to prevent data loss. In other words if there is a problem with the destination stream, stop pushing data into it and if the errored stream is the only stream being piped to then eventually the other streams internal buffer will fill up and it will stop reading from wherever it reads from until somebody reads from it meaning that if you see the error deal with it and pipe the thing to a new stream you won't loose any data because you stopped pushing data into the stream that was broken.

[frontierpsycho]

It sounds reasonable that this is default behaviour, but I have a use case where the destination stream throwing an error doesn't mean that subsequent messages will also fail.

It feels as if there are other reasonable use cases out there, mainly having to do with object streams, where each object is a self-contained piece of information that needs to be processed by the reader to produce a self contained result.

In contrast, this is not the case for data that is sequential and only makes sense as a whole, like data being piped into a file, of course. But I think that we should be able to configure our streams to exhibit both of these behaviours.

[calvinmetcalf]

You can configure that by adding an error listener on the dest which
repipes it

On Thu, Sep 24, 2015, 10:52 AM Nameless [email protected] wrote:

It sounds reasonable that this is default behaviour, but I have a use case
where the destination stream throwing an error doesn't mean that subsequent
messages will also fail.

It feels as if there are other reasonable use cases out there, mainly
having to do with object streams, where each object is a self-contained
piece of information that needs to be processed by the reader to produce a
self contained result.

In contrast, this is not the case for data that is sequential and only
makes sense as a whole, like data being piped into a file, of course. But I
think that we should be able to configure our streams to exhibit both of
these behaviours.

—
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/issues/3045#issuecomment-142952421.

[frontierpsycho]

That feels very much like an ugly hack, to be honest.

If I did that, I'd end up with a stream (the result of any streams I want to pipe) which doesn't encompass all of the behaviour I want it to, and it would be the responsibility of any and all readers to implement the missing behaviour on behalf of the stream.

[calvinmetcalf]

what behavior do you want exactly, something like?

class MyStream extends stream.Readable {
  pipeThroughError(dest, opts) {
    var self = this;
    // probably a bad idea in general but would give you all your behavior you wanted
    if (typeof dest.pipe === 'function' && typeof dest.pipeThroughError === 
'undefined') {
        dest.pipeThroughError = this.pipeThroughError;
    }
    return this.pipe(dest, opts).once('error', function (err) {
      if (EE.listenerCount(dest, 'error') === 0)
        dest.emit('error', err);
      self.pipeThroughError(dest, opts);
    })
  }
}

[frontierpsycho]

Yeah, that would be a way to do it, if I was using my own stream. However, I'm using SQS Readable Stream.

Thanks for this, though.

The thing with this issue though, is not so much about my own very specific case. It's more about the fact that I believe it's more natural for the platform to implement this, rather than the users of the platform to jump through hoops to achieve the same effect.

[calvinmetcalf]

So the tricky thing here (which came up before with adding an option to forward errors) is that you don't know if the stream your passing it to supports it, meaning if we added this feature it would still be fairly useless because streams that didn't support it would silently do what you didn't want them to do, maybe writing a version of pump that does what you want might be a better solution.

[frontierpsycho]

Alright, thanks for the comments.

[Fishrock123]

Sounds like this was answered. Let us know if you have more questions! :)