diff options
author | Caleb Lloyd <2414837+caleblloyd@users.noreply.github.com> | 2024-08-30 13:46:17 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-30 23:16:17 +0530 |
commit | 4639ae655e9db396fdf4408961db59372334b69b (patch) | |
tree | 2f661d5d296f07284e3a24907a34918f17e6df09 | |
parent | d71eebba0d1ddc43aaf1e0c41f459deebaddc9f7 (diff) |
fix(ext/node): session close during stream setup (#25170)
Signed-off-by: Caleb Lloyd <caleblloyd@gmail.com>
-rw-r--r-- | ext/node/polyfills/http2.ts | 18 | ||||
-rw-r--r-- | ext/node/polyfills/internal/errors.ts | 4 | ||||
-rw-r--r-- | tests/unit_node/http2_test.ts | 63 |
3 files changed, 82 insertions, 3 deletions
diff --git a/ext/node/polyfills/http2.ts b/ext/node/polyfills/http2.ts index cd6c47eeb..b4cda65f8 100644 --- a/ext/node/polyfills/http2.ts +++ b/ext/node/polyfills/http2.ts @@ -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]() { diff --git a/ext/node/polyfills/internal/errors.ts b/ext/node/polyfills/internal/errors.ts index 9e0905ef4..51bd7a025 100644 --- a/ext/node/polyfills/internal/errors.ts +++ b/ext/node/polyfills/internal/errors.ts @@ -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", ); diff --git a/tests/unit_node/http2_test.ts b/tests/unit_node/http2_test.ts index 8e98bd28d..1dfac8f8c 100644 --- a/tests/unit_node/http2_test.ts +++ b/tests/unit_node/http2_test.ts @@ -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); +}); |