Any error handlers named "onerror" get detached #41

Activity

1. kyriesent

   changed the title [-]any error handlers named "onerror" get detached[/-] [+]Any error handlers named "onerror" get detached[/+] on Oct 31, 2015
2. floatdrop

   added

   bug

   on Oct 31, 2015
3. floatdrop commented on Oct 31, 2015

   

   floatdrop

   on Oct 31, 2015

   Owner

   More actions

   May be it is possible to compare it to default handler (if they use same function object everywhere).
4. dead-claudia commented on May 22, 2016

   

   dead-claudia

   on May 22, 2016

   More actions

   @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.