mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-19 01:00:39 +00:00
fix(browser): surface 429 rate limit errors with actionable hints
Detect HTTP 429 responses from Browserbase at the fetch level and throw errors with actionable messages instead of generic 'HTTP 429'. Break out of the CDP retry loop on rate-limit errors to avoid amplifying the 429. Closes #40390 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -172,6 +172,14 @@ export async function fetchCdpChecked(
|
||||
fetch(url, { ...init, headers, signal: ctrl.signal }),
|
||||
);
|
||||
if (!res.ok) {
|
||||
if (res.status === 429) {
|
||||
const text = await res.text().catch(() => "");
|
||||
const detail = text ? ` (${text.slice(0, 200)})` : "";
|
||||
throw new Error(
|
||||
`Browserbase rate limit reached (max concurrent sessions).${detail} ` +
|
||||
`Do NOT retry - wait for the current session to complete, or upgrade your plan.`,
|
||||
);
|
||||
}
|
||||
throw new Error(`HTTP ${res.status}`);
|
||||
}
|
||||
return res;
|
||||
|
||||
@@ -133,6 +133,75 @@ describe("fetchBrowserJson loopback auth", () => {
|
||||
expect(thrown.message).not.toContain("Can't reach the OpenClaw browser control service");
|
||||
});
|
||||
|
||||
it("surfaces 429 from HTTP URL as rate-limit error with no-retry hint", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response("max concurrent sessions exceeded", { status: 429 })),
|
||||
);
|
||||
|
||||
const thrown = await fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/").catch(
|
||||
(err: unknown) => err,
|
||||
);
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error);
|
||||
if (!(thrown instanceof Error)) {
|
||||
throw new Error(`Expected Error, got ${String(thrown)}`);
|
||||
}
|
||||
expect(thrown.message).toContain("rate limit reached");
|
||||
expect(thrown.message).toContain("Do NOT retry the browser tool");
|
||||
expect(thrown.message).toContain("max concurrent sessions exceeded");
|
||||
});
|
||||
|
||||
it("surfaces 429 from HTTP URL without body detail when empty", async () => {
|
||||
vi.stubGlobal("fetch", vi.fn(async () => new Response("", { status: 429 })));
|
||||
|
||||
const thrown = await fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/").catch(
|
||||
(err: unknown) => err,
|
||||
);
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error);
|
||||
if (!(thrown instanceof Error)) {
|
||||
throw new Error(`Expected Error, got ${String(thrown)}`);
|
||||
}
|
||||
expect(thrown.message).toContain("rate limit reached");
|
||||
expect(thrown.message).toContain("Do NOT retry the browser tool");
|
||||
});
|
||||
|
||||
it("non-429 errors still produce generic messages", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response("internal error", { status: 500 })),
|
||||
);
|
||||
|
||||
const thrown = await fetchBrowserJson<{ ok: boolean }>("http://127.0.0.1:18888/").catch(
|
||||
(err: unknown) => err,
|
||||
);
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error);
|
||||
if (!(thrown instanceof Error)) {
|
||||
throw new Error(`Expected Error, got ${String(thrown)}`);
|
||||
}
|
||||
expect(thrown.message).toContain("internal error");
|
||||
expect(thrown.message).not.toContain("rate limit");
|
||||
});
|
||||
|
||||
it("surfaces 429 from dispatcher path as rate-limit error", async () => {
|
||||
mocks.dispatch.mockResolvedValueOnce({
|
||||
status: 429,
|
||||
body: { error: "too many sessions" },
|
||||
});
|
||||
|
||||
const thrown = await fetchBrowserJson<{ ok: boolean }>("/tabs").catch((err: unknown) => err);
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error);
|
||||
if (!(thrown instanceof Error)) {
|
||||
throw new Error(`Expected Error, got ${String(thrown)}`);
|
||||
}
|
||||
expect(thrown.message).toContain("rate limit reached");
|
||||
expect(thrown.message).toContain("Do NOT retry the browser tool");
|
||||
expect(thrown.message).toContain("too many sessions");
|
||||
});
|
||||
|
||||
it("keeps absolute URL failures wrapped as reachability errors", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
|
||||
@@ -102,6 +102,14 @@ 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.";
|
||||
|
||||
const BROWSER_RATE_LIMIT_MESSAGE =
|
||||
"Browserbase rate limit reached (max concurrent sessions). " +
|
||||
"Wait for the current session to complete, or upgrade your plan.";
|
||||
|
||||
function isRateLimitStatus(status: number): boolean {
|
||||
return status === 429;
|
||||
}
|
||||
|
||||
function resolveBrowserFetchOperatorHint(url: string): string {
|
||||
const isLocal = !isAbsoluteHttp(url);
|
||||
return isLocal
|
||||
@@ -176,6 +184,12 @@ async function fetchHttpJson<T>(
|
||||
const res = await fetch(url, { ...init, signal: ctrl.signal });
|
||||
if (!res.ok) {
|
||||
const text = await res.text().catch(() => "");
|
||||
if (isRateLimitStatus(res.status)) {
|
||||
const detail = text ? ` (${text.slice(0, 200)})` : "";
|
||||
throw new BrowserServiceError(
|
||||
`${BROWSER_RATE_LIMIT_MESSAGE}${detail} ${BROWSER_TOOL_MODEL_HINT}`,
|
||||
);
|
||||
}
|
||||
throw new BrowserServiceError(text || `HTTP ${res.status}`);
|
||||
}
|
||||
return (await res.json()) as T;
|
||||
@@ -269,6 +283,15 @@ export async function fetchBrowserJson<T>(
|
||||
});
|
||||
|
||||
if (result.status >= 400) {
|
||||
if (isRateLimitStatus(result.status)) {
|
||||
const detail =
|
||||
result.body && typeof result.body === "object" && "error" in result.body
|
||||
? ` (${String((result.body as { error?: unknown }).error).slice(0, 200)})`
|
||||
: "";
|
||||
throw new BrowserServiceError(
|
||||
`${BROWSER_RATE_LIMIT_MESSAGE}${detail} ${BROWSER_TOOL_MODEL_HINT}`,
|
||||
);
|
||||
}
|
||||
const message =
|
||||
result.body && typeof result.body === "object" && "error" in result.body
|
||||
? String((result.body as { error?: unknown }).error)
|
||||
|
||||
@@ -365,6 +365,11 @@ async function connectBrowser(cdpUrl: string): Promise<ConnectedBrowser> {
|
||||
return connected;
|
||||
} catch (err) {
|
||||
lastErr = err;
|
||||
// Don't retry rate-limit errors; retrying worsens the 429.
|
||||
const errMsg = err instanceof Error ? err.message : String(err);
|
||||
if (errMsg.includes("rate limit") || errMsg.includes("429")) {
|
||||
break;
|
||||
}
|
||||
const delay = 250 + attempt * 250;
|
||||
await new Promise((r) => setTimeout(r, delay));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user