mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(gateway): finish plugin HTTP responses after post-header failures (#102125)
* fix(gateway): finish plugin HTTP responses after post-header failures * test(gateway): satisfy plugin HTTP regression lint * fix(gateway): skip ending destroyed plugin responses --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
co-authored by
Peter Steinberger
parent
67be0cda44
commit
240d350c7f
@@ -1,5 +1,5 @@
|
||||
// Plugin HTTP routing tests cover route matching, gateway auth decisions, and upgrade dispatch.
|
||||
import type { IncomingMessage, ServerResponse } from "node:http";
|
||||
import { createServer, type IncomingMessage, type ServerResponse } from "node:http";
|
||||
import type { Duplex } from "node:stream";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { registerPluginHttpRoute } from "../../plugins/http-registry.js";
|
||||
@@ -450,6 +450,102 @@ describe("createGatewayPluginRequestHandler", () => {
|
||||
expect(setHeader).toHaveBeenCalledWith("Content-Type", "text/plain; charset=utf-8");
|
||||
expect(end).toHaveBeenCalledWith("Internal Server Error");
|
||||
});
|
||||
|
||||
it("ends a plugin route response when the route throws after sending headers", async () => {
|
||||
const log = createPluginLog();
|
||||
const handler = createGatewayPluginRequestHandler({
|
||||
registry: createTestRegistry({
|
||||
httpRoutes: [
|
||||
createRoute({
|
||||
path: "/partial",
|
||||
handler: async (_req, res) => {
|
||||
res.writeHead(200, { "Content-Type": "text/plain; charset=utf-8" });
|
||||
res.write("partial");
|
||||
throw new Error("boom");
|
||||
},
|
||||
}),
|
||||
],
|
||||
}),
|
||||
log,
|
||||
});
|
||||
const server = createServer((req, res) => {
|
||||
void (async () => {
|
||||
const handled = await handler(req, res);
|
||||
if (!handled) {
|
||||
res.statusCode = 404;
|
||||
res.end("not found");
|
||||
}
|
||||
})();
|
||||
});
|
||||
|
||||
await new Promise<void>((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");
|
||||
}
|
||||
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" });
|
||||
expect(log.warn).toHaveBeenCalledWith("plugin http route failed (route): Error: boom");
|
||||
} finally {
|
||||
if (timeout) {
|
||||
clearTimeout(timeout);
|
||||
}
|
||||
await new Promise<void>((resolve) => {
|
||||
server.close(() => resolve());
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("does not end a response the plugin already destroyed before throwing", async () => {
|
||||
const log = createPluginLog();
|
||||
const handler = createGatewayPluginRequestHandler({
|
||||
registry: createTestRegistry({
|
||||
httpRoutes: [
|
||||
createRoute({
|
||||
path: "/destroyed",
|
||||
handler: async (_req, res) => {
|
||||
Object.defineProperty(res, "headersSent", { value: true, configurable: true });
|
||||
res.destroy();
|
||||
throw new Error("boom");
|
||||
},
|
||||
}),
|
||||
],
|
||||
}),
|
||||
log,
|
||||
});
|
||||
const { res, end } = makeMockHttpResponse();
|
||||
|
||||
const handled = await handler({ url: "/destroyed" } as IncomingMessage, res);
|
||||
|
||||
expect(handled).toBe(true);
|
||||
expect(res.destroyed).toBe(true);
|
||||
expect(end).not.toHaveBeenCalled();
|
||||
expect(log.warn).toHaveBeenCalledWith("plugin http route failed (route): Error: boom");
|
||||
});
|
||||
});
|
||||
|
||||
describe("createGatewayPluginUpgradeHandler", () => {
|
||||
|
||||
@@ -201,6 +201,8 @@ export function createGatewayPluginRequestHandler(params: {
|
||||
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();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user