events: combined event target fixes

benchmark/events/eventtarget.js

@@ -2,7 +2,7 @@
 const common = require('../common.js');
 
 const bench = common.createBenchmark(main, {
-  n: [2e7],
+  n: [1e6],
   listeners: [1, 5, 10]
 }, { flags: ['--expose-internals'] });
 
@@ -13,11 +13,9 @@ function main({ n, listeners }) {
   for (let n = 0; n < listeners; n++)
     target.addEventListener('foo', () => {});
 
-  const event = new Event('foo');
-
   bench.start();
   for (let i = 0; i < n; i++) {
-    target.dispatchEvent(event);
+    target.dispatchEvent(new Event('foo'));
   }
   bench.end(n);
 

lib/events.js

@@ -623,41 +623,20 @@ function unwrapListeners(arr) {
 
 function once(emitter, name) {
   return new Promise((resolve, reject) => {
-    if (typeof emitter.addEventListener === 'function') {
-      // EventTarget does not have `error` event semantics like Node
-      // EventEmitters, we do not listen to `error` events here.
-      emitter.addEventListener(
-        name,
-        (...args) => { resolve(args); },
-        { once: true }
-      );
-      return;
-    }
-
-    const eventListener = (...args) => {
-      if (errorListener !== undefined) {
+    const errorListener = (err) => {
+      emitter.removeListener(name, resolver);
+      reject(err);
+    };
+    const resolver = (...args) => {
+      if (typeof emitter.removeListener === 'function') {
         emitter.removeListener('error', errorListener);
       }
       resolve(args);
     };
-    let errorListener;
-
-    // Adding an error listener is not optional because
-    // if an error is thrown on an event emitter we cannot
-    // guarantee that the actual event we are waiting will
-    // be fired. The result could be a silent way to create
-    // memory or file descriptor leaks, which is something
-    // we should avoid.
+    eventTargetAgnosticAddListener(emitter, name, resolver, { once: true });
     if (name !== 'error') {
-      errorListener = (err) => {
-        emitter.removeListener(name, eventListener);
-        reject(err);
-      };
-
-      emitter.once('error', errorListener);
+      addErrorHandlerIfEventEmitter(emitter, errorListener, { once: true });
     }
-
-    emitter.once(name, eventListener);
   });
 }
 
@@ -668,6 +647,38 @@ function createIterResult(value, done) {
   return { value, done };
 }
 
+function addErrorHandlerIfEventEmitter(emitter, handler, flags) {
+  if (typeof emitter.on === 'function') {
+    eventTargetAgnosticAddListener(emitter, 'error', handler, flags);
+  }
+}
+
+function eventTargetAgnosticRemoveListener(emitter, name, listener, flags) {
+  if (typeof emitter.removeListener === 'function') {
+    emitter.removeListener(name, listener);
+  } else if (typeof emitter.removeEventListener === 'function') {
+    emitter.removeEventListener(name, listener, flags);
+  } else {
+    throw new ERR_INVALID_ARG_TYPE('emitter', 'EventEmitter', emitter);
+  }
+}
+
+function eventTargetAgnosticAddListener(emitter, name, listener, flags) {
+  if (typeof emitter.on === 'function') {
+    if (flags && flags.once) {
+      emitter.once(name, listener);
+    } else {
+      emitter.on(name, listener);
+    }
+  } else if (typeof emitter.addEventListener === 'function') {
+    // EventTarget does not have `error` event semantics like Node
+    // EventEmitters, we do not listen to `error` events here.
+    emitter.addEventListener(name, (arg) => { listener(arg); }, flags);
+  } else {
+    throw new ERR_INVALID_ARG_TYPE('emitter', 'EventEmitter', emitter);
+  }
+}
+
 function on(emitter, event) {
   const unconsumedEvents = [];
   const unconsumedPromises = [];
@@ -704,8 +715,8 @@ function on(emitter, event) {
     },
 
     return() {
-      emitter.removeListener(event, eventHandler);
-      emitter.removeListener('error', errorHandler);
+      eventTargetAgnosticRemoveListener(emitter, event, eventHandler);
+      eventTargetAgnosticRemoveListener(emitter, 'error', errorHandler);
       finished = true;
 
       for (const promise of unconsumedPromises) {
@@ -721,17 +732,20 @@ function on(emitter, event) {
                                        'Error', err);
       }
       error = err;
-      emitter.removeListener(event, eventHandler);
-      emitter.removeListener('error', errorHandler);
+      eventTargetAgnosticRemoveListener(emitter, event, eventHandler);
+      eventTargetAgnosticRemoveListener(emitter, 'error', errorHandler);
     },
 
     [SymbolAsyncIterator]() {
       return this;
     }
   }, AsyncIteratorPrototype);
 
-  emitter.on(event, eventHandler);
-  emitter.on('error', errorHandler);
+  eventTargetAgnosticAddListener(emitter, event, eventHandler);
+  if (event !== 'error') {
+    addErrorHandlerIfEventEmitter(emitter, errorHandler);
+  }
+
 
   return iterator;