Skip to content

Commit 5fc43ff

Browse files
TakhoffmanTakhoffman
authored
fix(gateway): bound unanswered client requests (openclaw#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
1 parent bc33192 commit 5fc43ff

File tree

5 files changed

+220
-4
lines changed

5 files changed

+220
-4
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ Docs: https://docs.openclaw.ai
1717
### Fixes
1818

1919
- 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.
20+
- 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.
21+
- 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.
2022
- 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.
2123
- 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.
2224
- 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.

‎src/gateway/call.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ let lastClientOptions: {
1818
onHelloOk?: (hello: { features?: { methods?: string[] } }) => void | Promise<void>;
1919
onClose?: (code: number, reason: string) => void;
2020
} | null = null;
21+
let lastRequestOptions: {
22+
method?: string;
23+
params?: unknown;
24+
opts?: { expectFinal?: boolean; timeoutMs?: number | null };
25+
} | null = null;
2126
type StartMode = "hello" | "close" | "silent";
2227
let startMode: StartMode = "hello";
2328
let closeCode = 1006;
@@ -45,7 +50,12 @@ vi.mock("./client.js", () => ({
4550
}) {
4651
lastClientOptions = opts;
4752
}
48-
async request() {
53+
async request(
54+
method: string,
55+
params: unknown,
56+
opts?: { expectFinal?: boolean; timeoutMs?: number | null },
57+
) {
58+
lastRequestOptions = { method, params, opts };
4959
return { ok: true };
5060
}
5161
start() {
@@ -72,6 +82,7 @@ function resetGatewayCallMocks() {
7282
pickPrimaryTailnetIPv4.mockClear();
7383
pickPrimaryLanIPv4.mockClear();
7484
lastClientOptions = null;
85+
lastRequestOptions = null;
7586
startMode = "hello";
7687
closeCode = 1006;
7788
closeReason = "";
@@ -574,6 +585,25 @@ describe("callGateway error details", () => {
574585
expect(errMessage).toContain("gateway closed (1006");
575586
});
576587

588+
it("forwards caller timeout to client requests", async () => {
589+
setLocalLoopbackGatewayConfig();
590+
591+
await callGateway({ method: "health", timeoutMs: 45_000 });
592+
593+
expect(lastRequestOptions?.method).toBe("health");
594+
expect(lastRequestOptions?.opts?.timeoutMs).toBe(45_000);
595+
});
596+
597+
it("does not inject wrapper timeout defaults into expectFinal requests", async () => {
598+
setLocalLoopbackGatewayConfig();
599+
600+
await callGateway({ method: "health", expectFinal: true });
601+
602+
expect(lastRequestOptions?.method).toBe("health");
603+
expect(lastRequestOptions?.opts?.expectFinal).toBe(true);
604+
expect(lastRequestOptions?.opts?.timeoutMs).toBeUndefined();
605+
});
606+
577607
it("fails fast when remote mode is missing remote url", async () => {
578608
loadConfig.mockReturnValue({
579609
gateway: { mode: "remote", bind: "loopback", remote: {} },

‎src/gateway/call.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,7 @@ async function executeGatewayRequestWithScopes<T>(params: {
848848
});
849849
const result = await client.request<T>(opts.method, opts.params, {
850850
expectFinal: opts.expectFinal,
851+
timeoutMs: opts.timeoutMs,
851852
});
852853
ignoreClose = true;
853854
stop(undefined, result);

‎src/gateway/client.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ type Pending = {
4444
resolve: (value: unknown) => void;
4545
reject: (err: unknown) => void;
4646
expectFinal: boolean;
47+
timeout: NodeJS.Timeout | null;
4748
};
4849

4950
type GatewayClientErrorShape = {
@@ -78,6 +79,7 @@ export type GatewayClientOptions = {
7879
url?: string; // ws://127.0.0.1:18789
7980
connectDelayMs?: number;
8081
tickWatchMinIntervalMs?: number;
82+
requestTimeoutMs?: number;
8183
token?: string;
8284
bootstrapToken?: string;
8385
deviceToken?: string;
@@ -136,6 +138,7 @@ export class GatewayClient {
136138
private lastTick: number | null = null;
137139
private tickIntervalMs = 30_000;
138140
private tickTimer: NodeJS.Timeout | null = null;
141+
private readonly requestTimeoutMs: number;
139142

140143
constructor(opts: GatewayClientOptions) {
141144
this.opts = {
@@ -145,6 +148,10 @@ export class GatewayClient {
145148
? undefined
146149
: (opts.deviceIdentity ?? loadOrCreateDeviceIdentity()),
147150
};
151+
this.requestTimeoutMs =
152+
typeof opts.requestTimeoutMs === "number" && Number.isFinite(opts.requestTimeoutMs)
153+
? Math.max(1, Math.min(Math.floor(opts.requestTimeoutMs), 2_147_483_647))
154+
: 30_000;
148155
}
149156

150157
start() {
@@ -586,6 +593,9 @@ export class GatewayClient {
586593
return;
587594
}
588595
this.pending.delete(parsed.id);
596+
if (pending.timeout) {
597+
clearTimeout(pending.timeout);
598+
}
589599
if (parsed.ok) {
590600
pending.resolve(parsed.payload);
591601
} else {
@@ -638,6 +648,9 @@ export class GatewayClient {
638648

639649
private flushPendingErrors(err: Error) {
640650
for (const [, p] of this.pending) {
651+
if (p.timeout) {
652+
clearTimeout(p.timeout);
653+
}
641654
p.reject(err);
642655
}
643656
this.pending.clear();
@@ -697,7 +710,7 @@ export class GatewayClient {
697710
async request<T = Record<string, unknown>>(
698711
method: string,
699712
params?: unknown,
700-
opts?: { expectFinal?: boolean },
713+
opts?: { expectFinal?: boolean; timeoutMs?: number | null },
701714
): Promise<T> {
702715
if (!this.ws || this.ws.readyState !== WebSocket.OPEN) {
703716
throw new Error("gateway not connected");
@@ -710,11 +723,27 @@ export class GatewayClient {
710723
);
711724
}
712725
const expectFinal = opts?.expectFinal === true;
726+
const timeoutMs =
727+
opts?.timeoutMs === null
728+
? null
729+
: typeof opts?.timeoutMs === "number" && Number.isFinite(opts.timeoutMs)
730+
? Math.max(1, Math.min(Math.floor(opts.timeoutMs), 2_147_483_647))
731+
: expectFinal
732+
? null
733+
: this.requestTimeoutMs;
713734
const p = new Promise<T>((resolve, reject) => {
735+
const timeout =
736+
timeoutMs === null
737+
? null
738+
: setTimeout(() => {
739+
this.pending.delete(id);
740+
reject(new Error(`gateway request timeout for ${method}`));
741+
}, timeoutMs);
714742
this.pending.set(id, {
715743
resolve: (value) => resolve(value as T),
716744
reject,
717745
expectFinal,
746+
timeout,
718747
});
719748
});
720749
this.ws.send(JSON.stringify(frame));

‎src/gateway/client.watchdog.test.ts‎

Lines changed: 156 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createServer as createHttpsServer } from "node:https";
22
import { createServer } from "node:net";
3-
import { afterEach, describe, expect, test } from "vitest";
4-
import { WebSocketServer } from "ws";
3+
import { afterEach, describe, expect, test, vi } from "vitest";
4+
import { WebSocket, WebSocketServer } from "ws";
55
import { rawDataToString } from "../infra/ws.js";
66
import { GatewayClient } from "./client.js";
77

@@ -85,6 +85,160 @@ describe("GatewayClient", () => {
8585
}
8686
}, 4000);
8787

88+
test("times out unresolved requests and clears pending state", async () => {
89+
vi.useFakeTimers();
90+
try {
91+
const client = new GatewayClient({
92+
requestTimeoutMs: 25,
93+
});
94+
const send = vi.fn();
95+
(
96+
client as unknown as {
97+
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
98+
}
99+
).ws = {
100+
readyState: WebSocket.OPEN,
101+
send,
102+
close: vi.fn(),
103+
};
104+
105+
const requestPromise = client.request("status");
106+
const requestExpectation = expect(requestPromise).rejects.toThrow(
107+
"gateway request timeout for status",
108+
);
109+
expect(send).toHaveBeenCalledTimes(1);
110+
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
111+
112+
await vi.advanceTimersByTimeAsync(25);
113+
114+
await requestExpectation;
115+
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(0);
116+
} finally {
117+
vi.useRealTimers();
118+
}
119+
});
120+
121+
test("does not auto-timeout expectFinal requests", async () => {
122+
vi.useFakeTimers();
123+
try {
124+
const client = new GatewayClient({
125+
requestTimeoutMs: 25,
126+
});
127+
const send = vi.fn();
128+
(
129+
client as unknown as {
130+
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
131+
}
132+
).ws = {
133+
readyState: WebSocket.OPEN,
134+
send,
135+
close: vi.fn(),
136+
};
137+
138+
let settled = false;
139+
const requestPromise = client.request("chat.send", undefined, { expectFinal: true });
140+
void requestPromise.then(
141+
() => {
142+
settled = true;
143+
},
144+
() => {
145+
settled = true;
146+
},
147+
);
148+
expect(send).toHaveBeenCalledTimes(1);
149+
150+
await vi.advanceTimersByTimeAsync(25);
151+
152+
expect(settled).toBe(false);
153+
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
154+
155+
client.stop();
156+
await expect(requestPromise).rejects.toThrow("gateway client stopped");
157+
} finally {
158+
vi.useRealTimers();
159+
}
160+
});
161+
162+
test("clamps oversized explicit request timeouts before scheduling", async () => {
163+
vi.useFakeTimers();
164+
try {
165+
const client = new GatewayClient({
166+
requestTimeoutMs: 25,
167+
});
168+
const send = vi.fn();
169+
(
170+
client as unknown as {
171+
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
172+
}
173+
).ws = {
174+
readyState: WebSocket.OPEN,
175+
send,
176+
close: vi.fn(),
177+
};
178+
179+
let settled = false;
180+
const requestPromise = client.request("status", undefined, { timeoutMs: 2_592_010_000 });
181+
void requestPromise.then(
182+
() => {
183+
settled = true;
184+
},
185+
() => {
186+
settled = true;
187+
},
188+
);
189+
190+
await vi.advanceTimersByTimeAsync(1);
191+
192+
expect(settled).toBe(false);
193+
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
194+
195+
client.stop();
196+
await expect(requestPromise).rejects.toThrow("gateway client stopped");
197+
} finally {
198+
vi.useRealTimers();
199+
}
200+
});
201+
202+
test("clamps oversized default request timeouts before scheduling", async () => {
203+
vi.useFakeTimers();
204+
try {
205+
const client = new GatewayClient({
206+
requestTimeoutMs: 2_592_010_000,
207+
});
208+
const send = vi.fn();
209+
(
210+
client as unknown as {
211+
ws: WebSocket | { readyState: number; send: () => void; close: () => void };
212+
}
213+
).ws = {
214+
readyState: WebSocket.OPEN,
215+
send,
216+
close: vi.fn(),
217+
};
218+
219+
let settled = false;
220+
const requestPromise = client.request("status");
221+
void requestPromise.then(
222+
() => {
223+
settled = true;
224+
},
225+
() => {
226+
settled = true;
227+
},
228+
);
229+
230+
await vi.advanceTimersByTimeAsync(1);
231+
232+
expect(settled).toBe(false);
233+
expect((client as unknown as { pending: Map<string, unknown> }).pending.size).toBe(1);
234+
235+
client.stop();
236+
await expect(requestPromise).rejects.toThrow("gateway client stopped");
237+
} finally {
238+
vi.useRealTimers();
239+
}
240+
});
241+
88242
test("rejects mismatched tls fingerprint", async () => {
89243
const key = [
90244
"-----BEGIN PRIVATE KEY-----", // pragma: allowlist secret

0 commit comments

Comments
 (0)