fix(browser): keep dispatcher context with no-retry hints

Landed from #39090 by @NewdlDewdl.

Co-authored-by: NewdlDewdl <rohin.agrawal@gmail.com>
This commit is contained in:
Peter Steinberger
2026-03-07 20:45:06 +00:00
parent f17f2f918c
commit 9b4a114eb6
3 changed files with 79 additions and 12 deletions

View File

@@ -273,6 +273,7 @@ Docs: https://docs.openclaw.ai
- Daemon/Windows schtasks runtime detection: use locale-invariant `Last Run Result` running codes (`0x41301`/`267009`) as the primary running signal so `openclaw node status` no longer misreports active tasks as stopped on non-English Windows locales. (#39076) Thanks @ademczuk. - Daemon/Windows schtasks runtime detection: use locale-invariant `Last Run Result` running codes (`0x41301`/`267009`) as the primary running signal so `openclaw node status` no longer misreports active tasks as stopped on non-English Windows locales. (#39076) Thanks @ademczuk.
- Usage/token count formatting: round near-million token counts to millions (`1.0m`) instead of `1000k`, with explicit boundary coverage for `999_499` and `999_500`. (#39129) Thanks @CurryMessi. - Usage/token count formatting: round near-million token counts to millions (`1.0m`) instead of `1000k`, with explicit boundary coverage for `999_499` and `999_500`. (#39129) Thanks @CurryMessi.
- Gateway/session bootstrap cache invalidation ordering: clear bootstrap snapshots only after active embedded-run shutdown wait completes, preventing dying runs from repopulating stale cache between `/new`/`sessions.reset` turns. (#38873) Thanks @MumuTW. - Gateway/session bootstrap cache invalidation ordering: clear bootstrap snapshots only after active embedded-run shutdown wait completes, preventing dying runs from repopulating stale cache between `/new`/`sessions.reset` turns. (#38873) Thanks @MumuTW.
- Browser/dispatcher error clarity: preserve dispatcher-side failure context in browser fetch errors while still appending operator guidance and explicit no-retry model hints, preventing misleading `"Can't reach service"` wrapping and avoiding LLM retry loops. (#39090) Thanks @NewdlDewdl.
## 2026.3.2 ## 2026.3.2

View File

@@ -8,6 +8,8 @@ const mocks = vi.hoisted(() => ({
}, },
}, },
})), })),
startBrowserControlServiceFromConfig: vi.fn(async () => ({ ok: true })),
dispatch: vi.fn(async () => ({ status: 200, body: { ok: true } })),
})); }));
vi.mock("../config/config.js", async (importOriginal) => { vi.mock("../config/config.js", async (importOriginal) => {
@@ -20,12 +22,12 @@ vi.mock("../config/config.js", async (importOriginal) => {
vi.mock("./control-service.js", () => ({ vi.mock("./control-service.js", () => ({
createBrowserControlContext: vi.fn(() => ({})), createBrowserControlContext: vi.fn(() => ({})),
startBrowserControlServiceFromConfig: vi.fn(async () => ({ ok: true })), startBrowserControlServiceFromConfig: mocks.startBrowserControlServiceFromConfig,
})); }));
vi.mock("./routes/dispatcher.js", () => ({ vi.mock("./routes/dispatcher.js", () => ({
createBrowserRouteDispatcher: vi.fn(() => ({ createBrowserRouteDispatcher: vi.fn(() => ({
dispatch: vi.fn(async () => ({ status: 200, body: { ok: true } })), dispatch: mocks.dispatch,
})), })),
})); }));
@@ -54,6 +56,8 @@ describe("fetchBrowserJson loopback auth", () => {
}, },
}, },
}); });
mocks.startBrowserControlServiceFromConfig.mockReset().mockResolvedValue({ ok: true });
mocks.dispatch.mockReset().mockResolvedValue({ status: 200, body: { ok: true } });
}); });
afterEach(() => { afterEach(() => {
@@ -114,4 +118,32 @@ describe("fetchBrowserJson loopback auth", () => {
const headers = new Headers(init?.headers); const headers = new Headers(init?.headers);
expect(headers.get("authorization")).toBe("Bearer loopback-token"); expect(headers.get("authorization")).toBe("Bearer loopback-token");
}); });
it("preserves dispatcher error context while keeping no-retry hint", async () => {
mocks.dispatch.mockRejectedValueOnce(new Error("Chrome CDP handshake timeout"));
const thrown = await fetchBrowserJson<{ ok: boolean }>("/tabs").catch((err) => err as Error);
expect(thrown).toBeInstanceOf(Error);
expect(thrown.message).toContain("Chrome CDP handshake timeout");
expect(thrown.message).toContain("Do NOT retry the browser tool");
expect(thrown.message).not.toContain("Can't reach the OpenClaw browser control service");
});
it("keeps absolute URL failures wrapped as reachability errors", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () => {
throw new Error("socket hang up");
}),
);
const thrown = await fetchBrowserJson<{ ok: boolean }>("http://example.com/").catch(
(err) => err as Error,
);
expect(thrown).toBeInstanceOf(Error);
expect(thrown.message).toContain("Can't reach the OpenClaw browser control service");
expect(thrown.message).toContain("Do NOT retry the browser tool");
});
}); });

View File

@@ -98,17 +98,40 @@ function withLoopbackBrowserAuth(
}); });
} }
function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number): Error { const BROWSER_TOOL_MODEL_HINT =
"Do NOT retry the browser tool — it will keep failing. " +
"Use an alternative approach or inform the user that the browser is currently unavailable.";
function resolveBrowserFetchOperatorHint(url: string): string {
const isLocal = !isAbsoluteHttp(url); const isLocal = !isAbsoluteHttp(url);
// Human-facing hint for logs/diagnostics. return isLocal
const operatorHint = isLocal
? `Restart the OpenClaw gateway (OpenClaw.app menubar, or \`${formatCliCommand("openclaw gateway")}\`).` ? `Restart the OpenClaw gateway (OpenClaw.app menubar, or \`${formatCliCommand("openclaw gateway")}\`).`
: "If this is a sandboxed session, ensure the sandbox browser is running."; : "If this is a sandboxed session, ensure the sandbox browser is running.";
// Model-facing suffix: explicitly tell the LLM NOT to retry. }
// Without this, models see "try again" and enter an infinite tool-call loop.
const modelHint = function normalizeErrorMessage(err: unknown): string {
"Do NOT retry the browser tool — it will keep failing. " + if (err instanceof Error && err.message.trim().length > 0) {
"Use an alternative approach or inform the user that the browser is currently unavailable."; return err.message.trim();
}
return String(err);
}
function appendBrowserToolModelHint(message: string): string {
if (message.includes(BROWSER_TOOL_MODEL_HINT)) {
return message;
}
return `${message} ${BROWSER_TOOL_MODEL_HINT}`;
}
function enhanceDispatcherPathError(url: string, err: unknown): Error {
const msg = normalizeErrorMessage(err);
const suffix = `${resolveBrowserFetchOperatorHint(url)} ${BROWSER_TOOL_MODEL_HINT}`;
const normalized = msg.endsWith(".") ? msg : `${msg}.`;
return new Error(`${normalized} ${suffix}`, err instanceof Error ? { cause: err } : undefined);
}
function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number): Error {
const operatorHint = resolveBrowserFetchOperatorHint(url);
const msg = String(err); const msg = String(err);
const msgLower = msg.toLowerCase(); const msgLower = msg.toLowerCase();
const looksLikeTimeout = const looksLikeTimeout =
@@ -119,11 +142,15 @@ function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number):
msgLower.includes("aborterror"); msgLower.includes("aborterror");
if (looksLikeTimeout) { if (looksLikeTimeout) {
return new Error( return new Error(
`Can't reach the OpenClaw browser control service (timed out after ${timeoutMs}ms). ${operatorHint} ${modelHint}`, appendBrowserToolModelHint(
`Can't reach the OpenClaw browser control service (timed out after ${timeoutMs}ms). ${operatorHint}`,
),
); );
} }
return new Error( return new Error(
`Can't reach the OpenClaw browser control service. ${operatorHint} ${modelHint} (${msg})`, appendBrowserToolModelHint(
`Can't reach the OpenClaw browser control service. ${operatorHint} (${msg})`,
),
); );
} }
@@ -165,11 +192,13 @@ export async function fetchBrowserJson<T>(
init?: RequestInit & { timeoutMs?: number }, init?: RequestInit & { timeoutMs?: number },
): Promise<T> { ): Promise<T> {
const timeoutMs = init?.timeoutMs ?? 5000; const timeoutMs = init?.timeoutMs ?? 5000;
let isDispatcherPath = false;
try { try {
if (isAbsoluteHttp(url)) { if (isAbsoluteHttp(url)) {
const httpInit = withLoopbackBrowserAuth(url, init); const httpInit = withLoopbackBrowserAuth(url, init);
return await fetchHttpJson<T>(url, { ...httpInit, timeoutMs }); return await fetchHttpJson<T>(url, { ...httpInit, timeoutMs });
} }
isDispatcherPath = true;
const started = await startBrowserControlServiceFromConfig(); const started = await startBrowserControlServiceFromConfig();
if (!started) { if (!started) {
throw new Error("browser control disabled"); throw new Error("browser control disabled");
@@ -251,6 +280,11 @@ export async function fetchBrowserJson<T>(
if (err instanceof BrowserServiceError) { if (err instanceof BrowserServiceError) {
throw err; throw err;
} }
// Dispatcher-path failures are service-operation failures, not network
// reachability failures. Keep the original context, but retain anti-retry hints.
if (isDispatcherPath) {
throw enhanceDispatcherPathError(url, err);
}
throw enhanceBrowserFetchError(url, err, timeoutMs); throw enhanceBrowserFetchError(url, err, timeoutMs);
} }
} }