feat: make use of addAbortListener where applicable (nodejs#2195)

* feat: make use of `addAbortListener` where applicable * lint * fix UT
crysmags · Feb 27, 2024 · 8917287 · 8917287
1 parent 4b31a3b
commit 8917287
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 18 deletions.
diff --git a/lib/api/abort-signal.js b/lib/api/abort-signal.js
@@ -1,3 +1,4 @@
+const { addAbortListener } = require('../core/util')
 const { RequestAbortedError } = require('../core/errors')
 
 const kListener = Symbol('kListener')
@@ -29,11 +30,7 @@ function addSignal (self, signal) {
     abort(self)
   }
 
-  if ('addEventListener' in self[kSignal]) {
-    self[kSignal].addEventListener('abort', self[kListener])
-  } else {
-    self[kSignal].addListener('abort', self[kListener])
-  }
+  addAbortListener(self[kSignal], self[kListener])
 }
 
 function removeSignal (self) {

diff --git a/lib/api/readable.js b/lib/api/readable.js
@@ -155,12 +155,13 @@ module.exports = class BodyReadable extends Readable {
     const abortFn = () => {
       this.destroy()
     }
+    let signalListenerCleanup
     if (signal) {
       if (typeof signal !== 'object' || !('aborted' in signal)) {
         throw new InvalidArgumentError('signal must be an AbortSignal')
       }
       util.throwIfAborted(signal)
-      signal.addEventListener('abort', abortFn, { once: true })
+      signalListenerCleanup = util.addAbortListener(signal, abortFn)
     }
     try {
       for await (const chunk of this) {
@@ -173,8 +174,10 @@ module.exports = class BodyReadable extends Readable {
     } catch {
       util.throwIfAborted(signal)
     } finally {
-      if (signal) {
-        signal.removeEventListener('abort', abortFn)
+      if (typeof signalListenerCleanup === 'function') {
+        signalListenerCleanup()
+      } else if (signalListenerCleanup) {
+        signalListenerCleanup[Symbol.dispose]()
       }
     }
   }

diff --git a/lib/core/util.js b/lib/core/util.js
@@ -422,6 +422,24 @@ function throwIfAborted (signal) {
   }
 }
 
+let events
+function addAbortListener (signal, listener) {
+  if (typeof Symbol.dispose === 'symbol') {
+    if (!events) {
+      events = require('events')
+    }
+    if (typeof events.addAbortListener === 'function' && 'aborted' in signal) {
+      return events.addAbortListener(signal, listener)
+    }
+  }
+  if ('addEventListener' in signal) {
+    signal.addEventListener('abort', listener, { once: true })
+    return () => signal.removeEventListener('abort', listener)
+  }
+  signal.addListener('abort', listener)
+  return () => signal.removeListener('abort', listener)
+}
+
 const hasToWellFormed = !!String.prototype.toWellFormed
 
 /**
@@ -469,6 +487,7 @@ module.exports = {
   isFormDataLike,
   buildURL,
   throwIfAborted,
+  addAbortListener,
   nodeMajor,
   nodeMinor,
   nodeHasAutoSelectFamily: nodeMajor > 18 || (nodeMajor === 18 && nodeMinor >= 13)

diff --git a/lib/fetch/index.js b/lib/fetch/index.js
@@ -56,7 +56,7 @@ const {
 const { kHeadersList } = require('../core/symbols')
 const EE = require('events')
 const { Readable, pipeline } = require('stream')
-const { isErrored, isReadable, nodeMajor, nodeMinor } = require('../core/util')
+const { addAbortListener, isErrored, isReadable, nodeMajor, nodeMinor } = require('../core/util')
 const { dataURLProcessor, serializeAMimeType } = require('./dataURL')
 const { TransformStream } = require('stream/web')
 const { getGlobalDispatcher } = require('../global')
@@ -174,8 +174,8 @@ async function fetch (input, init = {}) {
   let controller = null
 
   // 11. Add the following abort steps to requestObject’s signal:
-  requestObject.signal.addEventListener(
-    'abort',
+  addAbortListener(
+    requestObject.signal,
     () => {
       // 1. Set locallyAborted to true.
       locallyAborted = true
@@ -189,8 +189,7 @@ async function fetch (input, init = {}) {
       // 4. Abort the fetch() call with p, request, responseObject,
       //    and requestObject’s signal’s abort reason.
       abortFetch(p, request, responseObject, requestObject.signal.reason)
-    },
-    { once: true }
+    }
   )
 
   // 12. Let handleFetchDone given response response be to finalize and

diff --git a/lib/fetch/request.js b/lib/fetch/request.js
@@ -340,6 +340,8 @@ class Request {
 
     // 28. Set this’s signal to a new AbortSignal object with this’s relevant
     // Realm.
+    // TODO: could this be simplified with AbortSignal.any
+    // (https://dom.spec.whatwg.org/#dom-abortsignal-any)
     const ac = new AbortController()
     this[kSignal] = ac.signal
     this[kSignal][kRealm] = this[kRealm]
@@ -385,7 +387,7 @@ class Request {
           }
         } catch {}
 
-        signal.addEventListener('abort', abort, { once: true })
+        util.addAbortListener(signal, abort)
         requestFinalizer.register(ac, { signal, abort })
       }
     }
@@ -733,12 +735,11 @@ class Request {
     if (this.signal.aborted) {
       ac.abort(this.signal.reason)
     } else {
-      this.signal.addEventListener(
-        'abort',
+      util.addAbortListener(
+        this.signal,
         () => {
           ac.abort(this.signal.reason)
-        },
-        { once: true }
+        }
       )
     }
     clonedRequestObject[kSignal] = ac.signal