fix: pipelining logic is not relevant for h2 (#2850)

nodejs · Feb 26, 2024 · 16e49ff · 16e49ff
1 parent 8b131ab
commit 16e49ff
Showing 1 changed file with 40 additions and 41 deletions.
diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js
@@ -457,21 +457,10 @@ function onHTTP2GoAway (code) {
   client[kSocket] = null
   client[kHTTP2Session] = null
 
-  if (client.destroyed) {
-    assert(this[kPending] === 0)
-
-    // Fail entire queue.
-    const requests = client[kQueue].splice(client[kRunningIdx])
-    for (let i = 0; i < requests.length; i++) {
-      const request = requests[i]
-      errorRequest(this, request, err)
-    }
-  } else if (client[kRunning] > 0) {
-    // Fail head of pipeline.
-    const request = client[kQueue][client[kRunningIdx]]
-    client[kQueue][client[kRunningIdx]++] = null
-
-    errorRequest(client, request, err)
+  const requests = client[kQueue].splice(client[kRunningIdx])
+  for (let i = 0; i < requests.length; i++) {
+    const request = requests[i]
+    errorRequest(this, request, err)
   }
 
   client[kPendingIdx] = client[kRunningIdx]
@@ -1089,7 +1078,9 @@ function onSocketClose () {
 
   client[kSocket] = null
 
-  if (client.destroyed) {
+  // TODO (fix): Always fail entire queue
+
+  if (client.destroyed || client[kHTTPConnVersion] === 'h2') {
     assert(client[kPending] === 0)
 
     // Fail entire queue.
@@ -1325,8 +1316,10 @@ function _resume (client, sync) {
       return
     }
 
-    if (client[kRunning] >= (client[kPipelining] || 1)) {
-      return
+    if (client[kHTTPConnVersion] === 'h1') {
+      if (client[kRunning] >= (client[kPipelining] || 1)) {
+        return
+      }
     }
 
     const request = client[kQueue][client[kPendingIdx]]
@@ -1353,35 +1346,41 @@ function _resume (client, sync) {
       return
     }
 
-    if (socket.destroyed || socket[kWriting] || socket[kReset] || socket[kBlocking]) {
+    if (socket.destroyed) {
       return
     }
 
-    if (client[kRunning] > 0 && !request.idempotent) {
-      // Non-idempotent request cannot be retried.
-      // Ensure that no other requests are inflight and
-      // could cause failure.
-      return
-    }
+    if (client[kHTTPConnVersion] === 'h1') {
+      if (socket[kWriting] || socket[kReset] || socket[kBlocking]) {
+        return
+      }
 
-    if (client[kRunning] > 0 && (request.upgrade || request.method === 'CONNECT')) {
-      // Don't dispatch an upgrade until all preceding requests have completed.
-      // A misbehaving server might upgrade the connection before all pipelined
-      // request has completed.
-      return
-    }
+      if (client[kRunning] > 0 && !request.idempotent) {
+        // Non-idempotent request cannot be retried.
+        // Ensure that no other requests are inflight and
+        // could cause failure.
+        return
+      }
 
-    if (client[kRunning] > 0 && util.bodyLength(request.body) !== 0 &&
-      (util.isStream(request.body) || util.isAsyncIterable(request.body) || util.isFormDataLike(request.body))) {
-      // Request with stream or iterator body can error while other requests
-      // are inflight and indirectly error those as well.
-      // Ensure this doesn't happen by waiting for inflight
-      // to complete before dispatching.
+      if (client[kRunning] > 0 && (request.upgrade || request.method === 'CONNECT')) {
+        // Don't dispatch an upgrade until all preceding requests have completed.
+        // A misbehaving server might upgrade the connection before all pipelined
+        // request has completed.
+        return
+      }
 
-      // Request with stream or iterator body cannot be retried.
-      // Ensure that no other requests are inflight and
-      // could cause failure.
-      return
+      if (client[kRunning] > 0 && util.bodyLength(request.body) !== 0 &&
+        (util.isStream(request.body) || util.isAsyncIterable(request.body) || util.isFormDataLike(request.body))) {
+        // Request with stream or iterator body can error while other requests
+        // are inflight and indirectly error those as well.
+        // Ensure this doesn't happen by waiting for inflight
+        // to complete before dispatching.
+
+        // Request with stream or iterator body cannot be retried.
+        // Ensure that no other requests are inflight and
+        // could cause failure.
+        return
+      }
     }
 
     if (!request.aborted && write(client, request)) {