diff --git a/packages/connect-node/src/http2-session-manager.spec.ts b/packages/connect-node/src/http2-session-manager.spec.ts index 3a79e507e..ce25b1800 100644 --- a/packages/connect-node/src/http2-session-manager.spec.ts +++ b/packages/connect-node/src/http2-session-manager.spec.ts @@ -479,6 +479,62 @@ describe("Http2SessionManager", () => { sm.abort(); }); }); + describe("with NO_ERROR and no open streams, immediately followed by a request", () => { + it("should not raise the deferred connection error as an uncaught exception", async () => { + const sm = new Http2SessionManager(server.getUrl()); + + // issue a request to open a connection, but close the request immediately + const req1 = await sm.request("POST", "/", {}, {}); + await new Promise((resolve) => setTimeout(resolve, 10)); + await new Promise((resolve) => + req1.close(http2.constants.NGHTTP2_NO_ERROR, resolve), + ); + await new Promise((resolve) => setTimeout(resolve, 10)); + assert.strictEqual( + sm.state(), + "idle", + "connection state after issuing a request and closing it", + ); + + // on the server, send a GOAWAY frame, and wait until the client + // session has processed it - the manager destroys the connection with + // a ConnectError, but Node.js defers the "error" event until the + // socket has closed + const conn = ( + sm as unknown as { s: { conn: http2.ClientHttp2Session } } + ).s.conn; + assert.strictEqual(serverSessions.length, 1); + serverSessions[0].goaway(http2.constants.NGHTTP2_NO_ERROR); + await new Promise((resolve) => + conn.once("goaway", () => resolve()), + ); + assert.strictEqual(conn.destroyed, true); + + // issue a second request before the deferred "error" event is raised + // on the destroyed connection - the manager exits the "ready" state + // and removes its error listeners, and the deferred error must not + // crash the process as an uncaught exception + const req2 = await sm.request("POST", "/", {}, {}); + await new Promise((resolve) => setTimeout(resolve, 20)); + assert.strictEqual( + sm.state(), + "open", + "connection state after issuing a second request", + ); + + // clean up + await new Promise((resolve) => + req2.close(http2.constants.NGHTTP2_NO_ERROR, resolve), + ); + + // Same as in the test above: the first session does not close on the + // server, and we have to close it here so that the test suite does not + // time out waiting for open connections. + await new Promise((resolve) => serverSessions[0].close(resolve)); + + sm.abort(); + }); + }); describe("with NO_ERROR and open stream that is closed after receiving the GOAWAY", () => { it("should close the session and open a new one for a second request", async () => { const sm = new Http2SessionManager(server.getUrl()); diff --git a/packages/connect-node/src/http2-session-manager.ts b/packages/connect-node/src/http2-session-manager.ts index d35e9965d..7a0fe78bb 100644 --- a/packages/connect-node/src/http2-session-manager.ts +++ b/packages/connect-node/src/http2-session-manager.ts @@ -722,6 +722,14 @@ function ready( // destroying the session), but later versions do not. // We call conn.destroy() because calling conn.close() ourselves is ineffective // here - it appears that the method is already being called, see https://github.com/nodejs/node/blob/198affc63973805ce5102d246f6b7822be57f5fc/lib/internal/http2/core.js#L681 + conn.once("error", () => { + // conn.destroy() defers the error until the socket has closed. If the + // manager exits the "ready" state before that (for example because a + // new request sees the destroyed connection and reconnects), our error + // listeners are removed in onExitState(), and the deferred error would + // be raised as an uncaught exception, crashing the process. + // We attach this one to swallow uncaught exceptions. + }); conn.destroy( new ConnectError( "received GOAWAY without any open streams",