Skip to content

Commit 3901094

Browse files
steipetesteipete
authored
fix(browser): cancel Chrome MCP requests on crash (#101454)
* fix(browser): cancel Chrome MCP requests on crash * test(browser): cover request budget options
1 parent 988f30e commit 3901094

14 files changed

Lines changed: 491 additions & 396 deletions

‎extensions/browser/src/browser/chrome-mcp.test.ts‎

Lines changed: 87 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import fs from "node:fs/promises";
33
import os from "node:os";
44
import path from "node:path";
5+
import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js";
56
import { MAX_TIMER_TIMEOUT_MS } from "openclaw/plugin-sdk/number-runtime";
67
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
78
import {
@@ -27,10 +28,22 @@ type ToolCall = {
2728
};
2829
type ToolCallMock = {
2930
mock: {
30-
calls: Array<[ToolCall]>;
31+
calls: Array<[ToolCall, unknown?, { signal?: AbortSignal; timeout?: number }?]>;
3132
};
3233
};
3334

35+
function createSdkTimeoutCallTool() {
36+
return vi.fn(
37+
async (_call: ToolCall, _resultSchema?: unknown, options?: { timeout?: number }) =>
38+
await new Promise<never>((_resolve, reject) => {
39+
setTimeout(
40+
() => reject(new McpError(ErrorCode.RequestTimeout, "Request timed out")),
41+
options?.timeout,
42+
);
43+
}),
44+
);
45+
}
46+
3447
type ChromeMcpSessionFactory = Exclude<
3548
Parameters<typeof setChromeMcpSessionFactoryForTest>[0],
3649
null
@@ -1195,20 +1208,29 @@ describe("chrome MCP page parsing", () => {
11951208

11961209
it("times out a stuck click and recovers on the next call", async () => {
11971210
let factoryCalls = 0;
1211+
let forwardedTimeout: number | undefined;
11981212
const factory: ChromeMcpSessionFactory = async () => {
11991213
factoryCalls += 1;
12001214
const session = createFakeSession();
1201-
const callTool = vi.fn(async ({ name }: ToolCall) => {
1202-
if (name === "click") {
1203-
return await new Promise(() => {});
1204-
}
1205-
if (name === "list_pages") {
1206-
return {
1207-
content: [{ type: "text", text: "## Pages\n1: https://example.com [selected]" }],
1208-
};
1209-
}
1210-
throw new Error(`unexpected tool ${name}`);
1211-
});
1215+
const callTool = vi.fn(
1216+
async ({ name }: ToolCall, _resultSchema?: unknown, options?: { timeout?: number }) => {
1217+
if (name === "click") {
1218+
forwardedTimeout = options?.timeout;
1219+
return await new Promise((_, reject) => {
1220+
setTimeout(
1221+
() => reject(new McpError(ErrorCode.RequestTimeout, "Request timed out")),
1222+
options?.timeout,
1223+
);
1224+
});
1225+
}
1226+
if (name === "list_pages") {
1227+
return {
1228+
content: [{ type: "text", text: "## Pages\n1: https://example.com [selected]" }],
1229+
};
1230+
}
1231+
throw new Error(`unexpected tool ${name}`);
1232+
},
1233+
);
12121234
session.client.callTool = callTool as typeof session.client.callTool;
12131235
return session;
12141236
};
@@ -1223,11 +1245,61 @@ describe("chrome MCP page parsing", () => {
12231245
}),
12241246
).rejects.toThrow(/timed out/i);
12251247

1248+
expect(forwardedTimeout).toBe(25);
12261249
const tabs = await listChromeMcpTabs("chrome-live");
12271250
expect(factoryCalls).toBe(2);
12281251
expect(tabs).toHaveLength(1);
12291252
});
12301253

1254+
it("cancels a stuck evaluate through the SDK signal and reconnects", async () => {
1255+
let factoryCalls = 0;
1256+
let forwardedSignal: AbortSignal | undefined;
1257+
let notifyToolStarted: (() => void) | undefined;
1258+
const toolStarted = new Promise<void>((resolve) => {
1259+
notifyToolStarted = resolve;
1260+
});
1261+
const factory: ChromeMcpSessionFactory = async () => {
1262+
factoryCalls += 1;
1263+
const session = createFakeSession();
1264+
if (factoryCalls === 1) {
1265+
session.client.callTool = vi.fn(
1266+
async (_call: ToolCall, _resultSchema?: unknown, options?: { signal?: AbortSignal }) =>
1267+
await new Promise((_resolve, reject) => {
1268+
const signal = options?.signal;
1269+
forwardedSignal = signal;
1270+
notifyToolStarted?.();
1271+
signal?.addEventListener(
1272+
"abort",
1273+
() => {
1274+
reject(signal.reason instanceof Error ? signal.reason : new Error("aborted"));
1275+
},
1276+
{
1277+
once: true,
1278+
},
1279+
);
1280+
}),
1281+
) as typeof session.client.callTool;
1282+
}
1283+
return session;
1284+
};
1285+
setChromeMcpSessionFactoryForTest(factory);
1286+
const ctrl = new AbortController();
1287+
const evaluatePromise = evaluateChromeMcpScript({
1288+
profileName: "chrome-live",
1289+
targetId: "1",
1290+
fn: "() => window.location.href",
1291+
signal: ctrl.signal,
1292+
});
1293+
1294+
await toolStarted;
1295+
expect(forwardedSignal).toBe(ctrl.signal);
1296+
ctrl.abort(new Error("target browser crashed"));
1297+
1298+
await expect(evaluatePromise).rejects.toThrow(/target browser crashed/i);
1299+
await expect(listChromeMcpTabs("chrome-live")).resolves.toHaveLength(2);
1300+
expect(factoryCalls).toBe(2);
1301+
});
1302+
12311303
it("does not dispatch a click when the signal is already aborted", async () => {
12321304
const session = createFakeSession();
12331305
const callTool = vi.fn(async (_call: ToolCall) => {
@@ -1403,11 +1475,7 @@ describe("chrome MCP page parsing", () => {
14031475
factoryCalls += 1;
14041476
const session = createFakeSession();
14051477
if (factoryCalls === 1) {
1406-
// First session: all tool calls hang — simulates a Chrome MCP subprocess that is
1407-
// completely blocked (e.g., stuck waiting for a slow navigation to complete).
1408-
session.client.callTool = vi.fn(
1409-
async () => new Promise<never>(() => {}),
1410-
) as typeof session.client.callTool;
1478+
session.client.callTool = createSdkTimeoutCallTool() as typeof session.client.callTool;
14111479
}
14121480
return session;
14131481
};
@@ -1437,12 +1505,10 @@ describe("chrome MCP page parsing", () => {
14371505
expect(tabs).toHaveLength(2);
14381506
});
14391507

1440-
it("forwards an explicit timeoutMs to take_snapshot via the callTool race", async () => {
1508+
it("forwards an explicit timeoutMs to take_snapshot through the SDK", async () => {
14411509
vi.useFakeTimers();
14421510
const session = createFakeSession();
1443-
session.client.callTool = vi.fn(
1444-
async () => new Promise<never>(() => {}),
1445-
) as typeof session.client.callTool;
1511+
session.client.callTool = createSdkTimeoutCallTool() as typeof session.client.callTool;
14461512
setChromeMcpSessionFactoryForTest(async () => session);
14471513

14481514
const snapshotPromise = takeChromeMcpSnapshot({

0 commit comments

Comments
 (0)