mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:30:43 +00:00
fix(browser): time out stuck chrome mcp clicks
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
committed by
Peter Steinberger
parent
c97c5a5aff
commit
adc05f090a
@@ -1,5 +1,6 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
clickChromeMcpElement,
|
||||
buildChromeMcpArgs,
|
||||
evaluateChromeMcpScript,
|
||||
listChromeMcpTabs,
|
||||
@@ -305,6 +306,41 @@ describe("chrome MCP page parsing", () => {
|
||||
expect(tabs).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("times out a stuck click and recovers on the next call", async () => {
|
||||
let factoryCalls = 0;
|
||||
const factory: ChromeMcpSessionFactory = async () => {
|
||||
factoryCalls += 1;
|
||||
const session = createFakeSession();
|
||||
const callTool = vi.fn(async ({ name }: ToolCall) => {
|
||||
if (name === "click") {
|
||||
return await new Promise(() => {});
|
||||
}
|
||||
if (name === "list_pages") {
|
||||
return {
|
||||
content: [{ type: "text", text: "## Pages\n1: https://example.com [selected]" }],
|
||||
};
|
||||
}
|
||||
throw new Error(`unexpected tool ${name}`);
|
||||
});
|
||||
session.client.callTool = callTool as typeof session.client.callTool;
|
||||
return session;
|
||||
};
|
||||
setChromeMcpSessionFactoryForTest(factory);
|
||||
|
||||
await expect(
|
||||
clickChromeMcpElement({
|
||||
profileName: "chrome-live",
|
||||
targetId: "1",
|
||||
uid: "btn-1",
|
||||
timeoutMs: 25,
|
||||
}),
|
||||
).rejects.toThrow(/timed out/i);
|
||||
|
||||
const tabs = await listChromeMcpTabs("chrome-live");
|
||||
expect(factoryCalls).toBe(2);
|
||||
expect(tabs).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("creates a fresh session when userDataDir changes for the same profile", async () => {
|
||||
const createdSessions: ChromeMcpSession[] = [];
|
||||
const closeMocks: Array<ReturnType<typeof vi.fn>> = [];
|
||||
|
||||
@@ -310,10 +310,15 @@ async function callTool(
|
||||
userDataDir: string | undefined,
|
||||
name: string,
|
||||
args: Record<string, unknown> = {},
|
||||
timeoutMs?: number,
|
||||
opts?: { timeoutMs?: number; signal?: AbortSignal },
|
||||
): Promise<ChromeMcpToolResult> {
|
||||
const cacheKey = buildChromeMcpSessionCacheKey(profileName, userDataDir);
|
||||
const session = await getSession(profileName, userDataDir);
|
||||
const timeoutMs = opts?.timeoutMs;
|
||||
const signal = opts?.signal;
|
||||
if (signal?.aborted) {
|
||||
throw signal.reason ?? new Error("aborted");
|
||||
}
|
||||
|
||||
const rawCall = session.client.callTool({
|
||||
name,
|
||||
@@ -321,35 +326,39 @@ async function callTool(
|
||||
}) as Promise<ChromeMcpToolResult>;
|
||||
|
||||
let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
|
||||
const callPromise: Promise<ChromeMcpToolResult> =
|
||||
timeoutMs !== undefined && timeoutMs > 0
|
||||
? Promise.race([
|
||||
rawCall,
|
||||
new Promise<never>((_, reject) => {
|
||||
timeoutHandle = setTimeout(() => {
|
||||
// Use transport-identity check so we never delete a freshly-created replacement session.
|
||||
const cur = sessions.get(cacheKey);
|
||||
if (cur?.transport === session.transport) {
|
||||
sessions.delete(cacheKey);
|
||||
}
|
||||
void session.client.close().catch(() => {});
|
||||
reject(
|
||||
new Error(
|
||||
`Chrome MCP "${name}" timed out after ${timeoutMs}ms. Session reset for reconnect.`,
|
||||
),
|
||||
);
|
||||
}, timeoutMs);
|
||||
}),
|
||||
])
|
||||
: rawCall;
|
||||
let abortListener: (() => void) | undefined;
|
||||
const racers: Array<Promise<ChromeMcpToolResult> | Promise<never>> = [rawCall];
|
||||
|
||||
if (timeoutMs !== undefined && timeoutMs > 0) {
|
||||
racers.push(
|
||||
new Promise<never>((_, reject) => {
|
||||
timeoutHandle = setTimeout(() => {
|
||||
reject(
|
||||
new Error(
|
||||
`Chrome MCP "${name}" timed out after ${timeoutMs}ms. Session reset for reconnect.`,
|
||||
),
|
||||
);
|
||||
}, timeoutMs);
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
if (signal) {
|
||||
racers.push(
|
||||
new Promise<never>((_, reject) => {
|
||||
abortListener = () => reject(signal.reason ?? new Error("aborted"));
|
||||
signal.addEventListener("abort", abortListener, { once: true });
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
let result: ChromeMcpToolResult;
|
||||
try {
|
||||
result = await callPromise;
|
||||
result = racers.length === 1 ? await rawCall : await Promise.race(racers);
|
||||
} catch (err) {
|
||||
// Transport/connection error or safety-net timeout — tear down session so it reconnects.
|
||||
void rawCall.catch(() => {});
|
||||
// Transport/connection error, timeout, or abort: tear down session so it reconnects.
|
||||
// Transport-identity check prevents clobbering a replacement session created concurrently.
|
||||
// Only close the client here if the timeout callback hasn't already done so.
|
||||
const cur = sessions.get(cacheKey);
|
||||
if (cur?.transport === session.transport) {
|
||||
sessions.delete(cacheKey);
|
||||
@@ -360,6 +369,9 @@ async function callTool(
|
||||
if (timeoutHandle !== undefined) {
|
||||
clearTimeout(timeoutHandle);
|
||||
}
|
||||
if (signal && abortListener) {
|
||||
signal.removeEventListener("abort", abortListener);
|
||||
}
|
||||
}
|
||||
// Tool-level errors (element not found, script error, etc.) don't indicate a
|
||||
// broken connection — don't tear down the session for these.
|
||||
@@ -507,7 +519,7 @@ export async function navigateChromeMcpPage(params: {
|
||||
url: params.url,
|
||||
timeout: resolvedTimeoutMs,
|
||||
},
|
||||
resolvedTimeoutMs + 5_000,
|
||||
{ timeoutMs: resolvedTimeoutMs + 5_000 },
|
||||
);
|
||||
const page = await findPageById(
|
||||
params.profileName,
|
||||
@@ -554,12 +566,23 @@ export async function clickChromeMcpElement(params: {
|
||||
targetId: string;
|
||||
uid: string;
|
||||
doubleClick?: boolean;
|
||||
timeoutMs?: number;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<void> {
|
||||
await callTool(params.profileName, params.userDataDir, "click", {
|
||||
pageId: parsePageId(params.targetId),
|
||||
uid: params.uid,
|
||||
...(params.doubleClick ? { dblClick: true } : {}),
|
||||
});
|
||||
await callTool(
|
||||
params.profileName,
|
||||
params.userDataDir,
|
||||
"click",
|
||||
{
|
||||
pageId: parsePageId(params.targetId),
|
||||
uid: params.uid,
|
||||
...(params.doubleClick ? { dblClick: true } : {}),
|
||||
},
|
||||
{
|
||||
timeoutMs: params.timeoutMs,
|
||||
signal: params.signal,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
export async function fillChromeMcpElement(params: {
|
||||
|
||||
Reference in New Issue
Block a user