mirror of https://github.com/openclaw/openclaw.git
fix(gateway): bound unanswered client requests (#45689)
* fix(gateway): bound unanswered client requests * fix(gateway): skip default timeout for expectFinal requests * fix(gateway): preserve gateway call timeouts * fix(gateway): localize request timeout policy * fix(gateway): clamp explicit request timeouts * fix(gateway): clamp default request timeout
This commit is contained in:
parent
bc3319207c
commit
5fc43ff0ec
|
|
@ -17,6 +17,8 @@ Docs: https://docs.openclaw.ai
|
|||
### Fixes
|
||||
|
||||
- Dashboard/chat UI: stop reloading full chat history on every live tool result in dashboard v2 so tool-heavy runs no longer trigger UI freeze/re-render storms while the final event still refreshes persisted history. (#45541) Thanks @BunsDev.
|
||||
- Gateway/client requests: reject unanswered gateway RPC calls after a bounded timeout and clear their pending state, so stalled connections no longer leak hanging `GatewayClient.request()` promises indefinitely.
|
||||
- Build/plugin-sdk bundling: bundle plugin-sdk subpath entries in one shared build pass so published packages stop duplicating shared chunks and avoid the recent plugin-sdk memory blow-up. (#45426) Thanks @TarasShyn.
|
||||
- Ollama/reasoning visibility: stop promoting native `thinking` and `reasoning` fields into final assistant text so local reasoning models no longer leak internal thoughts in normal replies. (#45330) Thanks @xi7ang.
|
||||
- Android/onboarding QR scan: switch setup QR scanning to Google Code Scanner so onboarding uses a more reliable scanner instead of the legacy embedded ZXing flow. (#45021) Thanks @obviyus.
|
||||
- Browser/existing-session: harden driver validation and session lifecycle so transport errors trigger reconnects while tool-level errors preserve the session, and extract shared ARIA role sets to deduplicate Playwright and Chrome MCP snapshot paths. (#45682) Thanks @odysseus0.
|
||||
|
|
|
|||
|
|
@ -18,6 +18,11 @@ let lastClientOptions: {
|
|||
onHelloOk?: (hello: { features?: { methods?: string[] } }) => void | Promise<void>;
|
||||
onClose?: (code: number, reason: string) => void;
|
||||
} | null = null;
|
||||
let lastRequestOptions: {
|
||||
method?: string;
|
||||
params?: unknown;
|
||||
opts?: { expectFinal?: boolean; timeoutMs?: number | null };
|
||||
} | null = null;
|
||||
type StartMode = "hello" | "close" | "silent";
|
||||
let startMode: StartMode = "hello";
|
||||
let closeCode = 1006;
|
||||
|
|
@ -45,7 +50,12 @@ vi.mock("./client.js", () => ({
|
|||
}) {
|
||||
lastClientOptions = opts;
|
||||
}
|
||||
async request() {
|
||||
async request(
|
||||
method: string,
|
||||
params: unknown,
|
||||
opts?: { expectFinal?: boolean; timeoutMs?: number | null },
|
||||
) {
|
||||
lastRequestOptions = { method, params, opts };
|
||||
return { ok: true };
|
||||
}
|
||||
start() {
|
||||
|
|
@ -72,6 +82,7 @@ function resetGatewayCallMocks() {
|
|||
pickPrimaryTailnetIPv4.mockClear();
|
||||
pickPrimaryLanIPv4.mockClear();
|
||||
lastClientOptions = null;
|
||||
lastRequestOptions = null;
|
||||
startMode = "hello";
|
||||
closeCode = 1006;
|
||||
closeReason = "";
|
||||
|
|
@ -574,6 +585,25 @@ describe("callGateway error details", () => {
|
|||
expect(errMessage).toContain("gateway closed (1006");
|
||||
});
|
||||
|
||||
it("forwards caller timeout to client requests", async () => {
|
||||
setLocalLoopbackGatewayConfig();
|
||||
|
||||
await callGateway({ method: "health", timeoutMs: 45_000 });
|
||||
|
||||
expect(lastRequestOptions?.method).toBe("health");
|
||||
expect(lastRequestOptions?.opts?.timeoutMs).toBe(45_000);
|
||||
});
|
||||
|
||||
it("does not inject wrapper timeout defaults into expectFinal requests", async () => {
|
||||
setLocalLoopbackGatewayConfig();
|
||||
|
||||
await callGateway({ method: "health", expectFinal: true });
|
||||
|
||||
expect(lastRequestOptions?.method).toBe("health");
|
||||
expect(lastRequestOptions?.opts?.expectFinal).toBe(true);
|
||||
expect(lastRequestOptions?.opts?.timeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("fails fast when remote mode is missing remote url", async () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: { mode: "remote", bind: "loopback", remote: {} },
|
||||
|
|
|
|||
|
|
@ -848,6 +848,7 @@ async function executeGatewayRequestWithScopes<T>(params: {
|
|||
});
|
||||
const result = await client.request<T>(opts.method, opts.params, {
|
||||
expectFinal: opts.expectFinal,
|
||||
timeoutMs: opts.timeoutMs,
|
||||
});
|
||||
ignoreClose = true;
|
||||
stop(undefined, result);
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ type Pending = {
|
|||
resolve: (value: unknown) => void;
|
||||
reject: (err: unknown) => void;
|
||||
expectFinal: boolean;
|
||||
timeout: NodeJS.Timeout | null;
|
||||
};
|
||||
|
||||
type GatewayClientErrorShape = {
|
||||
|
|
@ -78,6 +79,7 @@ export type GatewayClientOptions = {
|
|||
url?: string; // ws://127.0.0.1:18789
|
||||
connectDelayMs?: number;
|
||||
tickWatchMinIntervalMs?: number;
|
||||
requestTimeoutMs?: number;
|
||||
token?: string;
|
||||
bootstrapToken?: string;
|
||||
deviceToken?: string;
|
||||
|
|
@ -136,6 +138,7 @@ export class GatewayClient {
|
|||
private lastTick: number | null = null;
|
||||
private tickIntervalMs = 30_000;
|
||||
private tickTimer: NodeJS.Timeout | null = null;
|
||||
private readonly requestTimeoutMs: number;
|
||||
|
||||
constructor(opts: GatewayClientOptions) {
|
||||
this.opts = {
|
||||
|
|
@ -145,6 +148,10 @@ export class GatewayClient {
|
|||
? undefined
|
||||
: (opts.deviceIdentity ?? loadOrCreateDeviceIdentity()),
|
||||
};
|
||||
this.requestTimeoutMs =
|
||||
typeof opts.requestTimeoutMs === "number" && Number.isFinite(opts.requestTimeoutMs)
|
||||
? Math.max(1, Math.min(Math.floor(opts.requestTimeoutMs), 2_147_483_647))
|
||||
: 30_000;
|
||||
}
|
||||
|
||||
start() {
|
||||
|
|
@ -586,6 +593,9 @@ export class GatewayClient {
|
|||
return;
|
||||
}
|
||||
this.pending.delete(parsed.id);
|
||||
if (pending.timeout) {
|
||||
clearTimeout(pending.timeout);
|
||||
}
|
||||
if (parsed.ok) {
|
||||
pending.resolve(parsed.payload);
|
||||
} else {
|
||||
|
|
@ -638,6 +648,9 @@ export class GatewayClient {
|
|||
|
||||
private flushPendingErrors(err: Error) {
|
||||
for (const [, p] of this.pending) {
|
||||
if (p.timeout) {
|
||||
clearTimeout(p.timeout);
|
||||
}
|
||||
p.reject(err);
|
||||
}
|
||||
this.pending.clear();
|
||||
|
|
@ -697,7 +710,7 @@ export class GatewayClient {
|
|||
async request<T = Record<string, unknown>>(
|
||||
method: string,
|
||||
params?: unknown,
|
||||
opts?: { expectFinal?: boolean },
|
||||
opts?: { expectFinal?: boolean; timeoutMs?: number | null },
|
||||
): Promise<T> {
|
||||
if (!this.ws || this.ws.readyState !== WebSocket.OPEN) {
|
||||
throw new Error("gateway not connected");
|
||||
|
|
@ -710,11 +723,27 @@ export class GatewayClient {
|
|||
);
|
||||
}
|
||||
const expectFinal = opts?.expectFinal === true;
|
||||
const timeoutMs =
|
||||
opts?.timeoutMs === null
|
||||
? null
|
||||
: typeof opts?.timeoutMs === "number" && Number.isFinite(opts.timeoutMs)
|
||||
? Math.max(1, Math.min(Math.floor(opts.timeoutMs), 2_147_483_647))
|
||||
: expectFinal
|
||||
? null
|
||||
: this.requestTimeoutMs;
|
||||
const p = new Promise<T>((resolve, reject) => {
|
||||
const timeout =
|
||||
timeoutMs === null
|
||||
? null
|
||||
: setTimeout(() => {
|
||||
this.pending.delete(id);
|
||||
reject(new Error(`gateway request timeout for ${method}`));
|
||||
}, timeoutMs);
|
||||
this.pending.set(id, {
|
||||
resolve: (value) => resolve(value as T),
|
||||
reject,
|
||||
expectFinal,
|
||||
timeout,
|
||||
});
|
||||
});
|
||||
this.ws.send(JSON.stringify(frame));
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { createServer as createHttpsServer } from "node:https";
|
||||
import { createServer } from "node:net";
|
||||
import { afterEach, describe, expect, test } from "vitest";
|
||||
import { WebSocketServer } from "ws";
|
||||
import { afterEach, describe, expect, test, vi } from "vitest";
|
||||
import { WebSocket, WebSocketServer } from "ws";
|
||||
import { rawDataToString } from "../infra/ws.js";
|
||||
import { GatewayClient } from "./client.js";
|
||||
|
||||
|
|
@ -85,6 +85,160 @@ describe("GatewayClient", () => {
|
|||
}
|
||||
}, 4000);
|
||||
|
||||
test("times out unresolved requests and clears pending state", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const client = new GatewayClient({
|
||||
requestTimeoutMs: 25,
|
||||
});
|
||||
const send = vi.fn();
|
||||
(
|
||||
client as unknown as {
|
||||
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
|
||||
}
|
||||
).ws = {
|
||||
readyState: WebSocket.OPEN,
|
||||
send,
|
||||
close: vi.fn(),
|
||||
};
|
||||
|
||||
const requestPromise = client.request("status");
|
||||
const requestExpectation = expect(requestPromise).rejects.toThrow(
|
||||
"gateway request timeout for status",
|
||||
);
|
||||
expect(send).toHaveBeenCalledTimes(1);
|
||||
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(25);
|
||||
|
||||
await requestExpectation;
|
||||
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(0);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
test("does not auto-timeout expectFinal requests", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const client = new GatewayClient({
|
||||
requestTimeoutMs: 25,
|
||||
});
|
||||
const send = vi.fn();
|
||||
(
|
||||
client as unknown as {
|
||||
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
|
||||
}
|
||||
).ws = {
|
||||
readyState: WebSocket.OPEN,
|
||||
send,
|
||||
close: vi.fn(),
|
||||
};
|
||||
|
||||
let settled = false;
|
||||
const requestPromise = client.request("chat.send", undefined, { expectFinal: true });
|
||||
void requestPromise.then(
|
||||
() => {
|
||||
settled = true;
|
||||
},
|
||||
() => {
|
||||
settled = true;
|
||||
},
|
||||
);
|
||||
expect(send).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(25);
|
||||
|
||||
expect(settled).toBe(false);
|
||||
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
|
||||
|
||||
client.stop();
|
||||
await expect(requestPromise).rejects.toThrow("gateway client stopped");
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
test("clamps oversized explicit request timeouts before scheduling", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const client = new GatewayClient({
|
||||
requestTimeoutMs: 25,
|
||||
});
|
||||
const send = vi.fn();
|
||||
(
|
||||
client as unknown as {
|
||||
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
|
||||
}
|
||||
).ws = {
|
||||
readyState: WebSocket.OPEN,
|
||||
send,
|
||||
close: vi.fn(),
|
||||
};
|
||||
|
||||
let settled = false;
|
||||
const requestPromise = client.request("status", undefined, { timeoutMs: 2_592_010_000 });
|
||||
void requestPromise.then(
|
||||
() => {
|
||||
settled = true;
|
||||
},
|
||||
() => {
|
||||
settled = true;
|
||||
},
|
||||
);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
|
||||
expect(settled).toBe(false);
|
||||
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
|
||||
|
||||
client.stop();
|
||||
await expect(requestPromise).rejects.toThrow("gateway client stopped");
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
test("clamps oversized default request timeouts before scheduling", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const client = new GatewayClient({
|
||||
requestTimeoutMs: 2_592_010_000,
|
||||
});
|
||||
const send = vi.fn();
|
||||
(
|
||||
client as unknown as {
|
||||
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
|
||||
}
|
||||
).ws = {
|
||||
readyState: WebSocket.OPEN,
|
||||
send,
|
||||
close: vi.fn(),
|
||||
};
|
||||
|
||||
let settled = false;
|
||||
const requestPromise = client.request("status");
|
||||
void requestPromise.then(
|
||||
() => {
|
||||
settled = true;
|
||||
},
|
||||
() => {
|
||||
settled = true;
|
||||
},
|
||||
);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
|
||||
expect(settled).toBe(false);
|
||||
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
|
||||
|
||||
client.stop();
|
||||
await expect(requestPromise).rejects.toThrow("gateway client stopped");
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects mismatched tls fingerprint", async () => {
|
||||
const key = [
|
||||
"-----BEGIN PRIVATE KEY-----", // pragma: allowlist secret
|
||||
|
|
|
|||
Loading…
Reference in New Issue