From c19e55f2269a385196fcd3c9e5e2a32b042752be Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 31 Jul 2026 08:14:07 -0700 Subject: [PATCH] fix(gateway): finish failed HTTP responses without hanging or crashing (#116874) * fix(gateway): unify failed HTTP response finalization * test(gateway): avoid returning server-close promise executor --------- Co-authored-by: Peter Steinberger --- src/gateway/http-common.ts | 18 +++ src/gateway/server-http.request-trace.test.ts | 138 +++++++++++++++++- src/gateway/server-http.ts | 26 ++-- src/gateway/server/plugins-http.test.ts | 56 +++++++ src/gateway/server/plugins-http.ts | 9 +- 5 files changed, 229 insertions(+), 18 deletions(-) diff --git a/src/gateway/http-common.ts b/src/gateway/http-common.ts index 37cfbe776a58..894e663ae1a7 100644 --- a/src/gateway/http-common.ts +++ b/src/gateway/http-common.ts @@ -28,6 +28,24 @@ export function setDefaultSecurityHeaders( } } +/** Finish a failed request without rewriting committed headers or orphaning its transport. */ +export function finishFailedGatewayHttpResponse(res: ServerResponse): void { + if (res.destroyed || res.writableEnded) { + return; + } + if (!res.headersSent) { + res.statusCode = 500; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end("Internal Server Error"); + return; + } + + // Flush committed bytes before closing; truncated fixed-length bodies cannot reuse this socket. + const socket = res.socket; + res.end(); + socket?.end(); +} + export function sendJson(res: ServerResponse, status: number, body: unknown) { res.statusCode = status; res.setHeader("Content-Type", "application/json; charset=utf-8"); diff --git a/src/gateway/server-http.request-trace.test.ts b/src/gateway/server-http.request-trace.test.ts index 43496ffffcc7..0f4abf5148f0 100644 --- a/src/gateway/server-http.request-trace.test.ts +++ b/src/gateway/server-http.request-trace.test.ts @@ -1,9 +1,10 @@ // HTTP request trace tests ensure gateway request scope reaches logs and // diagnostic events for per-request debugging. import fs from "node:fs"; +import type { ServerResponse } from "node:http"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { emitDiagnosticEvent, onDiagnosticEvent, @@ -103,3 +104,138 @@ 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"); + }, + 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: () => ({}), + }); + const port = await listen(server); + + try { + const response = await 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(); + } + }, + ); + + it.each([ + { + label: "setHeader", + setContentLength: (res: ServerResponse) => res.setHeader("Content-Length", "10"), + }, + { + label: "writeHead", + setContentLength: (res: ServerResponse) => res.writeHead(200, { "Content-Length": "10" }), + }, + ])("closes an incomplete $label fixed-length response", async ({ setContentLength }) => { + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}); + const server = createGatewayHttpServer({ + clients: new Set(), + controlUiEnabled: false, + controlUiBasePath: "", + openAiChatCompletionsEnabled: false, + openResponsesEnabled: false, + handleHooksRequest: async (_req, res) => { + setContentLength(res); + res.write("partial"); + throw new Error("route failed before completing its fixed-length 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), + }).then(async (response) => await response.text()), + ).rejects.toMatchObject({ name: "TypeError" }); + } finally { + server.closeAllConnections(); + await closeServer(server); + errorLog.mockRestore(); + } + }); + + it("preserves the 500 response when its route fails before writing headers", async () => { + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}); + const server = createGatewayHttpServer({ + clients: new Set(), + controlUiEnabled: false, + controlUiBasePath: "", + openAiChatCompletionsEnabled: false, + openResponsesEnabled: false, + handleHooksRequest: async () => { + throw new Error("route failed before writing a response"); + }, + resolvedAuth, + getRuntimeConfig: () => ({}), + }); + const port = await listen(server); + + try { + const response = await fetch(`http://127.0.0.1:${port}/hooks/test`); + + expect(response.status).toBe(500); + expect(await response.text()).toBe("Internal Server Error"); + } finally { + server.closeAllConnections(); + await closeServer(server); + errorLog.mockRestore(); + } + }); +}); diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index b1e98833b348..87e4f7447e51 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -38,7 +38,11 @@ import { } from "./control-ui-routing.js"; import type { ControlUiRootState } from "./control-ui.js"; import type { AuthorizedGatewayHttpRequest } from "./http-auth-utils.js"; -import { sendGatewayAuthFailure, setDefaultSecurityHeaders } from "./http-common.js"; +import { + finishFailedGatewayHttpResponse, + sendGatewayAuthFailure, + setDefaultSecurityHeaders, +} from "./http-common.js"; import { resolveRequestClientIp } from "./net.js"; import { normalizePluginNodeCapabilityScopedUrl, @@ -544,13 +548,17 @@ export function createGatewayHttpServer(opts: { const getResolvedAuth = opts.getResolvedAuth ?? (() => resolvedAuth); const loadGatewayConfig = opts.getRuntimeConfig ?? getRuntimeConfig; const openAiCompatEnabled = openAiChatCompletionsEnabled || openResponsesEnabled; + const handleServerRequest = (req: IncomingMessage, res: ServerResponse) => { + void handleRequestWithTrace(req, res).catch((error: unknown) => { + console.error("[gateway-http] failed to finalize request:", error); + if (!res.destroyed) { + res.destroy(error instanceof Error ? error : undefined); + } + }); + }; const httpServer: HttpServer = opts.tlsOptions - ? createHttpsServer(opts.tlsOptions, (req, res) => { - void handleRequestWithTrace(req, res); - }) - : createHttpServer((req, res) => { - void handleRequestWithTrace(req, res); - }); + ? createHttpsServer(opts.tlsOptions, handleServerRequest) + : createHttpServer(handleServerRequest); function handleRequestWithTrace(req: IncomingMessage, res: ServerResponse) { return runWithDiagnosticTraceContext(createDiagnosticTraceContext(), () => @@ -979,9 +987,7 @@ export function createGatewayHttpServer(opts: { res.end("Not Found"); } catch (err) { console.error("[gateway-http] unhandled error in request handler:", err); - res.statusCode = 500; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end("Internal Server Error"); + finishFailedGatewayHttpResponse(res); } } diff --git a/src/gateway/server/plugins-http.test.ts b/src/gateway/server/plugins-http.test.ts index 98b94c905536..0b5e58d71d88 100644 --- a/src/gateway/server/plugins-http.test.ts +++ b/src/gateway/server/plugins-http.test.ts @@ -520,6 +520,62 @@ describe("createGatewayPluginRequestHandler", () => { } }); + it.each([ + { + label: "setHeader", + setContentLength: (res: ServerResponse) => res.setHeader("Content-Length", "10"), + }, + { + label: "writeHead", + setContentLength: (res: ServerResponse) => res.writeHead(200, { "Content-Length": "10" }), + }, + ])( + "closes an incomplete plugin $label fixed-length response after its route throws", + async ({ setContentLength }) => { + const log = createPluginLog(); + const handler = createGatewayPluginRequestHandler({ + registry: createTestRegistry({ + httpRoutes: [ + createRoute({ + path: "/incomplete", + handler: async (_req, res) => { + setContentLength(res); + res.write("partial"); + throw new Error("boom"); + }, + }), + ], + }), + log, + }); + const server = createServer((req, res) => { + void handler(req, res); + }); + + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => resolve()); + }); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("server did not bind to a TCP port"); + } + + try { + await expect( + fetch(`http://127.0.0.1:${address.port}/incomplete`, { + signal: AbortSignal.timeout(500), + }).then(async (response) => await response.text()), + ).rejects.toMatchObject({ name: "TypeError" }); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + } + }, + ); + it("does not end a response the plugin already destroyed before throwing", async () => { const log = createPluginLog(); const handler = createGatewayPluginRequestHandler({ diff --git a/src/gateway/server/plugins-http.ts b/src/gateway/server/plugins-http.ts index b508c9d25673..724097126288 100644 --- a/src/gateway/server/plugins-http.ts +++ b/src/gateway/server/plugins-http.ts @@ -10,6 +10,7 @@ import type { createSubsystemLogger } from "../../logging/subsystem.js"; import type { PluginHttpRouteRegistration, PluginRegistry } from "../../plugins/registry.js"; import { withPluginRuntimeGatewayRequestScope } from "../../plugins/runtime/gateway-request-scope.js"; import { respondControlUiPluginAuthCookieProbe } from "../control-ui-plugin-auth-cookie.js"; +import { finishFailedGatewayHttpResponse } from "../http-common.js"; import type { AuthorizedGatewayHttpRequest } from "../http-utils.js"; import type { GatewayRequestContext, GatewayRequestOptions } from "../server-methods/types.js"; import { @@ -272,13 +273,7 @@ export function createGatewayPluginRequestHandler(params: { } } catch (err) { log.warn(`plugin http route failed (${route.pluginId ?? "unknown"}): ${String(err)}`); - if (!res.headersSent) { - res.statusCode = 500; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end("Internal Server Error"); - } else if (!res.writableEnded && !res.destroyed) { - res.end(); - } + finishFailedGatewayHttpResponse(res); return true; } }