ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-27 04:47:03 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(browser): allow one retry after transient failures (#110607)

Browse Source 
* fix(browser): soften transient retry guidance

* style(browser): format retry hint test

* chore: keep release note in PR body
This commit is contained in:
Peter Steinberger
2026-07-18 10:58:22 +01:00
committed by GitHub
parent a068d38b66
commit e7fe47d63f
3 changed files with 334 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/browser/src/browser/client-fetch.attach-only.e2e.test.ts
+2
View File
@@ -75,6 +75,8 @@ describe("browser client fetch attachOnly diagnostics", () => {
const message = thrown instanceof Error ? thrown.message : String(thrown);
expect(message).toContain("browser profile is external to OpenClaw");
expect(message).toContain("Restarting the OpenClaw gateway will not launch it");
expect(message).toContain("Retry the browser tool once");
expect(message).toContain("If the same error persists");
expect(message).not.toContain("Restart the OpenClaw gateway");
expect(message).not.toContain("Do NOT retry the browser tool");
} finally {
extensions/browser/src/browser/client-fetch.loopback-auth.test.ts
+241 -12
View File
@@ -226,11 +226,16 @@ describe("fetchBrowserJson loopback auth", () => {
expect(headers.get("authorization")).toBeNull();
});
it("preserves dispatcher timeout context without no-retry hint", async () => {
it("preserves dispatcher timeout context with retry-once hint", async () => {
mocks.dispatch.mockRejectedValueOnce(new Error("Chrome CDP handshake timeout"));
await expectThrownBrowserFetchError(() => fetchBrowserJson<{ ok: boolean }>("/tabs"), {
contains: ["Chrome CDP handshake timeout", "Restart the OpenClaw gateway"],
contains: [
"Chrome CDP handshake timeout",
"Restart the OpenClaw gateway",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Can't reach the OpenClaw browser control service", "Do NOT retry the browser tool"],
});
});
@@ -267,6 +272,8 @@ describe("fetchBrowserJson loopback auth", () => {
"Chrome CDP handshake timeout",
"browser profile is external to OpenClaw",
"Restarting the OpenClaw gateway will not launch it",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Restart the OpenClaw gateway", "Do NOT retry the browser tool"],
},
@@ -319,6 +326,8 @@ describe("fetchBrowserJson loopback auth", () => {
"timed out",
"browser profile is external to OpenClaw",
"Restarting the OpenClaw gateway will not launch it",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Restart the OpenClaw gateway", "Do NOT retry the browser tool"],
},
@@ -342,7 +351,12 @@ describe("fetchBrowserJson loopback auth", () => {
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("/tabs?profile=openclaw"),
{
contains: ["Chrome CDP handshake timeout", "Restart the OpenClaw gateway"],
contains: [
"Chrome CDP handshake timeout",
"Restart the OpenClaw gateway",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["browser profile is external to OpenClaw", "Do NOT retry the browser tool"],
},
);
@@ -357,7 +371,12 @@ describe("fetchBrowserJson loopback auth", () => {
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("/tabs?profile=manual"),
{
contains: ["Chrome CDP handshake timeout", "Restart the OpenClaw gateway"],
contains: [
"Chrome CDP handshake timeout",
"Restart the OpenClaw gateway",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["browser profile is external to OpenClaw", "Do NOT retry the browser tool"],
},
);
@@ -380,7 +399,12 @@ describe("fetchBrowserJson loopback auth", () => {
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("/tabs?profile=missing"),
{
contains: ["Chrome CDP handshake timeout", "Restart the OpenClaw gateway"],
contains: [
"Chrome CDP handshake timeout",
"Restart the OpenClaw gateway",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["browser profile is external to OpenClaw", "Do NOT retry the browser tool"],
},
);
@@ -406,6 +430,8 @@ describe("fetchBrowserJson loopback auth", () => {
"Chrome CDP handshake timeout",
"browser profile is external to OpenClaw",
"Restarting the OpenClaw gateway will not launch it",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Restart the OpenClaw gateway", "Do NOT retry the browser tool"],
});
@@ -449,6 +475,51 @@ describe("fetchBrowserJson loopback auth", () => {
});
});
it("keeps transient dispatcher connection resets retryable once", async () => {
mocks.dispatch.mockRejectedValueOnce(new Error("Chrome CDP connection reset"));
await expectThrownBrowserFetchError(() => fetchBrowserJson<{ ok: boolean }>("/tabs"), {
contains: [
"Chrome CDP connection reset",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Do NOT retry the browser tool"],
});
});
it("uses top-level reset codes to classify dispatcher failures as transient", async () => {
mocks.dispatch.mockRejectedValueOnce(
Object.assign(new Error("socket closed"), { code: "ECONNRESET" }),
);
await expectThrownBrowserFetchError(() => fetchBrowserJson<{ ok: boolean }>("/tabs"), {
contains: ["socket closed", "Retry the browser tool once", "If the same error persists"],
omits: ["Do NOT retry the browser tool"],
});
});
it("keeps refusal causes non-retryable when the outer error mentions a timeout", async () => {
const refused = Object.assign(new Error("connect refused"), { code: "ECONNREFUSED" });
mocks.dispatch.mockRejectedValueOnce(
new Error("browser request timed out", { cause: refused }),
);
await expectThrownBrowserFetchError(() => fetchBrowserJson<{ ok: boolean }>("/tabs"), {
contains: ["browser request timed out", "Do NOT retry the browser tool"],
omits: ["Retry the browser tool once"],
});
});
it("keeps disabled browser control failures non-retryable", async () => {
mocks.dispatch.mockRejectedValueOnce(new Error("browser control disabled"));
await expectThrownBrowserFetchError(() => fetchBrowserJson<{ ok: boolean }>("/tabs"), {
contains: ["browser control disabled", "Do NOT retry the browser tool"],
omits: ["Retry the browser tool once"],
});
});
it("preserves validated structured errors from dispatcher routes", async () => {
mocks.dispatch.mockResolvedValueOnce({
status: 409,
@@ -540,7 +611,115 @@ describe("fetchBrowserJson loopback auth", () => {
() => fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/"),
{
contains: ["internal error"],
omits: ["rate limit"],
omits: ["rate limit", "Retry the browser tool once", "Do NOT retry the browser tool"],
},
);
});
it("keeps transient HTTP error payloads retryable once", async () => {
vi.stubGlobal(
"fetch",
vi.fn(
async () =>
new Response(JSON.stringify({ error: "Chrome CDP handshake timeout" }), {
status: 504,
}),
),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/"),
{
contains: [
"Chrome CDP handshake timeout",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Do NOT retry the browser tool"],
},
);
});
it.each([408, 504])("uses HTTP %i to classify generic payloads as transient", async (status) => {
vi.stubGlobal(
"fetch",
vi.fn(async () => new Response("request failed", { status })),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/"),
{
contains: ["request failed", "Retry the browser tool once", "If the same error persists"],
omits: ["Do NOT retry the browser tool"],
},
);
});
it("does not mark client validation errors transient from timeout wording alone", async () => {
vi.stubGlobal(
"fetch",
vi.fn(
async () =>
new Response(JSON.stringify({ error: "invalid timeout value" }), {
status: 400,
}),
),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/"),
{
contains: ["invalid timeout value"],
omits: ["Retry the browser tool once", "Do NOT retry the browser tool"],
},
);
});
it("keeps pre-annotated persistent payload hints mutually exclusive", async () => {
const persistentHint =
"Do NOT retry the browser tool — it will keep failing. Use an alternative approach or inform the user that the browser is currently unavailable.";
vi.stubGlobal(
"fetch",
vi.fn(
async () =>
new Response(JSON.stringify({ error: `browser request timed out. ${persistentHint}` }), {
status: 504,
}),
),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/"),
{
contains: ["browser request timed out", persistentHint],
omits: ["Retry the browser tool once"],
},
);
});
it("keeps transient dispatcher error payloads retryable once", async () => {
mocks.dispatch.mockResolvedValueOnce({
status: 500,
body: { error: "read ECONNRESET" },
});
await expectThrownBrowserFetchError(() => fetchBrowserJson<{ ok: boolean }>("/tabs"), {
contains: ["read ECONNRESET", "Retry the browser tool once", "If the same error persists"],
omits: ["Do NOT retry the browser tool"],
});
});
it("keeps authentication failures non-retryable", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () => new Response("Unauthorized", { status: 401 })),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/"),
{
contains: ["Unauthorized", "Do NOT retry the browser tool"],
omits: ["Retry the browser tool once"],
},
);
});
@@ -557,7 +736,7 @@ describe("fetchBrowserJson loopback auth", () => {
});
});
it("keeps absolute URL failures wrapped as reachability errors", async () => {
it("keeps transient absolute URL failures retryable once", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () => {
@@ -570,13 +749,51 @@ describe("fetchBrowserJson loopback auth", () => {
{
contains: [
"Can't reach the OpenClaw browser control service",
"Do NOT retry the browser tool",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Do NOT retry the browser tool"],
},
);
});
it("omits no-retry hint for absolute HTTP timeout failures", async () => {
it("uses nested reset causes to classify generic fetch failures as transient", async () => {
const reset = Object.assign(new Error("socket closed"), { code: "ECONNRESET" });
vi.stubGlobal(
"fetch",
vi.fn(async () => {
throw new TypeError("fetch failed", { cause: reset });
}),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://example.com/"),
{
contains: ["fetch failed", "Retry the browser tool once", "If the same error persists"],
omits: ["Do NOT retry the browser tool"],
},
);
});
it("uses nested refusal causes to keep unavailable services non-retryable", async () => {
const refused = Object.assign(new Error("connect refused"), { code: "ECONNREFUSED" });
vi.stubGlobal(
"fetch",
vi.fn(async () => {
throw new TypeError("fetch failed", { cause: refused });
}),
);
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://example.com/"),
{
contains: ["fetch failed", "Do NOT retry the browser tool"],
omits: ["Retry the browser tool once"],
},
);
});
it("uses retry-once hint for absolute HTTP timeout failures", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () => {
@@ -587,7 +804,11 @@ describe("fetchBrowserJson loopback auth", () => {
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://example.com/", { timeoutMs: 1234 }),
{
contains: ["timed out after 1234ms"],
contains: [
"timed out after 1234ms",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Do NOT retry the browser tool"],
},
);
@@ -604,7 +825,11 @@ describe("fetchBrowserJson loopback auth", () => {
await expectThrownBrowserFetchError(
() => fetchBrowserJson<{ ok: boolean }>("http://example.com/", { timeoutMs: Number.NaN }),
{
contains: ["timed out after 5000ms"],
contains: [
"timed out after 5000ms",
"Retry the browser tool once",
"If the same error persists",
],
omits: ["NaNms", "Do NOT retry the browser tool"],
},
);
@@ -628,7 +853,11 @@ describe("fetchBrowserJson loopback auth", () => {
timeoutMs: Number.MAX_SAFE_INTEGER,
}),
{
contains: [`timed out after ${MAX_TIMER_TIMEOUT_MS}ms`],
contains: [
`timed out after ${MAX_TIMER_TIMEOUT_MS}ms`,
"Retry the browser tool once",
"If the same error persists",
],
omits: ["Do NOT retry the browser tool"],
},
);
extensions/browser/src/browser/client-fetch.ts
+91 -22
View File
@@ -5,6 +5,7 @@
* in-process dispatcher, adding loopback auth and operator-facing diagnostics.
*/
import { parseBrowserHttpUrl } from "openclaw/plugin-sdk/browser-config";
import { extractErrorCode, formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
import { resolveTimerTimeoutMs } from "openclaw/plugin-sdk/number-runtime";
import { readResponseWithLimit } from "openclaw/plugin-sdk/response-limit-runtime";
import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
@@ -45,8 +46,10 @@ function browserServiceErrorFromPayload(
status?: number,
): BrowserServiceError {
const parsed = parseBrowserErrorPayload(value);
const message = parsed?.error ?? fallback;
const modelHint = resolveBrowserServiceModelHint(message, status);
return new BrowserServiceError(
parsed?.error ?? fallback,
modelHint ? appendBrowserToolModelHint(message, modelHint) : message,
parsed && "reason" in parsed ? parsed : undefined,
status,
);
@@ -126,9 +129,19 @@ function withLoopbackBrowserAuth(
});
}
const BROWSER_TOOL_MODEL_HINT =
const BROWSER_TOOL_PERSISTENT_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.";
const BROWSER_TOOL_TRANSIENT_MODEL_HINT =
"This may be a transient browser error. Retry the browser tool once. " +
"If the same error persists, use an alternative approach or inform the user that the browser is currently unavailable.";
// Retry history already lives in the model transcript. Keep this classifier stateless so one
// session's transient failure cannot suppress browser retries in another session.
const BROWSER_TRANSIENT_NETWORK_ERROR_RE =
/\b(?:ECONNRESET|ECONNABORTED|ENETRESET|ETIMEDOUT|EPIPE|EHOSTUNREACH|ENETUNREACH|EAI_AGAIN|UND_ERR_(?:CONNECT_TIMEOUT|HEADERS_TIMEOUT|BODY_TIMEOUT|SOCKET))\b|fetch failed|network error|other side closed|socket (?:hang up|terminated)|connection (?:reset|aborted|timed out)/i;
const BROWSER_PERSISTENT_FAILURE_RE =
/\bECONNREFUSED\b|connection refused|browser control (?:is )?(?:disabled|not enabled)|invalid (?:auth|authentication|credentials|password|token)|authentication (?:failed|required)|unauthorized/i;
const BROWSER_ERROR_BODY_LIMIT_BYTES = 16 * 1024;
// `response/body` supports 5M characters; 32 MiB covers worst-case JSON escaping while staying bounded.
@@ -185,38 +198,87 @@ function normalizeErrorMessage(err: unknown): string {
return String(err);
}
function appendBrowserToolModelHint(message: string): string {
if (message.includes(BROWSER_TOOL_MODEL_HINT)) {
return message;
}
return `${message} ${BROWSER_TOOL_MODEL_HINT}`;
function appendBrowserToolModelHint(message: string, hint: string): string {
const messageWithoutHints = message
.replaceAll(BROWSER_TOOL_PERSISTENT_MODEL_HINT, "")
.replaceAll(BROWSER_TOOL_TRANSIENT_MODEL_HINT, "")
.trim();
return `${messageWithoutHints} ${hint}`;
}
type BrowserFetchFailureKind = "timeout" | "aborted" | "persistent";
type BrowserFetchFailureKind = "timeout" | "aborted" | "transient-network" | "persistent";
function resolveBrowserFetchTimeoutMs(timeoutMs: number | undefined): number {
return resolveTimerTimeoutMs(timeoutMs, 5000);
}
function classifyBrowserFetchFailure(err: unknown): BrowserFetchFailureKind {
const msg = normalizeErrorMessage(err);
const msgLower = normalizeLowercaseStringOrEmpty(msg);
const directCode = extractErrorCode(err);
const formatted = formatErrorMessage(err);
const detail = directCode ? `${formatted} | ${directCode}` : formatted;
const detailLower = normalizeLowercaseStringOrEmpty(detail);
const nameLower = err instanceof Error ? normalizeLowercaseStringOrEmpty(err.name) : "";
if (nameLower === "aborterror") {
return "aborted";
}
if (BROWSER_PERSISTENT_FAILURE_RE.test(detail)) {
return "persistent";
}
const looksLikeTimeout =
nameLower.includes("timeout") || msgLower.includes("timed out") || msgLower.includes("timeout");
nameLower.includes("timeout") ||
detailLower.includes("timed out") ||
detailLower.includes("timeout");
if (looksLikeTimeout) {
return "timeout";
}
if (BROWSER_TRANSIENT_NETWORK_ERROR_RE.test(detail)) {
return "transient-network";
}
const looksLikeAbort =
nameLower === "aborterror" ||
msgLower.includes("aborterror") ||
msgLower.includes("aborted") ||
msgLower.includes("abort") ||
msgLower.includes("cancelled") ||
msgLower.includes("canceled");
detailLower.includes("aborterror") ||
detailLower.includes("aborted") ||
detailLower.includes("abort") ||
detailLower.includes("cancelled") ||
detailLower.includes("canceled");
return looksLikeAbort ? "aborted" : "persistent";
}
function isPersistentBrowserServiceFailure(message: string, status: number | undefined): boolean {
return status === 401 || BROWSER_PERSISTENT_FAILURE_RE.test(message);
}
function resolveBrowserServiceModelHint(
message: string,
status: number | undefined,
): string | undefined {
if (message.includes(BROWSER_TOOL_PERSISTENT_MODEL_HINT)) {
return BROWSER_TOOL_PERSISTENT_MODEL_HINT;
}
if (message.includes(BROWSER_TOOL_TRANSIENT_MODEL_HINT)) {
return BROWSER_TOOL_TRANSIENT_MODEL_HINT;
}
if (isPersistentBrowserServiceFailure(message, status)) {
return BROWSER_TOOL_PERSISTENT_MODEL_HINT;
}
if (status === 408 || status === 504) {
return BROWSER_TOOL_TRANSIENT_MODEL_HINT;
}
if (status === undefined || status < 500 || status > 599) {
return undefined;
}
const kind = classifyBrowserFetchFailure(new Error(message));
return kind === "timeout" || kind === "transient-network"
? BROWSER_TOOL_TRANSIENT_MODEL_HINT
: undefined;
}
function resolveBrowserToolModelHint(kind: BrowserFetchFailureKind): string | undefined {
if (kind === "timeout" || kind === "transient-network") {
return BROWSER_TOOL_TRANSIENT_MODEL_HINT;
}
return kind === "persistent" ? BROWSER_TOOL_PERSISTENT_MODEL_HINT : undefined;
}
async function discardResponseBody(res: Response): Promise<void> {
try {
await res.body?.cancel();
@@ -230,8 +292,8 @@ function enhanceDispatcherPathError(url: string, err: unknown): Error {
const kind = classifyBrowserFetchFailure(err);
const ownership = resolveDispatcherBrowserControlOwnership(url);
const operatorHint = resolveBrowserFetchOperatorHint(url, { ownership });
const suffix =
kind === "persistent" ? `${operatorHint} ${BROWSER_TOOL_MODEL_HINT}` : operatorHint;
const modelHint = resolveBrowserToolModelHint(kind);
const suffix = modelHint ? `${operatorHint} ${modelHint}` : operatorHint;
const normalized = msg.endsWith(".") ? msg : `${msg}.`;
return new Error(`${normalized} ${suffix}`, err instanceof Error ? { cause: err } : undefined);
}
@@ -242,7 +304,7 @@ function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number):
const kind = classifyBrowserFetchFailure(err);
if (kind === "timeout") {
return new Error(
`Can't reach the OpenClaw browser control service (timed out after ${timeoutMs}ms). ${operatorHint}`,
`Can't reach the OpenClaw browser control service (timed out after ${timeoutMs}ms). ${operatorHint} ${BROWSER_TOOL_TRANSIENT_MODEL_HINT}`,
err instanceof Error ? { cause: err } : undefined,
);
}
@@ -252,9 +314,16 @@ function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number):
err instanceof Error ? { cause: err } : undefined,
);
}
if (kind === "transient-network") {
return new Error(
`Can't reach the OpenClaw browser control service. ${operatorHint} (${msg}) ${BROWSER_TOOL_TRANSIENT_MODEL_HINT}`,
err instanceof Error ? { cause: err } : undefined,
);
}
return new Error(
appendBrowserToolModelHint(
`Can't reach the OpenClaw browser control service. ${operatorHint} (${msg})`,
BROWSER_TOOL_PERSISTENT_MODEL_HINT,
),
err instanceof Error ? { cause: err } : undefined,
);
@@ -298,7 +367,7 @@ async function fetchHttpJson<T>(
// Do not reflect upstream response text into the error surface (log/agent injection risk)
await discardResponseBody(res);
throw new BrowserServiceError(
`${resolveBrowserRateLimitMessage(url)} ${BROWSER_TOOL_MODEL_HINT}`,
`${resolveBrowserRateLimitMessage(url)} ${BROWSER_TOOL_PERSISTENT_MODEL_HINT}`,
);
}
// Overflow cancels the stream and releases its reader lock before the guarded fetch below.
@@ -420,7 +489,7 @@ export async function fetchBrowserJson<T>(
if (isRateLimitStatus(result.status)) {
// Do not reflect upstream response text into the error surface (log/agent injection risk)
throw new BrowserServiceError(
`${resolveBrowserRateLimitMessage(url)} ${BROWSER_TOOL_MODEL_HINT}`,
`${resolveBrowserRateLimitMessage(url)} ${BROWSER_TOOL_PERSISTENT_MODEL_HINT}`,
);
}
throw browserServiceErrorFromPayload(result.body, `HTTP ${result.status}`, result.status);