From 89a7e4e965bfefdbd699a15e5e17295a002ff27f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 19:34:32 -0700 Subject: [PATCH] fix(gateway): abort failed partial HTTP responses (#126130) --- src/gateway/http-common.ts | 6 +- src/gateway/server-http.request-trace.test.ts | 124 +++++++++--------- src/gateway/server/plugins-http.test.ts | 32 +---- 3 files changed, 73 insertions(+), 89 deletions(-) diff --git a/src/gateway/http-common.ts b/src/gateway/http-common.ts index 1473c7502b9c..488ff459cc88 100644 --- a/src/gateway/http-common.ts +++ b/src/gateway/http-common.ts @@ -42,10 +42,8 @@ export function finishFailedGatewayHttpResponse(res: ServerResponse): void { return; } - // Flush committed bytes before closing; truncated fixed-length bodies cannot reuse this socket. - const socket = res.socket; - res.end(); - socket?.end(); + // Ending would frame a partial chunked body as a complete successful response. + res.destroy(); } export function sendJson(res: ServerResponse, status: number, body: unknown) { diff --git a/src/gateway/server-http.request-trace.test.ts b/src/gateway/server-http.request-trace.test.ts index 0e1c171e8c99..2fa23f46e5c2 100644 --- a/src/gateway/server-http.request-trace.test.ts +++ b/src/gateway/server-http.request-trace.test.ts @@ -106,70 +106,74 @@ describe("gateway HTTP request trace scope", () => { }); describe("gateway HTTP request error cleanup", () => { - it.each([ - { - label: "partially written", - writeResponse: (res: ServerResponse) => res.write("partial"), - expectedBody: "partial", - }, - { - label: "already completed", - writeResponse: (res: ServerResponse) => res.end("complete"), - expectedBody: "complete", - }, - { - label: "fully written fixed-length", - writeResponse: (res: ServerResponse) => { - res.setHeader("Content-Length", "7"); - res.write("partial"); + it("preserves a response the route already completed before throwing", async () => { + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}); + const server = createGatewayHttpServer({ + clients: new Set(), + controlUiEnabled: false, + controlUiBasePath: "", + openAiChatCompletionsEnabled: false, + openResponsesEnabled: false, + handleHooksRequest: async (_req, res) => { + res.end("complete"); + throw new Error("route failed after completing a response"); }, - expectedBody: "partial", - }, - { - label: "fully written writeHead fixed-length", - writeResponse: (res: ServerResponse) => { - res.writeHead(200, { "Content-Length": "7" }); - res.write("partial"); - }, - expectedBody: "partial", - }, - ])( - "finishes a $label response after its route throws", - async ({ writeResponse, expectedBody }) => { - const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}); - const server = createGatewayHttpServer({ - clients: new Set(), - controlUiEnabled: false, - controlUiBasePath: "", - openAiChatCompletionsEnabled: false, - openResponsesEnabled: false, - handleHooksRequest: async (_req, res) => { - writeResponse(res); - throw new Error("route failed after writing a response"); - }, - resolvedAuth, - getRuntimeConfig: () => ({}), + resolvedAuth, + getRuntimeConfig: () => ({}), + }); + const port = await listen(server); + + try { + const response = await fetch(`http://127.0.0.1:${port}/hooks/test`, { + signal: AbortSignal.timeout(1_000), }); - const port = await listen(server); - try { - const response = await fetch(`http://127.0.0.1:${port}/hooks/test`, { + expect(response.status).toBe(200); + expect(await response.text()).toBe("complete"); + expect(errorLog).toHaveBeenCalledWith( + "[gateway-http] unhandled error in request handler:", + expect.any(Error), + ); + } finally { + server.closeAllConnections(); + await closeServer(server); + errorLog.mockRestore(); + } + }); + + it("aborts an incomplete unframed response after its route throws", async () => { + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}); + const server = createGatewayHttpServer({ + clients: new Set(), + controlUiEnabled: false, + controlUiBasePath: "", + openAiChatCompletionsEnabled: false, + openResponsesEnabled: false, + handleHooksRequest: async (_req, res) => { + res.write("partial"); + throw new Error("route failed after writing a partial response"); + }, + resolvedAuth, + getRuntimeConfig: () => ({}), + }); + const port = await listen(server); + + try { + await expect( + fetch(`http://127.0.0.1:${port}/hooks/test`, { signal: AbortSignal.timeout(1_000), - }); - - expect(response.status).toBe(200); - expect(await response.text()).toBe(expectedBody); - expect(errorLog).toHaveBeenCalledWith( - "[gateway-http] unhandled error in request handler:", - expect.any(Error), - ); - } finally { - server.closeAllConnections(); - await closeServer(server); - errorLog.mockRestore(); - } - }, - ); + }).then(async (response) => await response.text()), + ).rejects.toMatchObject({ name: "TypeError" }); + expect(errorLog).toHaveBeenCalledWith( + "[gateway-http] unhandled error in request handler:", + expect.any(Error), + ); + } finally { + server.closeAllConnections(); + await closeServer(server); + errorLog.mockRestore(); + } + }); it.each([ { diff --git a/src/gateway/server/plugins-http.test.ts b/src/gateway/server/plugins-http.test.ts index 57c67c504402..b0a6d632b049 100644 --- a/src/gateway/server/plugins-http.test.ts +++ b/src/gateway/server/plugins-http.test.ts @@ -395,7 +395,7 @@ describe("createGatewayPluginRequestHandler", () => { expect(end).toHaveBeenCalledWith("Internal Server Error"); }); - it("ends a plugin route response when the route throws after sending headers", async () => { + it("aborts an incomplete unframed response when the plugin route throws", async () => { const log = createPluginLog(); const handler = createGatewayPluginRequestHandler({ registry: createGatewayTestRegistry({ @@ -431,33 +431,15 @@ describe("createGatewayPluginRequestHandler", () => { if (!address || typeof address === "string") { throw new Error("server did not bind to a TCP port"); } - const controller = new AbortController(); - let timeout: ReturnType | undefined; - try { - const response = await fetch(`http://127.0.0.1:${address.port}/partial`, { - signal: controller.signal, - }); - const result = await Promise.race([ - response.text().then( - (body) => ({ kind: "body" as const, body }), - (err: unknown) => ({ kind: "error" as const, message: String(err) }), - ), - new Promise<{ kind: "timeout" }>((resolve) => { - timeout = setTimeout(() => { - controller.abort(); - resolve({ kind: "timeout" }); - }, 250); - }), - ]); - - expect(response.status).toBe(200); - expect(result).toEqual({ kind: "body", body: "partial" }); + await expect( + fetch(`http://127.0.0.1:${address.port}/partial`, { + signal: AbortSignal.timeout(1_000), + }).then(async (response) => await response.text()), + ).rejects.toMatchObject({ name: "TypeError" }); expect(log.warn).toHaveBeenCalledWith("plugin http route failed (route): Error: boom"); } finally { - if (timeout) { - clearTimeout(timeout); - } + server.closeAllConnections(); await new Promise((resolve) => { server.close(() => resolve()); });