Fix Node.js v16 error responses on HTTP1.1 (#1206)

Signed-off-by: Timo Stamm <[email protected]>
connectrpc · Sep 3, 2024 · d42cff5 · d42cff5
1 parent e442279
commit d42cff5
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 4 deletions.
diff --git a/.github/workflows/conformance-express.yaml b/.github/workflows/conformance-express.yaml
@@ -21,7 +21,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        node-version: [21.1.0, 20.9.0, 18.16.0]
+        node-version: [21.1.0, 20.9.0, 18.16.0, 16.20.0]
     name: "Node.js ${{ matrix.node-version }}"
     timeout-minutes: 10
     steps:

diff --git a/.github/workflows/conformance-fastify.yaml b/.github/workflows/conformance-fastify.yaml
@@ -21,7 +21,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        node-version: [21.1.0, 20.9.0, 18.16.0]
+        node-version: [21.1.0, 20.9.0, 18.16.0, 16.20.0]
     name: "Node.js ${{ matrix.node-version }}"
     timeout-minutes: 10
     steps:

diff --git a/.github/workflows/conformance-node.yaml b/.github/workflows/conformance-node.yaml
@@ -21,7 +21,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        node-version: [21.1.0, 20.9.0, 18.16.0]
+        node-version: [21.1.0, 20.9.0, 18.16.0, 16.20.0]
         side: [client, server]
     name: "Node.js ${{ matrix.node-version }} ${{ matrix.side }}"
     timeout-minutes: 10

diff --git a/packages/connect-node/src/node-universal-handler.ts b/packages/connect-node/src/node-universal-handler.ts
@@ -207,7 +207,16 @@ export async function universalResponseToNodeResponse(
 async function* asyncIterableFromNodeServerRequest(
   request: NodeServerRequest,
 ): AsyncIterable<Uint8Array> {
-  for await (const chunk of request) {
+  const it = request.iterator({
+    // Node.js v16 closes request and response when this option isn't disabled.
+    // When one of our handlers receives invalid data (such as an unexpected
+    // compression flag in a streaming request), we're unable to write the error
+    // response.
+    // Later major versions have a more sensible behavior - we can revert this
+    // workaround once we stop supporting v16.
+    destroyOnReturn: false,
+  });
+  for await (const chunk of it) {
     yield chunk;
   }
 }