Repository: denoland/deno
Pull request: fix(ext/node): session close during stream setup

- Fixes #25168 
- Adds tests for #25143

Changed files:
filename: ext/node/polyfills/http2.ts
patch: @@ -840,6 +840,11 @@ async function clientHttp2Request(
     reqHeaders,
   );
 
+  if (session.closed || session.destroyed) {
+    debugHttp2(">>> session closed during request promise");
+    throw new ERR_HTTP2_STREAM_CANCEL();
+  }
+
   return await op_http2_client_request(
     session[kDenoClientRid],
     pseudoHeaders,
@@ -900,6 +905,12 @@ export class ClientHttp2Stream extends Duplex {
         session[kDenoClientRid],
         this.#rid,
       );
+
+      if (session.closed || session.destroyed) {
+        debugHttp2(">>> session closed during response promise");
+        throw new ERR_HTTP2_STREAM_CANCEL();
+      }
+
       const [response, endStream] = await op_http2_client_get_response(
         this.#rid,
       );
@@ -918,7 +929,12 @@ export class ClientHttp2Stream extends Duplex {
       );
       this[kDenoResponse] = response;
       this.emit("ready");
-    })();
+    })().catch((e) => {
+      if (!(e instanceof ERR_HTTP2_STREAM_CANCEL)) {
+        debugHttp2(">>> request/response promise error", e);
+      }
+      this.destroy(e);
+    });
   }
 
   [kUpdateTimer]() {

filename: ext/node/polyfills/internal/errors.ts
patch: @@ -2295,10 +2295,10 @@ export class ERR_HTTP2_INVALID_SETTING_VALUE extends NodeRangeError {
 }
 export class ERR_HTTP2_STREAM_CANCEL extends NodeError {
   override cause?: Error;
-  constructor(error: Error) {
+  constructor(error?: Error) {
     super(
       "ERR_HTTP2_STREAM_CANCEL",
-      typeof error.message === "string"
+      error && typeof error.message === "string"
         ? `The pending stream has been canceled (caused by: ${error.message})`
         : "The pending stream has been canceled",
     );

filename: tests/unit_node/http2_test.ts
patch: @@ -315,3 +315,66 @@ Deno.test("[node/http2 ClientHttp2Session.socket]", async () => {
   client.socket.setTimeout(0);
   assertEquals(receivedData, "hello world\n");
 });
+
+Deno.test("[node/http2 client] connection states", async () => {
+  const expected = {
+    beforeConnect: { connecting: true, closed: false, destroyed: false },
+    afterConnect: { connecting: false, closed: false, destroyed: false },
+    afterClose: { connecting: false, closed: true, destroyed: false },
+    afterDestroy: { connecting: false, closed: true, destroyed: true },
+  };
+  const actual: Partial<typeof expected> = {};
+
+  const url = "http://127.0.0.1:4246";
+  const connectPromise = Promise.withResolvers<void>();
+  const client = http2.connect(url, {}, () => {
+    connectPromise.resolve();
+  });
+  client.on("error", (err) => console.error(err));
+
+  // close event happens after destory has been called
+  const destroyPromise = Promise.withResolvers<void>();
+  client.on("close", () => {
+    destroyPromise.resolve();
+  });
+
+  actual.beforeConnect = {
+    connecting: client.connecting,
+    closed: client.closed,
+    destroyed: client.destroyed,
+  };
+
+  await connectPromise.promise;
+  actual.afterConnect = {
+    connecting: client.connecting,
+    closed: client.closed,
+    destroyed: client.destroyed,
+  };
+
+  // leave a request open to prevent immediate destroy
+  const req = client.request();
+  req.on("data", () => {});
+  req.on("error", (err) => console.error(err));
+  const reqClosePromise = Promise.withResolvers<void>();
+  req.on("close", () => {
+    reqClosePromise.resolve();
+  });
+
+  client.close();
+  actual.afterClose = {
+    connecting: client.connecting,
+    closed: client.closed,
+    destroyed: client.destroyed,
+  };
+
+  await destroyPromise.promise;
+  actual.afterDestroy = {
+    connecting: client.connecting,
+    closed: client.closed,
+    destroyed: client.destroyed,
+  };
+
+  await reqClosePromise.promise;
+
+  assertEquals(actual, expected);
+});