fix(browser): surface 429 rate limit errors with actionable hints (#40491)
Merged via squash. Prepared head SHA: 13839c2dbd0396d8a582aa2c5c206d2efaff1b07 Co-authored-by: mvanhorn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Reviewed-by: @altaywtf
This commit is contained in:
@@ -77,6 +77,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Models/Alibaba Cloud Model Studio: wire `MODELSTUDIO_API_KEY` through shared env auth, implicit provider discovery, and shell-env fallback so onboarding works outside the wizard too. (#40634) Thanks @pomelo-nwu.
|
||||
- ACP/sessions_spawn: implicitly stream `mode="run"` ACP spawns to parent only for eligible subagent orchestrator sessions (heartbeat `target: "last"` with a usable session-local route), restoring parent progress relays without thread binding. (#42404) Thanks @davidguttman.
|
||||
- Sessions/reset model recompute: clear stale runtime model, context-token, and system-prompt metadata before session resets recompute the replacement session, so resets pick up current defaults and explicit overrides instead of reusing old runtime model state. (#41173) thanks @PonyX-lab.
|
||||
- Browser/Browserbase 429 handling: surface stable no-retry rate-limit guidance without buffering discarded HTTP 429 response bodies from remote browser services. (#40491) thanks @mvanhorn.
|
||||
|
||||
## 2026.3.8
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import { isLoopbackHost } from "../gateway/net.js";
|
||||
import { rawDataToString } from "../infra/ws.js";
|
||||
import { getDirectAgentForCdp, withNoProxyForCdpUrl } from "./cdp-proxy-bypass.js";
|
||||
import { CDP_HTTP_REQUEST_TIMEOUT_MS, CDP_WS_HANDSHAKE_TIMEOUT_MS } from "./cdp-timeouts.js";
|
||||
import { resolveBrowserRateLimitMessage } from "./client-fetch.js";
|
||||
import { getChromeExtensionRelayAuthHeaders } from "./extension-relay.js";
|
||||
|
||||
export { isLoopbackHost };
|
||||
@@ -172,6 +173,10 @@ export async function fetchCdpChecked(
|
||||
fetch(url, { ...init, headers, signal: ctrl.signal }),
|
||||
);
|
||||
if (!res.ok) {
|
||||
if (res.status === 429) {
|
||||
// Do not reflect upstream response text into the error surface (log/agent injection risk)
|
||||
throw new Error(`${resolveBrowserRateLimitMessage(url)} Do NOT retry the browser tool.`);
|
||||
}
|
||||
throw new Error(`HTTP ${res.status}`);
|
||||
}
|
||||
return res;
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { BrowserDispatchResponse } from "./routes/dispatcher.js";
|
||||
|
||||
function okDispatchResponse(): BrowserDispatchResponse {
|
||||
return { status: 200, body: { ok: true } };
|
||||
}
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
loadConfig: vi.fn(() => ({
|
||||
@@ -9,7 +14,7 @@ const mocks = vi.hoisted(() => ({
|
||||
},
|
||||
})),
|
||||
startBrowserControlServiceFromConfig: vi.fn(async () => ({ ok: true })),
|
||||
dispatch: vi.fn(async () => ({ status: 200, body: { ok: true } })),
|
||||
dispatch: vi.fn(async (): Promise<BrowserDispatchResponse> => okDispatchResponse()),
|
||||
}));
|
||||
|
||||
vi.mock("../config/config.js", async (importOriginal) => {
|
||||
@@ -57,7 +62,7 @@ describe("fetchBrowserJson loopback auth", () => {
|
||||
},
|
||||
});
|
||||
mocks.startBrowserControlServiceFromConfig.mockReset().mockResolvedValue({ ok: true });
|
||||
mocks.dispatch.mockReset().mockResolvedValue({ status: 200, body: { ok: true } });
|
||||
mocks.dispatch.mockReset().mockResolvedValue(okDispatchResponse());
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -133,6 +138,102 @@ 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 () => {
|
||||
const response = new Response("max concurrent sessions exceeded", { status: 429 });
|
||||
const text = vi.spyOn(response, "text");
|
||||
const cancel = vi.spyOn(response.body!, "cancel").mockResolvedValue(undefined);
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => response),
|
||||
);
|
||||
|
||||
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("Browser service rate limit reached");
|
||||
expect(thrown.message).toContain("Do NOT retry the browser tool");
|
||||
expect(thrown.message).not.toContain("max concurrent sessions exceeded");
|
||||
expect(text).not.toHaveBeenCalled();
|
||||
expect(cancel).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
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("keeps Browserbase-specific wording for Browserbase 429 responses", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response("max concurrent sessions exceeded", { status: 429 })),
|
||||
);
|
||||
|
||||
const thrown = await fetchBrowserJson<{ ok: boolean }>(
|
||||
"https://connect.browserbase.com/session",
|
||||
).catch((err: unknown) => err);
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error);
|
||||
if (!(thrown instanceof Error)) {
|
||||
throw new Error(`Expected Error, got ${String(thrown)}`);
|
||||
}
|
||||
expect(thrown.message).toContain("Browserbase rate limit reached");
|
||||
expect(thrown.message).toContain("upgrade your plan");
|
||||
expect(thrown.message).not.toContain("max concurrent sessions exceeded");
|
||||
});
|
||||
|
||||
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("Browser service rate limit reached");
|
||||
expect(thrown.message).toContain("Do NOT retry the browser tool");
|
||||
expect(thrown.message).not.toContain("too many sessions");
|
||||
});
|
||||
|
||||
it("keeps absolute URL failures wrapped as reachability errors", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
|
||||
@@ -102,6 +102,36 @@ 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_SERVICE_RATE_LIMIT_MESSAGE =
|
||||
"Browser service rate limit reached. " +
|
||||
"Wait for the current session to complete, or retry later.";
|
||||
|
||||
const BROWSERBASE_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 isBrowserbaseUrl(url: string): boolean {
|
||||
if (!isAbsoluteHttp(url)) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const host = new URL(url).hostname.toLowerCase();
|
||||
return host === "browserbase.com" || host.endsWith(".browserbase.com");
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveBrowserRateLimitMessage(url: string): string {
|
||||
return isBrowserbaseUrl(url)
|
||||
? BROWSERBASE_RATE_LIMIT_MESSAGE
|
||||
: BROWSER_SERVICE_RATE_LIMIT_MESSAGE;
|
||||
}
|
||||
|
||||
function resolveBrowserFetchOperatorHint(url: string): string {
|
||||
const isLocal = !isAbsoluteHttp(url);
|
||||
return isLocal
|
||||
@@ -123,6 +153,14 @@ function appendBrowserToolModelHint(message: string): string {
|
||||
return `${message} ${BROWSER_TOOL_MODEL_HINT}`;
|
||||
}
|
||||
|
||||
async function discardResponseBody(res: Response): Promise<void> {
|
||||
try {
|
||||
await res.body?.cancel();
|
||||
} catch {
|
||||
// Best effort only; we're already returning a stable error message.
|
||||
}
|
||||
}
|
||||
|
||||
function enhanceDispatcherPathError(url: string, err: unknown): Error {
|
||||
const msg = normalizeErrorMessage(err);
|
||||
const suffix = `${resolveBrowserFetchOperatorHint(url)} ${BROWSER_TOOL_MODEL_HINT}`;
|
||||
@@ -175,6 +213,13 @@ async function fetchHttpJson<T>(
|
||||
try {
|
||||
const res = await fetch(url, { ...init, signal: ctrl.signal });
|
||||
if (!res.ok) {
|
||||
if (isRateLimitStatus(res.status)) {
|
||||
// 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}`,
|
||||
);
|
||||
}
|
||||
const text = await res.text().catch(() => "");
|
||||
throw new BrowserServiceError(text || `HTTP ${res.status}`);
|
||||
}
|
||||
@@ -269,6 +314,12 @@ export async function fetchBrowserJson<T>(
|
||||
});
|
||||
|
||||
if (result.status >= 400) {
|
||||
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}`,
|
||||
);
|
||||
}
|
||||
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")) {
|
||||
break;
|
||||
}
|
||||
const delay = 250 + attempt * 250;
|
||||
await new Promise((r) => setTimeout(r, delay));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user