mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-22 23:01:11 +00:00
* fix(line): run post-ack webhook processing on its own admitted work root LINE acks the webhook and dispatches event processing fire-and-forget on the same async chain. The HTTP request admission that chain inherited is released as soon as the route handler returns, and a released admission refuses subordinate queue work - so every LINE inbound agent turn fails with "GatewayDrainingError: Gateway is draining; new tasks are not accepted" even though the gateway is healthy. DMs, group mentions, and postbacks are all affected; the user-visible symptom is the bot replying "Sorry, I encountered an error processing your message." to everything. Add runDetachedWebhookWork to the plugin-sdk webhook-request-guards surface (a thin wrapper over the gateway independent-root continuation, the same shape core uses in gateway/server/hooks.ts) and route all three LINE ack-first dispatch sites through it: the gateway monitor handler (the live path), and the createLineNodeWebhookHandler / Express middleware handlers (public webhook building blocks an embedder can register under the gateway). #65375 unified these three into one ack-first pattern; keeping the detach consistent avoids re-introducing the same latent defect in the two that are not on the live gateway path today. The continuation is reserved synchronously while the request is still admitted, so the detached processing stays accepted and a real restart drain can wait for it instead of stranding it mid-turn. Tests pin every layer: the guards suite proves detached post-ack work is admitted after the request admission is released (and that the inherited chain without the helper is refused); the monitor lifecycle suite and the webhook-node suite assert each dispatch site goes through the detached root. Red/green verified: reverting any dispatch fails its test. * fix(plugin-sdk): account for runDetachedWebhookWork in public surface budget * fix(channels): track detached webhook processing Co-authored-by: 許元豪 <146086744+edenfunf@users.noreply.github.com> * docs: refresh generated docs map * chore(plugin-sdk): refresh API baseline * fix(webhooks): preserve post-ack ordering * test(plugin-sdk): satisfy detached work lint --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
367 lines
11 KiB
TypeScript
367 lines
11 KiB
TypeScript
/**
|
|
* Tests webhook request guard body parsing and rejection behavior.
|
|
*/
|
|
import { EventEmitter } from "node:events";
|
|
import type { IncomingMessage } from "node:http";
|
|
import { describe, expect, it } from "vitest";
|
|
import { createMockServerResponse } from "../test-utils/mock-http-response.js";
|
|
import { createFixedWindowRateLimiter } from "./webhook-memory-guards.js";
|
|
import {
|
|
applyBasicWebhookRequestGuards,
|
|
beginWebhookRequestPipelineOrReject,
|
|
createWebhookInFlightLimiter,
|
|
isJsonContentType,
|
|
readWebhookBodyOrReject,
|
|
readJsonWebhookBodyOrReject,
|
|
runDetachedWebhookWork,
|
|
} from "./webhook-request-guards.js";
|
|
|
|
type MockIncomingMessage = IncomingMessage & {
|
|
destroyed?: boolean;
|
|
destroy: () => MockIncomingMessage;
|
|
};
|
|
|
|
function createMockRequest(params: {
|
|
method?: string;
|
|
headers?: Record<string, string>;
|
|
chunks?: string[];
|
|
emitEnd?: boolean;
|
|
}): MockIncomingMessage {
|
|
const req = new EventEmitter() as MockIncomingMessage;
|
|
req.method = params.method ?? "POST";
|
|
req.headers = params.headers ?? {};
|
|
req.destroyed = false;
|
|
req.destroy = (() => {
|
|
req.destroyed = true;
|
|
return req;
|
|
}) as MockIncomingMessage["destroy"];
|
|
|
|
if (params.chunks) {
|
|
void Promise.resolve().then(() => {
|
|
for (const chunk of params.chunks ?? []) {
|
|
req.emit("data", Buffer.from(chunk, "utf-8"));
|
|
}
|
|
if (params.emitEnd !== false) {
|
|
req.emit("end");
|
|
}
|
|
});
|
|
}
|
|
|
|
return req;
|
|
}
|
|
|
|
async function readJsonBody(chunks: string[], emptyObjectOnEmpty = false) {
|
|
const req = createMockRequest({ chunks });
|
|
const res = createMockServerResponse();
|
|
return {
|
|
result: await readJsonWebhookBodyOrReject({
|
|
req,
|
|
res,
|
|
maxBytes: 1024,
|
|
emptyObjectOnEmpty,
|
|
}),
|
|
res,
|
|
};
|
|
}
|
|
|
|
async function readRawBody(params: Parameters<typeof createMockRequest>[0], profile?: "pre-auth") {
|
|
const req = createMockRequest(params);
|
|
const res = createMockServerResponse();
|
|
return {
|
|
result: await readWebhookBodyOrReject({
|
|
req,
|
|
res,
|
|
profile,
|
|
}),
|
|
res,
|
|
};
|
|
}
|
|
|
|
describe("isJsonContentType", () => {
|
|
it.each([
|
|
{ name: "accepts application/json", input: "application/json", expected: true },
|
|
{
|
|
name: "accepts +json suffixes",
|
|
input: "application/cloudevents+json; charset=utf-8",
|
|
expected: true,
|
|
},
|
|
{ name: "rejects non-json media types", input: "text/plain", expected: false },
|
|
{ name: "rejects missing media types", input: undefined, expected: false },
|
|
])("$name", ({ input, expected }) => {
|
|
expect(isJsonContentType(input)).toBe(expected);
|
|
});
|
|
});
|
|
|
|
describe("applyBasicWebhookRequestGuards", () => {
|
|
it("rejects disallowed HTTP methods", () => {
|
|
const req = createMockRequest({ method: "GET" });
|
|
const res = createMockServerResponse();
|
|
const ok = applyBasicWebhookRequestGuards({
|
|
req,
|
|
res,
|
|
allowMethods: ["POST"],
|
|
});
|
|
expect(ok).toBe(false);
|
|
expect(res.statusCode).toBe(405);
|
|
expect(res.getHeader("allow")).toBe("POST");
|
|
});
|
|
|
|
it("enforces rate limits", () => {
|
|
const limiter = createFixedWindowRateLimiter({
|
|
windowMs: 60_000,
|
|
maxRequests: 1,
|
|
maxTrackedKeys: 10,
|
|
});
|
|
const req1 = createMockRequest({ method: "POST" });
|
|
const res1 = createMockServerResponse();
|
|
const req2 = createMockRequest({ method: "POST" });
|
|
const res2 = createMockServerResponse();
|
|
expect(
|
|
applyBasicWebhookRequestGuards({
|
|
req: req1,
|
|
res: res1,
|
|
rateLimiter: limiter,
|
|
rateLimitKey: "k",
|
|
nowMs: 1_000,
|
|
}),
|
|
).toBe(true);
|
|
expect(
|
|
applyBasicWebhookRequestGuards({
|
|
req: req2,
|
|
res: res2,
|
|
rateLimiter: limiter,
|
|
rateLimitKey: "k",
|
|
nowMs: 1_001,
|
|
}),
|
|
).toBe(false);
|
|
expect(res2.statusCode).toBe(429);
|
|
});
|
|
|
|
it.each([
|
|
{
|
|
name: "allows matching JSON requests",
|
|
req: createMockRequest({
|
|
method: "POST",
|
|
headers: { "content-type": "application/json" },
|
|
}),
|
|
expectedOk: true,
|
|
expectedStatusCode: 200,
|
|
},
|
|
{
|
|
name: "rejects non-json requests when required",
|
|
req: createMockRequest({
|
|
method: "POST",
|
|
headers: { "content-type": "text/plain" },
|
|
}),
|
|
expectedOk: false,
|
|
expectedStatusCode: 415,
|
|
},
|
|
])("$name", ({ req, expectedOk, expectedStatusCode }) => {
|
|
const res = createMockServerResponse();
|
|
const ok = applyBasicWebhookRequestGuards({
|
|
req,
|
|
res,
|
|
requireJsonContentType: true,
|
|
});
|
|
expect(ok).toBe(expectedOk);
|
|
expect(res.statusCode).toBe(expectedStatusCode);
|
|
});
|
|
});
|
|
|
|
describe("readJsonWebhookBodyOrReject", () => {
|
|
it.each([
|
|
{
|
|
name: "returns parsed JSON body",
|
|
chunks: ['{"ok":true}'],
|
|
expected: { ok: true, value: { ok: true } },
|
|
expectedStatusCode: 200,
|
|
expectedBody: undefined,
|
|
},
|
|
{
|
|
name: "preserves valid JSON null payload",
|
|
chunks: ["null"],
|
|
expected: { ok: true, value: null },
|
|
expectedStatusCode: 200,
|
|
expectedBody: undefined,
|
|
},
|
|
{
|
|
name: "writes 400 on invalid JSON payload",
|
|
chunks: ["{bad json"],
|
|
expected: { ok: false },
|
|
expectedStatusCode: 400,
|
|
expectedBody: "Bad Request",
|
|
},
|
|
])("$name", async ({ chunks, expected, expectedStatusCode, expectedBody }) => {
|
|
const { result, res } = await readJsonBody(chunks);
|
|
expect(result).toEqual(expected);
|
|
expect(res.statusCode).toBe(expectedStatusCode);
|
|
expect(res.body).toBe(expectedBody);
|
|
});
|
|
});
|
|
|
|
describe("readWebhookBodyOrReject", () => {
|
|
it("returns raw body contents", async () => {
|
|
const { result } = await readRawBody({ chunks: ["plain text"] });
|
|
expect(result).toEqual({ ok: true, value: "plain text" });
|
|
});
|
|
|
|
it("enforces strict pre-auth default body limits", async () => {
|
|
const { result, res } = await readRawBody(
|
|
{
|
|
headers: { "content-length": String(70 * 1024) },
|
|
},
|
|
"pre-auth",
|
|
);
|
|
expect(result).toEqual({ ok: false });
|
|
expect(res.statusCode).toBe(413);
|
|
});
|
|
});
|
|
|
|
describe("beginWebhookRequestPipelineOrReject", () => {
|
|
it("falls back for non-finite in-flight limiter options", () => {
|
|
const limiter = createWebhookInFlightLimiter({
|
|
maxInFlightPerKey: Number.NaN,
|
|
maxTrackedKeys: Number.NaN,
|
|
});
|
|
const releases: Array<() => void> = [];
|
|
try {
|
|
for (let index = 0; index < 8; index += 1) {
|
|
const result = beginWebhookRequestPipelineOrReject({
|
|
req: createMockRequest({ method: "POST" }),
|
|
res: createMockServerResponse(),
|
|
allowMethods: ["POST"],
|
|
inFlightLimiter: limiter,
|
|
inFlightKey: "ip:127.0.0.1",
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (result.ok) {
|
|
releases.push(result.release);
|
|
}
|
|
}
|
|
|
|
const overflowRes = createMockServerResponse();
|
|
const overflow = beginWebhookRequestPipelineOrReject({
|
|
req: createMockRequest({ method: "POST" }),
|
|
res: overflowRes,
|
|
allowMethods: ["POST"],
|
|
inFlightLimiter: limiter,
|
|
inFlightKey: "ip:127.0.0.1",
|
|
});
|
|
|
|
expect(overflow.ok).toBe(false);
|
|
expect(overflowRes.statusCode).toBe(429);
|
|
} finally {
|
|
for (const release of releases) {
|
|
release();
|
|
}
|
|
}
|
|
});
|
|
|
|
it("enforces in-flight request limits and releases slots", () => {
|
|
const limiter = createWebhookInFlightLimiter({
|
|
maxInFlightPerKey: 1,
|
|
maxTrackedKeys: 10,
|
|
});
|
|
|
|
const first = beginWebhookRequestPipelineOrReject({
|
|
req: createMockRequest({ method: "POST" }),
|
|
res: createMockServerResponse(),
|
|
allowMethods: ["POST"],
|
|
inFlightLimiter: limiter,
|
|
inFlightKey: "ip:127.0.0.1",
|
|
});
|
|
expect(first.ok).toBe(true);
|
|
|
|
const secondRes = createMockServerResponse();
|
|
const second = beginWebhookRequestPipelineOrReject({
|
|
req: createMockRequest({ method: "POST" }),
|
|
res: secondRes,
|
|
allowMethods: ["POST"],
|
|
inFlightLimiter: limiter,
|
|
inFlightKey: "ip:127.0.0.1",
|
|
});
|
|
expect(second.ok).toBe(false);
|
|
expect(secondRes.statusCode).toBe(429);
|
|
|
|
if (first.ok) {
|
|
first.release();
|
|
}
|
|
|
|
const third = beginWebhookRequestPipelineOrReject({
|
|
req: createMockRequest({ method: "POST" }),
|
|
res: createMockServerResponse(),
|
|
allowMethods: ["POST"],
|
|
inFlightLimiter: limiter,
|
|
inFlightKey: "ip:127.0.0.1",
|
|
});
|
|
expect(third.ok).toBe(true);
|
|
if (third.ok) {
|
|
third.release();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("runDetachedWebhookWork", () => {
|
|
it("defers the callback until the request handler can acknowledge", async () => {
|
|
const { runWithGatewayHttpWorkAdmission } =
|
|
await import("../gateway/server/http-work-admission.js");
|
|
const order: string[] = [];
|
|
const detached: Promise<void>[] = [];
|
|
|
|
await runWithGatewayHttpWorkAdmission(createMockServerResponse(), async () => {
|
|
detached.push(
|
|
runDetachedWebhookWork(async () => {
|
|
order.push("work");
|
|
}),
|
|
);
|
|
order.push("ack");
|
|
expect(order).toEqual(["ack"]);
|
|
return true;
|
|
});
|
|
|
|
await Promise.all(detached);
|
|
expect(order).toEqual(["ack", "work"]);
|
|
});
|
|
|
|
it("keeps post-ack processing admitted after the request admission is released", async () => {
|
|
const { runWithGatewayHttpWorkAdmission } =
|
|
await import("../gateway/server/http-work-admission.js");
|
|
const { enqueueCommandInLane } = await import("../process/command-queue.js");
|
|
|
|
let detached: Promise<number> | null = null;
|
|
await runWithGatewayHttpWorkAdmission(createMockServerResponse(), async () => {
|
|
// Ack-first shape: dispatch continues after the handler (and its
|
|
// admission) completes; the queue enqueue happens well past release.
|
|
detached = runDetachedWebhookWork(async () => {
|
|
await new Promise<void>((resolve) => {
|
|
setTimeout(resolve, 25);
|
|
});
|
|
return await enqueueCommandInLane("detached-webhook-work-test", async () => 42);
|
|
});
|
|
return true;
|
|
});
|
|
|
|
await expect(detached).resolves.toBe(42);
|
|
});
|
|
|
|
it("refuses the same post-ack processing when it merely inherits the request admission", async () => {
|
|
const { runWithGatewayHttpWorkAdmission } =
|
|
await import("../gateway/server/http-work-admission.js");
|
|
const { enqueueCommandInLane } = await import("../process/command-queue.js");
|
|
|
|
let inherited: Promise<number> | null = null;
|
|
await runWithGatewayHttpWorkAdmission(createMockServerResponse(), async () => {
|
|
inherited = (async () => {
|
|
await new Promise<void>((resolve) => {
|
|
setTimeout(resolve, 25);
|
|
});
|
|
return await enqueueCommandInLane("inherited-webhook-work-test", async () => 42);
|
|
})();
|
|
inherited.catch(() => {});
|
|
return true;
|
|
});
|
|
|
|
await expect(inherited).rejects.toThrow("Gateway is draining");
|
|
});
|
|
});
|