Merge branch 'main' into fix/http2_queue

nodejs · Oct 30, 2024 · 27796e7 · 27796e7
2 parents 9eff556 + afeb626
commit 27796e7
Show file tree

Hide file tree

Showing 4 changed files with 638 additions and 39 deletions.
diff --git a/lib/interceptor/dns.js b/lib/interceptor/dns.js
@@ -13,7 +13,6 @@ class DNSInstance {
   affinity = null
   lookup = null
   pick = null
-  lastIpFamily = null
 
   constructor (opts) {
     this.#maxTTL = opts.maxTTL
@@ -61,9 +60,7 @@ class DNSInstance {
         const ip = this.pick(
           origin,
           records,
-          // Only set affinity if dual stack is disabled
-          // otherwise let it go through normal flow
-          !newOpts.dualStack && newOpts.affinity
+          newOpts.affinity
         )
 
         cb(
@@ -78,9 +75,7 @@ class DNSInstance {
       const ip = this.pick(
         origin,
         ips,
-        // Only set affinity if dual stack is disabled
-        // otherwise let it go through normal flow
-        !newOpts.dualStack && newOpts.affinity
+        newOpts.affinity
       )
 
       // If no IPs we lookup - deleting old records
@@ -123,36 +118,36 @@ class DNSInstance {
 
   #defaultPick (origin, hostnameRecords, affinity) {
     let ip = null
-    const { records, offset = 0 } = hostnameRecords
-    let newOffset = 0
+    const { records, offset } = hostnameRecords
+
+    let family
+    if (this.dualStack) {
+      if (affinity == null) {
+        // Balance between ip families
+        if (offset == null || offset === maxInt) {
+          hostnameRecords.offset = 0
+          affinity = 4
+        } else {
+          hostnameRecords.offset++
+          affinity = (hostnameRecords.offset & 1) === 1 ? 6 : 4
+        }
+      }
 
-    if (offset === maxInt) {
-      newOffset = 0
+      if (records[affinity] != null && records[affinity].ips.length > 0) {
+        family = records[affinity]
+      } else {
+        family = records[affinity === 4 ? 6 : 4]
+      }
     } else {
-      newOffset = offset + 1
+      family = records[affinity]
     }
 
-    // We balance between the two IP families
-    // If dual-stack disabled, we automatically pick the affinity
-    const newIpFamily = (newOffset & 1) === 1 ? 4 : 6
-    const family =
-      this.dualStack === false
-        ? records[this.affinity] // If dual-stack is disabled, we pick the default affiniy
-        : records[affinity] ?? records[newIpFamily]
-
-    // If no IPs and we have tried both families or dual stack is disabled, we return null
-    if (
-      (family == null || family.ips.length === 0) &&
-      // eslint-disable-next-line eqeqeq
-      (this.dualStack === false || this.lastIpFamily != newIpFamily)
-    ) {
+    // If no IPs we return null
+    if (family == null || family.ips.length === 0) {
       return ip
     }
 
-    family.offset = family.offset ?? 0
-    hostnameRecords.offset = newOffset
-
-    if (family.offset === maxInt) {
+    if (family.offset == null || family.offset === maxInt) {
       family.offset = 0
     } else {
       family.offset++
@@ -172,7 +167,6 @@ class DNSInstance {
       return this.pick(origin, hostnameRecords, affinity)
     }
 
-    this.lastIpFamily = newIpFamily
     return ip
   }
 
@@ -301,12 +295,20 @@ module.exports = interceptorOpts => {
     throw new InvalidArgumentError('Invalid pick. Must be a function')
   }
 
+  const dualStack = interceptorOpts?.dualStack ?? true
+  let affinity
+  if (dualStack) {
+    affinity = interceptorOpts?.affinity ?? null
+  } else {
+    affinity = interceptorOpts?.affinity ?? 4
+  }
+
   const opts = {
     maxTTL: interceptorOpts?.maxTTL ?? 10e3, // Expressed in ms
     lookup: interceptorOpts?.lookup ?? null,
     pick: interceptorOpts?.pick ?? null,
-    dualStack: interceptorOpts?.dualStack ?? true,
-    affinity: interceptorOpts?.affinity ?? 4,
+    dualStack,
+    affinity,
     maxItems: interceptorOpts?.maxItems ?? Infinity
   }
 

diff --git a/lib/web/fetch/index.js b/lib/web/fetch/index.js
@@ -1943,8 +1943,10 @@ async function httpNetworkFetch (
   // 19. Run these steps in parallel:
 
   //    1. Run these steps, but abort when fetchParams is canceled:
-  fetchParams.controller.onAborted = onAborted
-  fetchParams.controller.on('terminated', onAborted)
+  if (!fetchParams.controller.resume) {
+    fetchParams.controller.on('terminated', onAborted)
+  }
+
   fetchParams.controller.resume = async () => {
     // 1. While true
     while (true) {
@@ -2205,10 +2207,6 @@ async function httpNetworkFetch (
             fetchParams.controller.off('terminated', this.abort)
           }
 
-          if (fetchParams.controller.onAborted) {
-            fetchParams.controller.off('terminated', fetchParams.controller.onAborted)
-          }
-
           fetchParams.controller.ended = true
 
           this.body.push(null)

diff --git a/test/fetch/issue-1711.js b/test/fetch/issue-1711.js
@@ -31,3 +31,30 @@ test('Redirecting a bunch does not cause a MaxListenersExceededWarning', async (
 
   assert.deepStrictEqual(response.url, `${url}/${redirects - 1}`)
 })
+
+test(
+  'aborting a Stream throws',
+  () => {
+    return new Promise((resolve, reject) => {
+      const httpServer = createServer((request, response) => {
+        response.end(new Uint8Array(20000))
+      }).listen(async () => {
+        const serverAddress = httpServer.address()
+
+        if (typeof serverAddress === 'object') {
+          const abortController = new AbortController()
+          const readStream = (await fetch(`http://localhost:${serverAddress?.port}`, { signal: abortController.signal })).arrayBuffer()
+          abortController.abort()
+          setTimeout(reject)
+
+          try {
+            await readStream
+          } catch {
+            httpServer.close()
+            resolve()
+          }
+        }
+      })
+    })
+  }
+)