mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-18 08:31:49 -06:00
fix(synology-chat): bound user_list fetches with a wall-clock deadline (#109111)
* fix(synology-chat): bound user_list fetches with a wall-clock deadline
* refactor(synology-chat): keep deadline internal
* style(synology-chat): format deadline proof
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
(cherry picked from commit f7cc0ef838)
This commit is contained in:
@@ -13,7 +13,9 @@ describe("Synology Chat user_list loopback", () => {
|
||||
if (server) {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server?.close((err) => (err ? reject(err) : resolve()));
|
||||
server?.closeAllConnections?.();
|
||||
});
|
||||
server = undefined;
|
||||
}
|
||||
});
|
||||
|
||||
@@ -71,4 +73,80 @@ describe("Synology Chat user_list loopback", () => {
|
||||
`fetchChatUsers: user_list response exceeded ${USER_LIST_RESPONSE_MAX_BYTES} bytes, using cached data`,
|
||||
);
|
||||
});
|
||||
|
||||
it("bounds a dripping user_list body with a wall-clock deadline", async () => {
|
||||
let requestCount = 0;
|
||||
server = http.createServer((_req, res) => {
|
||||
requestCount += 1;
|
||||
res.on("error", () => {});
|
||||
res.writeHead(200, {
|
||||
"Content-Type": "application/json",
|
||||
"Transfer-Encoding": "chunked",
|
||||
});
|
||||
if (requestCount === 1) {
|
||||
res.end(
|
||||
JSON.stringify({
|
||||
success: true,
|
||||
data: { users: [{ user_id: 21, username: "cached", nickname: "drip-user" }] },
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
// Keep sending bytes so ClientRequest socket-idle alone would never fire.
|
||||
const dripTimer = setInterval(() => {
|
||||
if (res.writableEnded || res.destroyed) {
|
||||
return;
|
||||
}
|
||||
res.write("x");
|
||||
}, 20);
|
||||
res.on("close", () => clearInterval(dripTimer));
|
||||
res.write("x");
|
||||
});
|
||||
server.on("clientError", (_err, socket) => socket.destroy());
|
||||
server.listen(0, "127.0.0.1");
|
||||
await once(server, "listening");
|
||||
|
||||
const address = server.address();
|
||||
if (!address || typeof address === "string") {
|
||||
throw new Error("expected loopback server address");
|
||||
}
|
||||
const incomingUrl =
|
||||
`http://127.0.0.1:${address.port}/webapi/entry.cgi?` +
|
||||
"api=SYNO.Chat.External&method=chatbot&version=2";
|
||||
const now = vi.spyOn(Date, "now");
|
||||
now.mockReturnValue(1_700_000_100_000);
|
||||
|
||||
await expect(
|
||||
resolveLegacyWebhookNameToChatUserId({
|
||||
incomingUrl,
|
||||
mutableWebhookUsername: "drip-user",
|
||||
}),
|
||||
).resolves.toBe(21);
|
||||
|
||||
now.mockReturnValue(1_700_000_100_000 + 10 * 60 * 1000);
|
||||
const warnings: string[] = [];
|
||||
const timeoutMs = 250;
|
||||
const nativeSetTimeout = globalThis.setTimeout;
|
||||
const timeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
timeoutSpy.mockImplementationOnce(((
|
||||
callback: (...args: unknown[]) => void,
|
||||
_delay?: number,
|
||||
...args: unknown[]
|
||||
) => nativeSetTimeout(callback, timeoutMs, ...args)) as typeof setTimeout);
|
||||
const startedAt = performance.now();
|
||||
await expect(
|
||||
resolveLegacyWebhookNameToChatUserId({
|
||||
incomingUrl,
|
||||
mutableWebhookUsername: "drip-user",
|
||||
log: { warn: (...args) => warnings.push(args.map(String).join(" ")) },
|
||||
}),
|
||||
).resolves.toBe(21);
|
||||
const elapsedMs = performance.now() - startedAt;
|
||||
|
||||
expect(requestCount).toBe(2);
|
||||
expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), 15_000);
|
||||
expect(warnings).toContain("fetchChatUsers: request timed out, using cached data");
|
||||
expect(elapsedMs).toBeGreaterThanOrEqual(timeoutMs - 50);
|
||||
expect(elapsedMs).toBeLessThan(timeoutMs + 1_500);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,6 +19,8 @@ import { z } from "zod";
|
||||
const MIN_SEND_INTERVAL_MS = 500;
|
||||
/** user_list JSON can be larger than inbound webhook pre-auth payloads. */
|
||||
const USER_LIST_RESPONSE_MAX_BYTES = 1 * 1024 * 1024;
|
||||
/** Wall-clock budget for user_list fetch including response body. */
|
||||
const USER_LIST_REQUEST_TIMEOUT_MS = 15_000;
|
||||
let lastSendTime = 0;
|
||||
let sendQueue: Promise<void> = Promise.resolve();
|
||||
|
||||
@@ -161,14 +163,21 @@ async function fetchChatUsers(
|
||||
if (cached && now - cached.cachedAt < CACHE_TTL_MS) {
|
||||
return cached.users;
|
||||
}
|
||||
|
||||
return new Promise((resolve) => {
|
||||
let settled = false;
|
||||
let deadlineTimer: ReturnType<typeof setTimeout> | undefined;
|
||||
const clearDeadline = () => {
|
||||
if (deadlineTimer !== undefined) {
|
||||
clearTimeout(deadlineTimer);
|
||||
deadlineTimer = undefined;
|
||||
}
|
||||
};
|
||||
const finish = (users: ChatUser[]) => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
clearDeadline();
|
||||
resolve(users);
|
||||
};
|
||||
let parsedUrl: URL;
|
||||
@@ -227,14 +236,21 @@ async function fetchChatUsers(
|
||||
})();
|
||||
})
|
||||
.on("error", (err) => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
log?.warn(`fetchChatUsers: HTTP error — ${err instanceof Error ? err.message : err}`);
|
||||
finish(cached?.users ?? []);
|
||||
});
|
||||
req.setTimeout?.(15_000, () => {
|
||||
// Use a wall-clock deadline, not ClientRequest.setTimeout. Node's socket
|
||||
// idle timer resets on every data chunk, so a slow drip can hang user_list
|
||||
// past the intended budget while body reads have no separate idle bound.
|
||||
deadlineTimer = setTimeout(() => {
|
||||
log?.warn("fetchChatUsers: request timed out, using cached data");
|
||||
req.destroy?.();
|
||||
finish(cached?.users ?? []);
|
||||
});
|
||||
}, USER_LIST_REQUEST_TIMEOUT_MS);
|
||||
deadlineTimer.unref?.();
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user