fix: requeue h2 requests after goaway

lib/dispatcher/client-h2.js

@@ -174,18 +174,22 @@ function canRetryRequestAfterGoAway (request) {
   return body == null || util.isBuffer(body) || util.isBlobLike(body)
 }
 
-function closeRequestStream (request, code = NGHTTP2_REFUSED_STREAM) {
-  const stream = request[kRequestStream]
-
-  clearRequestStream(request)
-
+function closeStream (stream, code = NGHTTP2_REFUSED_STREAM) {
   if (stream != null && !stream.destroyed && !stream.closed) {
     try {
       stream.close(code)
     } catch {}
   }
 }
 
+function detachRequestStreamForClose (request) {
+  const stream = request[kRequestStream]
+
+  clearRequestStream(request)
+
+  return stream
+}
+
 function connectH2 (client, socket) {
   client[kSocket] = socket
 
@@ -447,11 +451,20 @@ function onHttp2SessionError (err) {
   assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID')
 
   this[kSocket][kError] = err
+
+  if (this[kReceivedGoAway]) {
+    return
+  }
+
   this[kClient][kOnError](err)
 }
 
 function onHttp2FrameError (type, code, id) {
   if (id === 0) {
+    if (this[kReceivedGoAway]) {
+      return
+    }
+
     const err = new InformationalError(`HTTP/2: "frameError" received - type ${type}, code ${code}`)
     this[kSocket][kError] = err
     this[kClient][kOnError](err)
@@ -485,12 +498,16 @@ function onHttp2SessionGoAway (errorCode, lastStreamID) {
   const previousPendingIdx = client[kPendingIdx]
   const pendingIdx = getGoAwayPendingIdx(client, lastStreamID)
   const retriableRequests = []
+  const streamsToClose = []
 
+  // Closing one stream after GOAWAY can synchronously emit frameError on
+  // sibling streams. Detach all affected requests first so those errors do
+  // not fail requests that are about to be requeued.
   for (let i = pendingIdx; i < previousPendingIdx; i++) {
     const request = client[kQueue][i]
 
     if (request != null) {
-      closeRequestStream(request)
+      streamsToClose.push(detachRequestStreamForClose(request))
 
       if (canRetryRequestAfterGoAway(request)) {
         retriableRequests.push(request)
@@ -500,6 +517,10 @@ function onHttp2SessionGoAway (errorCode, lastStreamID) {
     }
   }
 
+  for (let i = 0; i < streamsToClose.length; i++) {
+    closeStream(streamsToClose[i])
+  }
+
   if (pendingIdx !== previousPendingIdx) {
     const remainingPendingRequests = client[kQueue].slice(previousPendingIdx)
     client[kQueue].length = pendingIdx
@@ -595,6 +616,10 @@ function onHttp2SocketError (err) {
 
   this[kError] = err
 
+  if (this[kHTTP2Session]?.[kReceivedGoAway]) {
+    return
+  }
+
   this[kClient][kOnError](err)
 }
 

test/http2-goaway.js

@@ -2,12 +2,12 @@
 
 const { tspl } = require('@matteo.collina/tspl')
 const { test, after } = require('node:test')
-const { createSecureServer } = require('node:http2')
+const { constants, createSecureServer } = require('node:http2')
 const { once } = require('node:events')
 
 const pem = require('@metcoder95/https-pem')
 
-const { Client } = require('..')
+const { Client, Pool } = require('..')
 
 test('#3046 - GOAWAY Frame', async t => {
   t = tspl(t, { plan: 8 })
@@ -145,6 +145,65 @@ test('#3753 - Handle GOAWAY Gracefully', async (t) => {
   await t.completed
 })
 
+test('#5468 - requeue multiplexed requests after mid-burst GOAWAY', async (t) => {
+  t = tspl(t, { plan: 4 })
+
+  const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))
+  let firstStream = false
+  let streams = 0
+
+  server.on('stream', (stream) => {
+    stream.on('error', () => {})
+    streams++
+
+    if (!firstStream) {
+      firstStream = true
+      stream.session?.goaway(constants.NGHTTP2_INTERNAL_ERROR, 0)
+      return
+    }
+
+    stream.respond({ ':status': 200 })
+    stream.end('ok')
+  })
+
+  after(() => server.close())
+  await once(server.listen(0), 'listening')
+
+  const pool = new Pool(`https://localhost:${server.address().port}`, {
+    allowH2: true,
+    connections: 10,
+    connect: {
+      rejectUnauthorized: false
+    }
+  })
+  after(async () => pool.close())
+
+  const requests = 40
+  const codes = {}
+  let ok = 0
+
+  await Promise.all(Array.from({ length: requests }, async () => {
+    try {
+      const response = await pool.request({ path: '/', method: 'GET' })
+      await response.body.dump()
+
+      if (response.statusCode === 200) {
+        ok++
+      }
+    } catch (err) {
+      const code = err?.code || err?.cause?.code || err?.name || 'unknown'
+      codes[code] = (codes[code] || 0) + 1
+    }
+  }))
+
+  t.strictEqual(codes.UND_ERR_INFO, undefined)
+  t.ok((codes.ERR_HTTP2_SESSION_ERROR || 0) <= 2)
+  t.ok(ok >= requests - 2)
+  t.ok(streams >= requests - 1)
+
+  await t.completed
+})
+
 test('#5089 - Handle GOAWAY Gracefully', async (t) => {
   t = tspl(t, { plan: 7 })