mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-22 10:25:20 -06:00
fix(gateway): abort failed partial HTTP responses (#126130)
This commit is contained in:
committed by
GitHub
parent
0eac4f7a3c
commit
89a7e4e965
@@ -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) {
|
||||
|
||||
@@ -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([
|
||||
{
|
||||
|
||||
@@ -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<typeof setTimeout> | 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<void>((resolve) => {
|
||||
server.close(() => resolve());
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user