Skip to content

Commit

Permalink
fix(ext/node): session close during stream setup (#25170)
Browse files Browse the repository at this point in the history
Signed-off-by: Caleb Lloyd <caleblloyd@gmail.com>
  • Loading branch information
caleblloyd authored and lucacasonato committed Sep 4, 2024
1 parent f3e0d98 commit 74ea800
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 3 deletions.
18 changes: 17 additions & 1 deletion ext/node/polyfills/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
);
Expand All @@ -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]() {
Expand Down
4 changes: 2 additions & 2 deletions ext/node/polyfills/internal/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
Expand Down
63 changes: 63 additions & 0 deletions tests/unit_node/http2_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

0 comments on commit 74ea800

Please sign in to comment.