From 588ab306da437e2c4a77c0d12f583ab896b20d4e Mon Sep 17 00:00:00 2001
From: Daeyeon Jeong <daeyeon.dev@gmail.com>
Date: Wed, 15 Jun 2022 04:30:51 +0900
Subject: [PATCH] events: fix adding abort listener in `events.once`

Event listeners passed to un/subscribe the abort event are mismatched.
This removes the wrapper function in `eventTargetAgnosticAddListener()`
and directly passes the given listener to the `EventTarget`.

IMO, removing the wrapper seems harmless, and the `AbortSignal` is
seemingly the only `EventTarget` passed to this function for now.

Fixes: https://github.com/nodejs/node/issues/43337
Refs: https://github.com/nodejs/node/pull/33659
Refs: https://github.com/nodejs/node/pull/34997

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: https://github.com/nodejs/node/pull/43373
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 lib/events.js                     |  6 +++---
 test/parallel/test-events-once.js | 35 +++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/lib/events.js b/lib/events.js
index 1e568e1cf40..50708f70058 100644
--- a/lib/events.js
+++ b/lib/events.js
@@ -965,6 +965,8 @@ async function once(emitter, name, options = kEmptyObject) {
     };
     eventTargetAgnosticAddListener(emitter, name, resolver, { once: true });
     if (name !== 'error' && typeof emitter.once === 'function') {
+      // EventTarget does not have `error` event semantics like Node
+      // EventEmitters, we listen to `error` events only on EventEmitters.
       emitter.once('error', errorListener);
     }
     function abortListener() {
@@ -1004,9 +1006,7 @@ function eventTargetAgnosticAddListener(emitter, name, listener, flags) {
       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);
+    emitter.addEventListener(name, listener, flags);
   } else {
     throw new ERR_INVALID_ARG_TYPE('emitter', 'EventEmitter', emitter);
   }
diff --git a/test/parallel/test-events-once.js b/test/parallel/test-events-once.js
index 56042b1ecee..6acb7956535 100644
--- a/test/parallel/test-events-once.js
+++ b/test/parallel/test-events-once.js
@@ -1,5 +1,5 @@
 'use strict';
-// Flags: --no-warnings
+// Flags: --expose-internals --no-warnings
 
 const common = require('../common');
 const { once, EventEmitter } = require('events');
@@ -9,6 +9,7 @@ const {
   fail,
   rejects,
 } = require('assert');
+const { kEvents } = require('internal/event_target');
 
 async function onceAnEvent() {
   const ee = new EventEmitter();
@@ -65,6 +66,32 @@ async function catchesErrors() {
   strictEqual(ee.listenerCount('myevent'), 0);
 }
 
+async function catchesErrorsWithAbortSignal() {
+  const ee = new EventEmitter();
+  const ac = new AbortController();
+  const signal = ac.signal;
+
+  const expected = new Error('boom');
+  let err;
+  process.nextTick(() => {
+    ee.emit('error', expected);
+  });
+
+  try {
+    const promise = once(ee, 'myevent', { signal });
+    strictEqual(ee.listenerCount('error'), 1);
+    strictEqual(signal[kEvents].size, 1);
+
+    await promise;
+  } catch (e) {
+    err = e;
+  }
+  strictEqual(err, expected);
+  strictEqual(ee.listenerCount('error'), 0);
+  strictEqual(ee.listenerCount('myevent'), 0);
+  strictEqual(signal[kEvents].size, 0);
+}
+
 async function stopListeningAfterCatchingError() {
   const ee = new EventEmitter();
 
@@ -165,7 +192,10 @@ async function abortSignalAfterEvent() {
     ee.emit('foo');
     ac.abort();
   });
-  await once(ee, 'foo', { signal: ac.signal });
+  const promise = once(ee, 'foo', { signal: ac.signal });
+  strictEqual(ac.signal[kEvents].size, 1);
+  await promise;
+  strictEqual(ac.signal[kEvents].size, 0);
 }
 
 async function abortSignalRemoveListener() {
@@ -221,6 +251,7 @@ Promise.all([
   onceAnEventWithNullOptions(),
   onceAnEventWithTwoArgs(),
   catchesErrors(),
+  catchesErrorsWithAbortSignal(),
   stopListeningAfterCatchingError(),
   onceError(),
   onceWithEventTarget(),