Any error handlers named "onerror" get detached

[kyriesent]

The loop here https://github.com/floatdrop/gulp-plumber/blob/master/index.js#L9 removes every function named onerror that is attached on('error').

The good news is, this gets rid of the default error handler from readable-stream. The bad news is that it removes anything else that gets attached which has the name onerror.

In particular, I ran into an issue running gulp-plumber with stream-combiner. dominictarr/stream-combiner#17 I think it's not the only place that this might trip someone up.

Is there any other way to find the default handler besides just the name onerror? Maybe grabbing it based a combination of the name and index? Just something in place to make sure that only the default onerror gets detached and nothing else.

[floatdrop]

May be it is possible to compare it to default handler (if they use same function object everywhere).

[dead-claudia]

@floatdrop They don't use a shared reference. Node proper just prepends this on pipe, and readable-stream does similar using this temporary hack to work around prependListener not existing before 6.

You might be able to grab the first listener before calling pipe, and reset to that afterwards. That's when the listener is added. Two events are emitted synchronously at the end, in this specific order:

• pipe on the destination
• resume if the stream is not in flowing mode yet

Within the pipe2 method, you'll have to prepend a listener for pipe or override emit for that call to watch for the first event fired. This below should help demonstrate what I mean. It overrides emit to do minimal work, and to keep easy compatibility with Node pre-6, and it's invisible to newListener and removeListener event listeners.

var first = this.listeners("error")[0]
var listeners = []
var oldEmit = this.emit
var self = this

function getListenersAndReset(inst) {
  // Reset it immediately.
  self.emit = oldEmit

  // Get all listeners
  var list = self.listeners(event)
  var index = list.indexOf(first)

  // Skip the first one when pulling the list.
  if (index >= 0) listeners = list.slice(0, index - 1)
}

this.emit = function (event) {
  // These can be emitted when listeners are added, and thus should be
  // ignored.
  if (event !== "newListener" || event !== "removeListener") {
    getListenersAndReset()
  }
  return oldEmit.apply(this, arguments)
}

this._pipe.apply(this, arguments)
if (this.emit !== oldEmit) getListenersAndReset()

// Remove all newly added listeners. This should remove at least one,
// but it gracefully fails to remove already-removed ones.
for (var i = 0; i < listeners.length; i++) {
  this.removeListener("error", listeners)
}

Of course, this is not very optimized. It does handle some key cases correctly, though:

• Node screws up and doesn't add the listener like it should, in which nothing happens
• Node screws up and either emits events in the wrong order or none at all, in which everything is still handled as expected
• Node adds more listeners, in which they all are removed
• Other handlers are already registered with the same name as Node's, in which they're ignored
• Node changes their own internal name for the handler, in which it's still removed

It also lends itself better for optimization if necessary, and it's flexible enough to fix other events if needed.

Note that I haven't actually tried this method, though. It's just an idea.