diff --git a/docs/tools/browser.md b/docs/tools/browser.md index a325c82b28a..9446161baaa 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -316,6 +316,11 @@ main model can read the screenshot directly. current process. `OPENCLAW_BROWSER_HEADLESS=0` forces headed mode for ordinary starts and returns an actionable error on Linux hosts without a display server; an explicit `start --headless` request still wins for that one launch. +- The browser-control route and programmatic client keep the no-display error's + human-readable `error` and expose the stable reason + `no_display_for_headed_profile`. Its `details` contain only `profile`, + `requestedHeadless`, `headlessSource`, and `displayPresent`, so API clients can + choose the correct remediation without matching message text. - `executablePath` can be set globally or per local managed profile. Per-profile values override `browser.executablePath`, so different managed profiles can launch different Chromium-based browsers. Both forms accept `~` for your OS home directory. - `color` (top-level and per-profile) tints the browser UI so you can see which profile is active. - Default profile is `openclaw` (managed standalone). Use `defaultProfile: "user"` to opt into the signed-in user browser. diff --git a/extensions/browser/src/browser-proxy-envelope.ts b/extensions/browser/src/browser-proxy-envelope.ts new file mode 100644 index 00000000000..a0a4999bbec --- /dev/null +++ b/extensions/browser/src/browser-proxy-envelope.ts @@ -0,0 +1,76 @@ +/** + * Browser node-proxy response envelope shared by the node host and Gateway. + */ +import { parseBrowserErrorPayload, type BrowserNoDisplayErrorMetadata } from "./browser/errors.js"; + +/** Additive opt-in for structured browser route errors over node.invoke. */ +export const BROWSER_PROXY_ERROR_ENVELOPE = "browser-v1" as const; + +export type BrowserProxyFile = { + path: string; + base64: string; + mimeType?: string; +}; + +export type BrowserProxyErrorBody = + | { error: string } + | ({ error: string } & BrowserNoDisplayErrorMetadata); + +export type BrowserProxySuccess = { + result: unknown; + files?: BrowserProxyFile[]; +}; + +export type BrowserProxyFailure = { + error: { + status: number; + body: BrowserProxyErrorBody; + }; +}; + +export type BrowserProxyEnvelope = BrowserProxySuccess | BrowserProxyFailure; + +function normalizeBrowserProxyErrorBody( + value: unknown, + fallback?: string, +): BrowserProxyErrorBody | null { + const parsed = parseBrowserErrorPayload(value); + if (parsed) { + return parsed; + } + return fallback ? { error: fallback } : null; +} + +/** Build a route-failure envelope while allowing only closed Browser metadata. */ +export function createBrowserProxyFailure(status: number, body: unknown): BrowserProxyFailure { + return { + error: { + status, + body: normalizeBrowserProxyErrorBody(body, `HTTP ${status}`) ?? { error: `HTTP ${status}` }, + }, + }; +} + +/** Parse an untrusted node response without forwarding arbitrary metadata. */ +export function parseBrowserProxyFailure(value: unknown): BrowserProxyFailure | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const error = (value as { error?: unknown }).error; + if (!error || typeof error !== "object" || Array.isArray(error)) { + return null; + } + const candidate = error as { status?: unknown; body?: unknown }; + if ( + !Number.isInteger(candidate.status) || + (candidate.status as number) < 400 || + (candidate.status as number) > 599 + ) { + return null; + } + const body = normalizeBrowserProxyErrorBody(candidate.body); + if (!body) { + return null; + } + return { error: { status: candidate.status as number, body } }; +} diff --git a/extensions/browser/src/browser-tool.actions.ts b/extensions/browser/src/browser-tool.actions.ts index dfc550231af..81a23225deb 100644 --- a/extensions/browser/src/browser-tool.actions.ts +++ b/extensions/browser/src/browser-tool.actions.ts @@ -288,9 +288,12 @@ function isChromeStaleTargetError(profile: string | undefined, err: unknown): bo if (!profile) { return false; } + const status = + err && typeof err === "object" && "status" in err ? (err as { status?: unknown }).status : null; + const msg = String(err); + const isTabNotFound = (status === 404 || msg.includes("404:")) && msg.includes("tab not found"); if (profile === "user") { - const msg = String(err); - return msg.includes("404:") && msg.includes("tab not found"); + return isTabNotFound; } const cfg = browserToolActionDeps.getRuntimeConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -298,8 +301,7 @@ function isChromeStaleTargetError(profile: string | undefined, err: unknown): bo if (!browserProfile || !getBrowserProfileCapabilities(browserProfile).usesChromeMcp) { return false; } - const msg = String(err); - return msg.includes("404:") && msg.includes("tab not found"); + return isTabNotFound; } function stripTargetIdFromActRequest( diff --git a/extensions/browser/src/browser-tool.test.ts b/extensions/browser/src/browser-tool.test.ts index 801435dc085..f2c4d491fad 100644 --- a/extensions/browser/src/browser-tool.test.ts +++ b/extensions/browser/src/browser-tool.test.ts @@ -466,6 +466,7 @@ function nodeInvokeCall(callIndex: number): { path?: string; profile?: string; timeoutMs?: number; + errorEnvelope?: string; query?: { refs?: string }; body?: Record; }; @@ -1019,6 +1020,7 @@ describe("browser tool snapshot maxChars", () => { expect(request.nodeId).toBe("node-1"); expect(request.command).toBe("browser.proxy"); expect(request.params?.timeoutMs).toBe(20_000); + expect(request.params?.errorEnvelope).toBe("browser-v1"); expect(browserClientMocks.browserStatus).not.toHaveBeenCalled(); }); @@ -1051,6 +1053,79 @@ describe("browser tool snapshot maxChars", () => { expect(browserClientMocks.browserStatus).not.toHaveBeenCalled(); }); + it("preserves validated browser errors returned by a node proxy", async () => { + mockSingleBrowserProxyNode(); + gatewayMocks.callGatewayTool.mockResolvedValueOnce({ + ok: true, + payload: { + error: { + status: 409, + body: { + error: "headed mode needs a display", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }, + }, + }, + }); + const tool = createBrowserTool(); + + const error = await tool.execute!("call-1", { + action: "start", + target: "node", + profile: "openclaw", + }).catch((err: unknown) => err); + + expect(error).toMatchObject({ + name: "BrowserServiceError", + message: "headed mode needs a display", + status: 409, + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }); + }); + + it("drops unrecognized metadata returned by a node proxy", async () => { + mockSingleBrowserProxyNode(); + gatewayMocks.callGatewayTool.mockResolvedValueOnce({ + ok: true, + payload: { + error: { + status: 409, + body: { + error: "headed mode needs a display", + reason: "untrusted_reason", + details: { remediation: "run arbitrary text" }, + }, + }, + }, + }); + const tool = createBrowserTool(); + + const error = await tool.execute!("call-1", { + action: "start", + target: "node", + profile: "openclaw", + }).catch((err: unknown) => err); + + expect(error).toMatchObject({ + name: "BrowserServiceError", + message: "headed mode needs a display", + }); + expect(error).not.toHaveProperty("reason", "untrusted_reason"); + expect(error).not.toHaveProperty("details.remediation"); + }); + it("returns a browser doctor report on host", async () => { const tool = createBrowserTool(); await tool.execute?.("call-1", { action: "doctor" }); @@ -2139,7 +2214,10 @@ describe("browser tool act stale target recovery", () => { user: { driver: "existing-session", attachOnly: true, color: "#00AA00" }, }); gatewayMocks.callGatewayTool - .mockRejectedValueOnce(new Error("INVALID_REQUEST: Error: 404: tab not found")) + .mockResolvedValueOnce({ + ok: true, + payload: { error: { status: 404, body: { error: "tab not found" } } }, + }) .mockResolvedValueOnce({ ok: true, payload: { result: { tabs: [{ targetId: "only-tab" }] } }, diff --git a/extensions/browser/src/browser-tool.ts b/extensions/browser/src/browser-tool.ts index 0ee0e9fde75..a6986e3d8f8 100644 --- a/extensions/browser/src/browser-tool.ts +++ b/extensions/browser/src/browser-tool.ts @@ -5,6 +5,13 @@ * maps high-level actions onto browser control client calls. */ import crypto from "node:crypto"; +import { + BROWSER_PROXY_ERROR_ENVELOPE, + parseBrowserProxyFailure, + type BrowserProxyEnvelope, + type BrowserProxyFile, + type BrowserProxySuccess, +} from "./browser-proxy-envelope.js"; import { describeBrowserTool } from "./browser-tool-description.js"; import { executeActAction, @@ -56,6 +63,7 @@ import { trackSessionBrowserTab, untrackSessionBrowserTab, } from "./browser-tool.runtime.js"; +import { BrowserServiceError } from "./browser/client-fetch.js"; import { DEFAULT_BROWSER_SCREENSHOT_TIMEOUT_MS } from "./browser/constants.js"; import { parseBrowserNavigationUrl } from "./browser/navigation-guard.js"; import { normalizeBrowserScreenshot } from "./browser/screenshot.js"; @@ -249,17 +257,6 @@ function readActRequestParam(params: Record) { return request as Parameters[1]; } -type BrowserProxyFile = { - path: string; - base64: string; - mimeType?: string; -}; - -type BrowserProxyResult = { - result: unknown; - files?: BrowserProxyFile[]; -}; - const DEFAULT_BROWSER_PROXY_TIMEOUT_MS = 20_000; const BROWSER_PROXY_GATEWAY_TIMEOUT_SLACK_MS = 5_000; @@ -360,7 +357,7 @@ async function callBrowserProxy(params: { body?: unknown; timeoutMs?: number; profile?: string; -}): Promise { +}): Promise { const proxyTimeoutMs = typeof params.timeoutMs === "number" && Number.isFinite(params.timeoutMs) ? Math.max(1, Math.floor(params.timeoutMs)) @@ -379,27 +376,35 @@ async function callBrowserProxy(params: { body: params.body, timeoutMs: proxyTimeoutMs, profile: params.profile, + errorEnvelope: BROWSER_PROXY_ERROR_ENVELOPE, }, idempotencyKey: crypto.randomUUID(), }, { scopes: ["operator.admin"] }, ); const parsed = unwrapBrowserProxyPayload(payload); + const failure = parseBrowserProxyFailure(parsed); + if (failure) { + const { status, body } = failure.error; + throw new BrowserServiceError(body.error, "reason" in body ? body : undefined, status); + } if (!parsed || typeof parsed !== "object" || !("result" in parsed)) { throw new Error("browser proxy failed"); } return parsed; } -function unwrapBrowserProxyPayload(payload: { payload?: unknown; payloadJSON?: unknown } | null) { +function unwrapBrowserProxyPayload( + payload: { payload?: unknown; payloadJSON?: unknown } | null, +): BrowserProxyEnvelope | null { if (payload?.payload !== undefined) { - return payload.payload; + return payload.payload as BrowserProxyEnvelope; } if (typeof payload?.payloadJSON !== "string" || !payload.payloadJSON.trim()) { return null; } try { - return JSON.parse(payload.payloadJSON) as BrowserProxyResult; + return JSON.parse(payload.payloadJSON) as BrowserProxyEnvelope; } catch { return null; } diff --git a/extensions/browser/src/browser/chrome.internal.test.ts b/extensions/browser/src/browser/chrome.internal.test.ts index ce7c0bac49d..4e5f035656c 100644 --- a/extensions/browser/src/browser/chrome.internal.test.ts +++ b/extensions/browser/src/browser/chrome.internal.test.ts @@ -68,6 +68,7 @@ import { stopOpenClawChrome, } from "./chrome.js"; import type { ResolvedBrowserConfig, ResolvedBrowserProfile } from "./config.js"; +import { BROWSER_ERROR_REASONS, BrowserProfileUnavailableError } from "./errors.js"; const CHROME_TEST_WS_MAX_PAYLOAD_BYTES = 1024 * 1024; @@ -573,6 +574,34 @@ describe("chrome.ts internal", () => { expect(spawnMock).not.toHaveBeenCalled(); }); + it("returns structured no-display details before spawning headed Chrome", async () => { + const profile = { + ...makeProfile(51110), + driver: "openclaw", + attachOnly: false, + headless: false, + headlessSource: "profile", + } as ResolvedBrowserProfile; + const error = await launchOpenClawChrome(makeResolved(), profile, { + platform: "linux", + env: { DISPLAY: undefined, WAYLAND_DISPLAY: undefined }, + }).catch((err: unknown) => err); + + expect(error).toBeInstanceOf(BrowserProfileUnavailableError); + expect(error).toMatchObject({ + metadata: { + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details: { + profile: profile.name, + requestedHeadless: false, + headlessSource: "profile", + displayPresent: false, + }, + }, + }); + expect(spawnMock).not.toHaveBeenCalled(); + }); + it("throws when no supported browser executable is found", async () => { // Strip all candidate executables — override config so no explicit // path is set, then mock existsSync to return false for everything. @@ -2032,7 +2061,6 @@ describe("chrome.ts internal", () => { ); }); const profile = makeLoopbackProfile(54324); - const { BrowserProfileUnavailableError } = await import("./errors.js"); await expect(launchOpenClawChrome(makeResolved(), profile)).rejects.toBeInstanceOf( BrowserProfileUnavailableError, ); diff --git a/extensions/browser/src/browser/chrome.ts b/extensions/browser/src/browser/chrome.ts index 663d010c0a8..226666a5957 100644 --- a/extensions/browser/src/browser/chrome.ts +++ b/extensions/browser/src/browser/chrome.ts @@ -77,7 +77,7 @@ import { DEFAULT_OPENCLAW_BROWSER_COLOR, DEFAULT_OPENCLAW_BROWSER_PROFILE_NAME, } from "./constants.js"; -import { BrowserProfileUnavailableError } from "./errors.js"; +import { BROWSER_ERROR_REASONS, BrowserProfileUnavailableError } from "./errors.js"; import { ensureOutputDirectory } from "./output-directories.js"; import { DEFAULT_DOWNLOAD_DIR } from "./paths.js"; @@ -973,7 +973,17 @@ export async function launchOpenClawChrome( launchOptions, ); if (missingDisplayError) { - throw new BrowserProfileUnavailableError(missingDisplayError); + throw new BrowserProfileUnavailableError(missingDisplayError.message, { + metadata: { + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details: { + profile: profile.name, + requestedHeadless: false, + headlessSource: missingDisplayError.headlessSource, + displayPresent: false, + }, + }, + }); } // Surface `loopbackMode=block` before spawning Chrome. The CDP fetch and diff --git a/extensions/browser/src/browser/client-fetch.error-body-boundary.test.ts b/extensions/browser/src/browser/client-fetch.error-body-boundary.test.ts index 279a35ac823..f748d51739f 100644 --- a/extensions/browser/src/browser/client-fetch.error-body-boundary.test.ts +++ b/extensions/browser/src/browser/client-fetch.error-body-boundary.test.ts @@ -109,6 +109,23 @@ describe("fetchHttpJson error body boundary", () => { return; } + if (req.url === "/structured") { + res.writeHead(409, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + error: "display required", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }), + ); + return; + } + res.writeHead(500, { "Content-Type": "text/plain" }); let written = 0; let closed = false; @@ -187,4 +204,20 @@ describe("fetchHttpJson error body boundary", () => { }); await expect(smallConnectionClosed).resolves.toBeUndefined(); }); + + it("preserves validated structured errors over HTTP", async () => { + const error = await fetchBrowserJson(`${baseUrl}/structured`).catch((err: unknown) => err); + + expect(error).toMatchObject({ + name: "BrowserServiceError", + message: "display required", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }); + }); }); diff --git a/extensions/browser/src/browser/client-fetch.loopback-auth.test.ts b/extensions/browser/src/browser/client-fetch.loopback-auth.test.ts index 6ba0cd65271..c08b0fb6618 100644 --- a/extensions/browser/src/browser/client-fetch.loopback-auth.test.ts +++ b/extensions/browser/src/browser/client-fetch.loopback-auth.test.ts @@ -449,6 +449,38 @@ describe("fetchBrowserJson loopback auth", () => { }); }); + it("preserves validated structured errors from dispatcher routes", async () => { + mocks.dispatch.mockResolvedValueOnce({ + status: 409, + body: { + error: "display required", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "request", + displayPresent: false, + }, + }, + }); + + const error = await fetchBrowserJson("/start?headless=false", { method: "POST" }).catch( + (err: unknown) => err, + ); + + expect(error).toMatchObject({ + name: "BrowserServiceError", + message: "display required", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "request", + displayPresent: false, + }, + }); + }); + 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"); diff --git a/extensions/browser/src/browser/client-fetch.ts b/extensions/browser/src/browser/client-fetch.ts index abfed2bc65d..bc9771eb4a7 100644 --- a/extensions/browser/src/browser/client-fetch.ts +++ b/extensions/browser/src/browser/client-fetch.ts @@ -16,17 +16,42 @@ import { isLoopbackHost } from "../gateway/net.js"; import { getBridgeAuthForPort } from "./bridge-auth-registry.js"; import { resolveBrowserConfig, resolveProfile } from "./config.js"; import { resolveBrowserControlAuth } from "./control-auth.js"; +import { + parseBrowserErrorPayload, + type BrowserNoDisplayErrorMetadata, + type BrowserNoDisplayErrorDetails, +} from "./errors.js"; import { resolveBrowserRateLimitMessage } from "./rate-limit-message.js"; // Application-level error from the browser control service (service is reachable // but returned an error response). Must NOT be wrapped with "Can't reach ..." messaging. -class BrowserServiceError extends Error { - constructor(message: string) { +export class BrowserServiceError extends Error { + readonly status?: number; + readonly reason?: BrowserNoDisplayErrorMetadata["reason"]; + readonly details?: BrowserNoDisplayErrorDetails; + + constructor(message: string, metadata?: BrowserNoDisplayErrorMetadata, status?: number) { super(message); this.name = "BrowserServiceError"; + this.status = status; + this.reason = metadata?.reason; + this.details = metadata?.details; } } +function browserServiceErrorFromPayload( + value: unknown, + fallback: string, + status?: number, +): BrowserServiceError { + const parsed = parseBrowserErrorPayload(value); + return new BrowserServiceError( + parsed?.error ?? fallback, + parsed && "reason" in parsed ? parsed : undefined, + status, + ); +} + type LoopbackBrowserAuthDeps = { getRuntimeConfig: typeof getRuntimeConfig; resolveBrowserControlAuth: typeof resolveBrowserControlAuth; @@ -277,7 +302,15 @@ async function fetchHttpJson( () => undefined, ); const text = body ? new TextDecoder().decode(body) : ""; - throw new BrowserServiceError(text || `HTTP ${res.status}`); + let parsed: unknown; + if (text) { + try { + parsed = JSON.parse(text); + } catch { + // Plain-text errors remain part of the existing browser-control contract. + } + } + throw browserServiceErrorFromPayload(parsed, text || `HTTP ${res.status}`, res.status); } const body = await readResponseWithLimit(res, BROWSER_SUCCESS_BODY_LIMIT_BYTES, { onOverflow: ({ maxBytes }) => @@ -386,11 +419,7 @@ export async function fetchBrowserJson( `${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) - : `HTTP ${result.status}`; - throw new BrowserServiceError(message); + throw browserServiceErrorFromPayload(result.body, `HTTP ${result.status}`, result.status); } return result.body as T; } catch (err) { diff --git a/extensions/browser/src/browser/config.test.ts b/extensions/browser/src/browser/config.test.ts index c36c7e09b23..1a54910b9fe 100644 --- a/extensions/browser/src/browser/config.test.ts +++ b/extensions/browser/src/browser/config.test.ts @@ -532,7 +532,21 @@ describe("browser config", () => { platform: "linux", env: noDisplayEnv, }), - ).toContain("browser.profiles.openclaw.headless=false"); + ).toMatchObject({ + message: expect.stringContaining("browser.profiles.openclaw.headless=false"), + headlessSource: "profile", + }); + + expect( + getManagedBrowserMissingDisplayError(defaultResolved, defaultProfile, { + headlessOverride: false, + platform: "linux", + env: noDisplayEnv, + }), + ).toMatchObject({ + message: expect.stringContaining("request override"), + headlessSource: "request", + }); }); }); diff --git a/extensions/browser/src/browser/config.ts b/extensions/browser/src/browser/config.ts index 2570138d44f..22585f1ced0 100644 --- a/extensions/browser/src/browser/config.ts +++ b/extensions/browser/src/browser/config.ts @@ -148,6 +148,11 @@ type ManagedBrowserHeadlessMode = { source: ManagedBrowserHeadlessSource; }; +export type ManagedBrowserMissingDisplayError = { + message: string; + headlessSource: Exclude; +}; + /** Inputs used to resolve managed Chrome headless mode. */ export type ManagedBrowserHeadlessOptions = { headlessOverride?: boolean; @@ -722,7 +727,7 @@ export function getManagedBrowserMissingDisplayError( resolved: ResolvedBrowserConfig, profile: ResolvedBrowserProfile, params: ManagedBrowserHeadlessOptions = {}, -): string | null { +): ManagedBrowserMissingDisplayError | null { if (!isLocalManagedProfile(profile)) { return null; } @@ -732,8 +737,12 @@ export function getManagedBrowserMissingDisplayError( return null; } - const mode = resolveManagedBrowserHeadlessMode(resolved, profile, { env, platform }); - if (mode.headless) { + const mode = resolveManagedBrowserHeadlessMode(resolved, profile, { + ...params, + env, + platform, + }); + if (mode.headless || mode.source === "linux-display-fallback") { return null; } @@ -745,9 +754,11 @@ export function getManagedBrowserMissingDisplayError( : mode.source === "profile" ? `browser.profiles.${profile.name}.headless=false` : "browser.headless=false"; - return ( - `Headed browser start requested for profile "${profile.name}" via ${sourceHint}, ` + - "but no Linux display server was detected ($DISPLAY/$WAYLAND_DISPLAY unset). " + - `Set ${OPENCLAW_BROWSER_HEADLESS_ENV}=1, remove the headed override, or launch under Xvfb.` - ); + return { + message: + `Headed browser start requested for profile "${profile.name}" via ${sourceHint}, ` + + "but no Linux display server was detected ($DISPLAY/$WAYLAND_DISPLAY unset). " + + `Set ${OPENCLAW_BROWSER_HEADLESS_ENV}=1, remove the headed override, or launch under Xvfb.`, + headlessSource: mode.source, + }; } diff --git a/extensions/browser/src/browser/errors.test.ts b/extensions/browser/src/browser/errors.test.ts index 0832578e66a..b11520902e1 100644 --- a/extensions/browser/src/browser/errors.test.ts +++ b/extensions/browser/src/browser/errors.test.ts @@ -1,6 +1,12 @@ // Browser tests cover errors plugin behavior. import { describe, expect, it } from "vitest"; -import { BrowserTabNotFoundError } from "./errors.js"; +import { + BROWSER_ERROR_REASONS, + BrowserProfileUnavailableError, + BrowserTabNotFoundError, + parseBrowserErrorPayload, + toBrowserErrorResponse, +} from "./errors.js"; describe("BrowserTabNotFoundError", () => { it("teaches agents that bare numbers are not stable tab targets", () => { @@ -11,3 +17,49 @@ describe("BrowserTabNotFoundError", () => { ); }); }); + +describe("no-display browser errors", () => { + const details = { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "profile", + displayPresent: false, + } as const; + + it("maps a closed reason and typed details", () => { + expect( + toBrowserErrorResponse( + new BrowserProfileUnavailableError("display required", { + metadata: { + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details, + }, + }), + ), + ).toEqual({ + status: 409, + message: "display required", + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details, + }); + }); + + it("accepts only valid no-display metadata from route payloads", () => { + const payload = { + error: "display required", + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details, + }; + expect(parseBrowserErrorPayload(payload)).toEqual({ + error: "display required", + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details, + }); + expect( + parseBrowserErrorPayload({ + ...payload, + details: { ...details, requestedHeadless: true, remediation: "untrusted" }, + }), + ).toEqual({ error: "display required" }); + }); +}); diff --git a/extensions/browser/src/browser/errors.ts b/extensions/browser/src/browser/errors.ts index b1ccaecfecb..0f83d532e08 100644 --- a/extensions/browser/src/browser/errors.ts +++ b/extensions/browser/src/browser/errors.ts @@ -9,6 +9,29 @@ export const BROWSER_ENDPOINT_BLOCKED_MESSAGE = "browser endpoint blocked by pol /** Stable message for blocked page navigation targets. */ export const BROWSER_NAVIGATION_BLOCKED_MESSAGE = "browser navigation blocked by policy"; +/** Stable machine-readable browser error reasons. */ +export const BROWSER_ERROR_REASONS = { + noDisplayForHeadedProfile: "no_display_for_headed_profile", +} as const; + +const NO_DISPLAY_HEADLESS_SOURCES = ["request", "env", "profile", "config", "default"] as const; + +export type BrowserNoDisplayErrorDetails = { + profile: string; + requestedHeadless: false; + headlessSource: (typeof NO_DISPLAY_HEADLESS_SOURCES)[number]; + displayPresent: false; +}; + +export type BrowserNoDisplayErrorMetadata = { + reason: typeof BROWSER_ERROR_REASONS.noDisplayForHeadedProfile; + details: BrowserNoDisplayErrorDetails; +}; + +type WithNoDisplayMetadata = T | (T & BrowserNoDisplayErrorMetadata); +export type BrowserErrorResponse = WithNoDisplayMetadata<{ status: number; message: string }>; +export type BrowserErrorPayload = WithNoDisplayMetadata<{ error: string }>; + /** Base browser error carrying an HTTP status code. */ export class BrowserError extends Error { status: number; @@ -83,8 +106,14 @@ export class BrowserResetUnsupportedError extends BrowserError { /** Raised when a profile is configured but not currently reachable. */ export class BrowserProfileUnavailableError extends BrowserError { - constructor(message: string, options?: ErrorOptions) { + readonly metadata?: BrowserNoDisplayErrorMetadata; + + constructor( + message: string, + options?: ErrorOptions & { metadata?: BrowserNoDisplayErrorMetadata }, + ) { super(message, 409, options); + this.metadata = options?.metadata; } } @@ -96,10 +125,14 @@ export class BrowserResourceExhaustedError extends BrowserError { } /** Map browser-domain errors to HTTP response details. */ -export function toBrowserErrorResponse(err: unknown): { - status: number; - message: string; -} | null { +export function toBrowserErrorResponse(err: unknown): BrowserErrorResponse | null { + if (err instanceof BrowserProfileUnavailableError && err.metadata) { + return { + status: err.status, + message: err.message, + ...err.metadata, + }; + } if (err instanceof BrowserError) { return { status: err.status, message: err.message }; } @@ -119,3 +152,45 @@ export function toBrowserErrorResponse(err: unknown): { } return null; } + +function parseNoDisplayDetails(value: unknown): BrowserNoDisplayErrorDetails | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const details = value as Record; + if ( + typeof details.profile !== "string" || + details.profile.length === 0 || + details.requestedHeadless !== false || + !NO_DISPLAY_HEADLESS_SOURCES.includes( + details.headlessSource as BrowserNoDisplayErrorDetails["headlessSource"], + ) || + details.displayPresent !== false + ) { + return null; + } + return { + profile: details.profile, + requestedHeadless: false, + headlessSource: details.headlessSource as BrowserNoDisplayErrorDetails["headlessSource"], + displayPresent: false, + }; +} + +/** Parse only the closed browser error metadata contract from a route payload. */ +export function parseBrowserErrorPayload(value: unknown): BrowserErrorPayload | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const body = value as Record; + if (typeof body.error !== "string" || body.error.length === 0) { + return null; + } + if (body.reason === BROWSER_ERROR_REASONS.noDisplayForHeadedProfile) { + const details = parseNoDisplayDetails(body.details); + if (details) { + return { error: body.error, reason: body.reason, details }; + } + } + return { error: body.error }; +} diff --git a/extensions/browser/src/browser/routes/agent.shared.test.ts b/extensions/browser/src/browser/routes/agent.shared.test.ts index cc1a20bdf15..0cea99e211e 100644 --- a/extensions/browser/src/browser/routes/agent.shared.test.ts +++ b/extensions/browser/src/browser/routes/agent.shared.test.ts @@ -1,9 +1,11 @@ // Browser tests cover agent.shared plugin behavior. import { describe, expect, it, vi } from "vitest"; +import { BrowserProfileUnavailableError, toBrowserErrorResponse } from "../errors.js"; import type { BrowserRouteContext, ProfileContext } from "../server-context.js"; import "../../test-support/browser-security.mock.js"; import { readBody, + handleRouteError, resolveSafeRouteTabUrl, resolveTargetIdFromBody, resolveTargetIdFromQuery, @@ -71,6 +73,30 @@ function routeContextForTab( } describe("browser route shared helpers", () => { + it("preserves structured browser errors on agent routes", () => { + const response = createBrowserRouteResponse(); + const error = new BrowserProfileUnavailableError("display required", { + metadata: { + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "env", + displayPresent: false, + }, + }, + }); + + handleRouteError({ mapTabError: toBrowserErrorResponse } as never, response.res, error); + + expect(response.statusCode).toBe(409); + expect(response.body).toMatchObject({ + error: "display required", + reason: "no_display_for_headed_profile", + details: { headlessSource: "env" }, + }); + }); + describe("readBody", () => { it("returns object bodies", () => { expect(readBody(requestWithBody({ one: 1 }))).toEqual({ one: 1 }); diff --git a/extensions/browser/src/browser/routes/agent.shared.ts b/extensions/browser/src/browser/routes/agent.shared.ts index 9c70f1999f1..d323141ec4f 100644 --- a/extensions/browser/src/browser/routes/agent.shared.ts +++ b/extensions/browser/src/browser/routes/agent.shared.ts @@ -15,7 +15,7 @@ import type { PwAiModule } from "../pw-ai-module.js"; import { getPwAiModule as getPwAiModuleBase } from "../pw-ai-module.js"; import type { BrowserRouteContext, ProfileContext } from "../server-context.js"; import type { BrowserRequest, BrowserResponse } from "./types.js"; -import { getProfileContext, jsonError } from "./utils.js"; +import { getProfileContext, jsonBrowserError, jsonError } from "./utils.js"; export const SELECTOR_UNSUPPORTED_MESSAGE = [ "Error: 'selector' is not supported. Use 'ref' from snapshot instead.", @@ -52,11 +52,11 @@ export function resolveTargetIdFromQuery(query: Record): string export function handleRouteError(ctx: BrowserRouteContext, res: BrowserResponse, err: unknown) { const mapped = ctx.mapTabError(err); if (mapped) { - return jsonError(res, mapped.status, mapped.message); + return jsonBrowserError(res, mapped); } const browserMapped = toBrowserErrorResponse(err); if (browserMapped) { - return jsonError(res, browserMapped.status, browserMapped.message); + return jsonBrowserError(res, browserMapped); } jsonError(res, 500, String(err)); } diff --git a/extensions/browser/src/browser/routes/basic.existing-session.test.ts b/extensions/browser/src/browser/routes/basic.existing-session.test.ts index 1b468220efe..d3383dbccfb 100644 --- a/extensions/browser/src/browser/routes/basic.existing-session.test.ts +++ b/extensions/browser/src/browser/routes/basic.existing-session.test.ts @@ -111,8 +111,13 @@ async function callBasicRouteWithState(params: { async function callStartRoute(params: { profile?: Record; query?: Record; + error?: Error; }) { - const ensureBrowserAvailable = vi.fn(async () => {}); + const ensureBrowserAvailable = vi.fn(async () => { + if (params.error) { + throw params.error; + } + }); const profile = { name: "openclaw", driver: "openclaw", @@ -273,6 +278,34 @@ describe("basic browser routes", () => { expect(ensureBrowserAvailable).toHaveBeenCalledWith({ headless: true }); }); + it("returns structured no-display metadata without replacing the error message", async () => { + const { response } = await callStartRoute({ + error: new BrowserProfileUnavailableError("display required", { + metadata: { + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "profile", + displayPresent: false, + }, + }, + }), + }); + + expect(response.statusCode).toBe(409); + expect(response.body).toEqual({ + error: "display required", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "profile", + displayPresent: false, + }, + }); + }); + it("rejects invalid start headless values", async () => { const { response, ensureBrowserAvailable } = await callStartRoute({ query: { headless: "maybe" }, diff --git a/extensions/browser/src/browser/routes/basic.ts b/extensions/browser/src/browser/routes/basic.ts index cbb985822c4..faa8559fdf5 100644 --- a/extensions/browser/src/browser/routes/basic.ts +++ b/extensions/browser/src/browser/routes/basic.ts @@ -19,6 +19,7 @@ import type { BrowserRequest, BrowserResponse, BrowserRouteRegistrar } from "./t import { asyncBrowserRoute, getProfileContext, + jsonBrowserError, jsonError, toBoolean, toStringOrEmpty, @@ -53,7 +54,7 @@ async function probeChromeMcpPageReady(profileCtx: ProfileContext, timeoutMs: nu function handleBrowserRouteError(res: BrowserResponse, err: unknown) { const mapped = toBrowserErrorResponse(err); if (mapped) { - return jsonError(res, mapped.status, mapped.message); + return jsonBrowserError(res, mapped); } jsonError(res, 500, String(err)); } diff --git a/extensions/browser/src/browser/routes/permissions.test.ts b/extensions/browser/src/browser/routes/permissions.test.ts index 16cccaba807..46002220ac9 100644 --- a/extensions/browser/src/browser/routes/permissions.test.ts +++ b/extensions/browser/src/browser/routes/permissions.test.ts @@ -1,5 +1,6 @@ // Browser tests cover permissions plugin behavior. import { beforeEach, describe, expect, it, vi } from "vitest"; +import { BROWSER_ERROR_REASONS, BrowserProfileUnavailableError } from "../errors.js"; import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helpers.js"; const cdpMocks = vi.hoisted(() => ({ @@ -83,10 +84,14 @@ async function callGrant( options: { profile?: Record; ssrfPolicy?: Record; + ensureBrowserAvailable?: () => Promise; } = {}, ) { const { app, postHandlers } = createBrowserRouteApp(); const profileCtx = createProfileContext(options.profile); + if (options.ensureBrowserAvailable) { + profileCtx.ensureBrowserAvailable = vi.fn(options.ensureBrowserAvailable); + } registerBrowserPermissionRoutes(app, createRouteContext(profileCtx, options.ssrfPolicy) as never); const handler = postHandlers.get("/permissions/grant"); expect(handler).toBeTypeOf("function"); @@ -167,6 +172,47 @@ describe("browser permission routes", () => { }); }); + it("preserves structured browser availability errors", async () => { + const error = new BrowserProfileUnavailableError( + 'Managed browser profile "openclaw" requires a display.', + { + metadata: { + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }, + }, + ); + const { response } = await callGrant( + { + origin: "https://meet.google.com", + permissions: ["audioCapture"], + }, + { + ensureBrowserAvailable: async () => { + throw error; + }, + }, + ); + + expect(response.statusCode).toBe(409); + expect(response.body).toStrictEqual({ + error: error.message, + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }); + expect(cdpMocks.getChromeWebSocketUrl).not.toHaveBeenCalled(); + }); + it("rejects loose timeoutMs values before granting permissions", async () => { const { response, profileCtx } = await callGrant({ origin: "https://meet.google.com", diff --git a/extensions/browser/src/browser/routes/permissions.ts b/extensions/browser/src/browser/routes/permissions.ts index 8a4a5c770db..e296f28682d 100644 --- a/extensions/browser/src/browser/routes/permissions.ts +++ b/extensions/browser/src/browser/routes/permissions.ts @@ -10,12 +10,19 @@ import type { SsrFPolicy } from "../../infra/net/ssrf.js"; import { resolveCdpControlPolicy } from "../cdp-reachability-policy.js"; import { withCdpSocket } from "../cdp.helpers.js"; import { getChromeWebSocketUrl } from "../chrome.js"; +import { toBrowserErrorResponse } from "../errors.js"; import { getPwAiModule } from "../pw-ai-module.js"; import type { BrowserRouteContext } from "../server-context.js"; import type { ProfileContext } from "../server-context.js"; import { readRouteTimerTimeoutMs } from "./route-numeric.js"; import type { BrowserRouteRegistrar } from "./types.js"; -import { asyncBrowserRoute, getProfileContext, jsonError, toStringOrEmpty } from "./utils.js"; +import { + asyncBrowserRoute, + getProfileContext, + jsonBrowserError, + jsonError, + toStringOrEmpty, +} from "./utils.js"; const permissionRouteDeps = { getPwAiModule, @@ -199,6 +206,10 @@ export function registerBrowserPermissionRoutes( }); return res.json({ ok: true, origin, ...granted }); } catch (error) { + const mapped = toBrowserErrorResponse(error); + if (mapped) { + return jsonBrowserError(res, mapped); + } return jsonError(res, 500, error instanceof Error ? error.message : String(error)); } }), diff --git a/extensions/browser/src/browser/routes/tabs.ts b/extensions/browser/src/browser/routes/tabs.ts index f5a94e58e40..23850651204 100644 --- a/extensions/browser/src/browser/routes/tabs.ts +++ b/extensions/browser/src/browser/routes/tabs.ts @@ -21,7 +21,7 @@ import { resolveTargetIdFromTabs } from "../target-id.js"; import { browserNavigationPolicyForProfile, resolveProfileContext } from "./agent.shared.js"; import { readRouteNonNegativeInteger } from "./route-numeric.js"; import type { BrowserRequest, BrowserResponse, BrowserRouteRegistrar } from "./types.js"; -import { asyncBrowserRoute, jsonError, toStringOrEmpty } from "./utils.js"; +import { asyncBrowserRoute, jsonBrowserError, jsonError, toStringOrEmpty } from "./utils.js"; const DEFAULT_TAB_REACHABILITY_TIMEOUT_MS = 300; const TAB_REACHABILITY_RETRY_DELAY_MS = 250; @@ -35,7 +35,7 @@ function handleTabsRouteError( if (opts?.mapTabError) { const mapped = ctx.mapTabError(err); if (mapped) { - return jsonError(res, mapped.status, mapped.message); + return jsonBrowserError(res, mapped); } } return jsonError(res, 500, String(err)); diff --git a/extensions/browser/src/browser/routes/utils.ts b/extensions/browser/src/browser/routes/utils.ts index e448bb9ff5b..16e1b5c1c77 100644 --- a/extensions/browser/src/browser/routes/utils.ts +++ b/extensions/browser/src/browser/routes/utils.ts @@ -4,6 +4,7 @@ * Wraps async handlers, profile lookup, JSON errors, and route value coercion * shared across browser control endpoints. */ +import type { BrowserErrorResponse } from "../errors.js"; import type { BrowserRouteContext, ProfileContext } from "../server-context.js"; import type { BrowserRequest, BrowserResponse, BrowserRouteHandler } from "./types.js"; @@ -52,6 +53,15 @@ export function jsonError(res: BrowserResponse, status: number, message: string) res.status(status).json({ error: message }); } +/** Send a mapped browser-domain error while preserving validated metadata. */ +export function jsonBrowserError(res: BrowserResponse, error: BrowserErrorResponse) { + const body = + "reason" in error + ? { error: error.message, reason: error.reason, details: error.details } + : { error: error.message }; + res.status(error.status).json(body); +} + /** Coerce route values to strings while treating nullish values as empty. */ export function toStringOrEmpty(value: unknown) { if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { diff --git a/extensions/browser/src/browser/server-context.availability.ts b/extensions/browser/src/browser/server-context.availability.ts index c835cd52340..6a61bc8f86f 100644 --- a/extensions/browser/src/browser/server-context.availability.ts +++ b/extensions/browser/src/browser/server-context.availability.ts @@ -22,7 +22,7 @@ import { stopOpenClawChrome, } from "./chrome.js"; import type { ResolvedBrowserProfile } from "./config.js"; -import { BrowserProfileUnavailableError } from "./errors.js"; +import { BROWSER_ERROR_REASONS, BrowserProfileUnavailableError } from "./errors.js"; import { getExtensionRelayModule } from "./extension-relay.runtime.js"; import { getBrowserProfileCapabilities } from "./profile-capabilities.js"; import { @@ -349,7 +349,16 @@ export function createProfileAvailability({ try { return await launchOpenClawChrome(current.resolved, profile, launchOptions); } catch (err) { - recordManagedLaunchFailure(profileState, err); + // Missing-display rejection happens before a process launch. Do not let + // repeated headed requests block the explicit headless recovery path. + if ( + !( + err instanceof BrowserProfileUnavailableError && + err.metadata?.reason === BROWSER_ERROR_REASONS.noDisplayForHeadedProfile + ) + ) { + recordManagedLaunchFailure(profileState, err); + } throw err; } }; diff --git a/extensions/browser/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts b/extensions/browser/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts index e0db377b473..6f24e5014b7 100644 --- a/extensions/browser/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts +++ b/extensions/browser/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts @@ -5,7 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import "./server-context.chrome-test-harness.js"; import { PROFILE_ATTACH_RETRY_TIMEOUT_MS } from "./cdp-timeouts.js"; import * as chromeModule from "./chrome.js"; -import { BrowserProfileUnavailableError } from "./errors.js"; +import { BROWSER_ERROR_REASONS, BrowserProfileUnavailableError } from "./errors.js"; import { createBrowserRouteContext } from "./server-context.js"; import { makeBrowserServerState, mockLaunchedChrome } from "./server-context.test-harness.js"; @@ -255,6 +255,43 @@ describe("browser server-context ensureBrowserAvailable", () => { expect(stopOpenClawChrome).not.toHaveBeenCalled(); }); + it("does not let no-display preflight failures block explicit headless recovery", async () => { + const { launchOpenClawChrome, stopOpenClawChrome, isChromeCdpReady, state } = + setupEnsureBrowserAvailableHarness(); + isChromeCdpReady.mockResolvedValue(true); + launchOpenClawChrome.mockRejectedValue( + new BrowserProfileUnavailableError("display required", { + metadata: { + reason: BROWSER_ERROR_REASONS.noDisplayForHeadedProfile, + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }, + }), + ); + + for (let attempt = 0; attempt < 3; attempt += 1) { + const ctx = createBrowserRouteContext({ getState: () => state }); + await expect(ctx.forProfile("openclaw").ensureBrowserAvailable()).rejects.toThrow( + "display required", + ); + } + + mockLaunchedChrome(launchOpenClawChrome, 987); + const recoveryCtx = createBrowserRouteContext({ getState: () => state }); + const recovery = recoveryCtx.forProfile("openclaw").ensureBrowserAvailable({ headless: true }); + await vi.advanceTimersByTimeAsync(100); + await expect(recovery).resolves.toBeUndefined(); + + expect(launchOpenClawChrome).toHaveBeenCalledTimes(4); + expect(launchOpenClawChrome.mock.calls.at(-1)?.[2]).toEqual({ headlessOverride: true }); + expect(state.profiles.get("openclaw")?.managedLaunchFailure).toBeUndefined(); + expect(stopOpenClawChrome).not.toHaveBeenCalled(); + }); + it("allows one managed Chrome launch attempt after the cooldown expires", async () => { const { launchOpenClawChrome, isChromeCdpReady, state } = setupEnsureBrowserAvailableHarness(); isChromeCdpReady.mockResolvedValue(true); diff --git a/extensions/browser/src/browser/server-context.types.ts b/extensions/browser/src/browser/server-context.types.ts index b901695f269..5a823110615 100644 --- a/extensions/browser/src/browser/server-context.types.ts +++ b/extensions/browser/src/browser/server-context.types.ts @@ -6,6 +6,7 @@ import type { Server } from "node:http"; import type { RunningChrome } from "./chrome.js"; import type { BrowserTab, BrowserTransport } from "./client.types.js"; import type { ResolvedBrowserConfig, ResolvedBrowserProfile } from "./config.js"; +import type { BrowserErrorResponse } from "./errors.js"; import type { ExtensionRelayHandle } from "./extension-relay/relay-server.js"; export type { BrowserTab }; @@ -83,7 +84,7 @@ export type BrowserRouteContext = { forProfile: (profileName?: string) => ProfileContext; listProfiles: () => Promise; // Legacy methods delegate to default profile for backward compatibility - mapTabError: (err: unknown) => { status: number; message: string } | null; + mapTabError: (err: unknown) => BrowserErrorResponse | null; } & BrowserProfileActions; /** Operations scoped to a single resolved Browser profile. */ diff --git a/extensions/browser/src/gateway/browser-request.profile-from-body.test.ts b/extensions/browser/src/gateway/browser-request.profile-from-body.test.ts index 0d69fbd9a42..00dff42053c 100644 --- a/extensions/browser/src/gateway/browser-request.profile-from-body.test.ts +++ b/extensions/browser/src/gateway/browser-request.profile-from-body.test.ts @@ -31,15 +31,12 @@ vi.mock("../sdk-node-runtime.js", async () => { import { browserHandlers } from "./browser-request.js"; -type RespondCall = [boolean, unknown?, { code: number; message: string }?]; +type RespondCall = [boolean, unknown?, { code: string; message: string; details?: unknown }?]; -function createContext() { - const invoke = vi.fn(async () => ({ - ok: true, - payload: { - result: { ok: true }, - }, - })); +function createContext(invokeResult?: unknown) { + const invoke = vi.fn(async () => + invokeResult === undefined ? { ok: true, payload: { result: { ok: true } } } : invokeResult, + ); const listConnected = vi.fn(() => [ { nodeId: "node-1", @@ -54,9 +51,9 @@ function createContext() { }; } -async function runBrowserRequest(params: Record) { +async function runBrowserRequest(params: Record, invokeResult?: unknown) { const respond = vi.fn(); - const nodeRegistry = createContext(); + const nodeRegistry = createContext(invokeResult); await browserHandlers["browser.request"]({ params, respond: respond as never, @@ -103,6 +100,7 @@ describe("browser.request profile selection", () => { const invoke = invokeParams(nodeRegistry); expect(invoke.command).toBe("browser.proxy"); expect(invoke.params?.profile).toBe("work"); + expect(invoke.params?.errorEnvelope).toBe("browser-v1"); expect(firstRespondCall(respond)[0]).toBe(true); }); @@ -174,4 +172,30 @@ describe("browser.request profile selection", () => { expect(invoke.params?.path).toBe("/profiles"); expect(firstRespondCall(respond)[0]).toBe(true); }); + + it("maps validated node-proxy route failures like local route failures", async () => { + const errorBody = { + error: "headed mode needs a display", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }; + const { respond } = await runBrowserRequest( + { method: "POST", path: "/start" }, + { ok: true, payload: { error: { status: 409, body: errorBody } } }, + ); + + const [ok, payload, error] = firstRespondCall(respond); + expect(ok).toBe(false); + expect(payload).toBeUndefined(); + expect(error).toMatchObject({ + code: "INVALID_REQUEST", + message: "headed mode needs a display", + details: errorBody, + }); + }); }); diff --git a/extensions/browser/src/gateway/browser-request.ts b/extensions/browser/src/gateway/browser-request.ts index b8b5d963275..7b974dd1d7f 100644 --- a/extensions/browser/src/gateway/browser-request.ts +++ b/extensions/browser/src/gateway/browser-request.ts @@ -8,6 +8,13 @@ import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { + BROWSER_PROXY_ERROR_ENVELOPE, + parseBrowserProxyFailure, + type BrowserProxyEnvelope, + type BrowserProxyFile, + type BrowserProxySuccess, +} from "../browser-proxy-envelope.js"; import { ErrorCodes, applyBrowserProxyPaths, @@ -37,17 +44,6 @@ type BrowserRequestParams = { timeoutMs?: number; }; -type BrowserProxyFile = { - path: string; - base64: string; - mimeType?: string; -}; - -type BrowserProxyResult = { - result: unknown; - files?: BrowserProxyFile[]; -}; - function isBrowserNode(node: NodeSession) { const caps = Array.isArray(node.caps) ? node.caps : []; const commands = Array.isArray(node.commands) ? node.commands : []; @@ -214,6 +210,7 @@ export async function handleBrowserGatewayRequest({ body, timeoutMs, profile: resolveRequestedBrowserProfile({ query, body }), + errorEnvelope: BROWSER_PROXY_ERROR_ENVELOPE, }; const res = await context.nodeRegistry.invoke({ nodeId: nodeTarget.nodeId, @@ -226,14 +223,22 @@ export async function handleBrowserGatewayRequest({ return; } const payload = res.payloadJSON ? safeParseJson(res.payloadJSON) : res.payload; - const proxy = payload && typeof payload === "object" ? (payload as BrowserProxyResult) : null; + const failure = parseBrowserProxyFailure(payload); + if (failure) { + const { status, body: errorBody } = failure.error; + const code = status >= 500 ? ErrorCodes.UNAVAILABLE : ErrorCodes.INVALID_REQUEST; + respond(false, undefined, errorShape(code, errorBody.error, { details: errorBody })); + return; + } + const proxy = payload && typeof payload === "object" ? (payload as BrowserProxyEnvelope) : null; if (!proxy || !("result" in proxy)) { respond(false, undefined, errorShape(ErrorCodes.UNAVAILABLE, "browser proxy failed")); return; } - const mapping = await persistProxyFiles(proxy.files); - applyProxyPaths(proxy.result, mapping); - respond(true, proxy.result); + const success = proxy as BrowserProxySuccess; + const mapping = await persistProxyFiles(success.files); + applyProxyPaths(success.result, mapping); + respond(true, success.result); return; } diff --git a/extensions/browser/src/node-host/invoke-browser.test.ts b/extensions/browser/src/node-host/invoke-browser.test.ts index f6b0005f162..40bf1d7f381 100644 --- a/extensions/browser/src/node-host/invoke-browser.test.ts +++ b/extensions/browser/src/node-host/invoke-browser.test.ts @@ -297,7 +297,7 @@ describe("runBrowserProxyCommand", () => { expected: "404: tab not found", }, { status: 503, body: { error: "" }, expected: "HTTP 503" }, - ])("preserves browser response status in errors: $expected", async (response) => { + ])("preserves legacy node errors without envelope opt-in: $expected", async (response) => { dispatcherMocks.dispatch.mockResolvedValue(response); await expect( @@ -312,6 +312,51 @@ describe("runBrowserProxyCommand", () => { ).rejects.toThrow(response.expected); }); + it("preserves only validated browser error metadata in the proxy envelope", async () => { + dispatcherMocks.dispatch.mockResolvedValue({ + status: 409, + body: { + error: "headed mode needs a display", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + remediation: "untrusted", + }, + untrusted: "drop me", + }, + }); + + const result = JSON.parse( + await runBrowserProxyCommand( + JSON.stringify({ + method: "POST", + path: "/start", + profile: "openclaw", + errorEnvelope: "browser-v1", + }), + ), + ); + + expect(result).toEqual({ + error: { + status: 409, + body: { + error: "headed mode needs a display", + reason: "no_display_for_headed_profile", + details: { + profile: "openclaw", + requestedHeadless: false, + headlessSource: "config", + displayPresent: false, + }, + }, + }, + }); + }); + it("rejects unauthorized query.profile when allowProfiles is configured", async () => { configMocks.loadConfig.mockReturnValue({ browser: {}, diff --git a/extensions/browser/src/node-host/invoke-browser.ts b/extensions/browser/src/node-host/invoke-browser.ts index 2a3a2d38b27..41fbfc3c01a 100644 --- a/extensions/browser/src/node-host/invoke-browser.ts +++ b/extensions/browser/src/node-host/invoke-browser.ts @@ -5,6 +5,12 @@ import fsPromises from "node:fs/promises"; import { resolveTimerTimeoutMs } from "openclaw/plugin-sdk/number-runtime"; import { normalizeStringEntries } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { + BROWSER_PROXY_ERROR_ENVELOPE, + createBrowserProxyFailure, + type BrowserProxyEnvelope, + type BrowserProxyFile, +} from "../browser-proxy-envelope.js"; import { redactCdpUrl } from "../browser/cdp.helpers.js"; import { loadBrowserConfigForRuntimeRefresh } from "../browser/config-refresh-source.js"; import { resolveBrowserConfig } from "../browser/config.js"; @@ -28,17 +34,7 @@ type BrowserProxyParams = { body?: unknown; timeoutMs?: number; profile?: string; -}; - -type BrowserProxyFile = { - path: string; - base64: string; - mimeType?: string; -}; - -type BrowserProxyResult = { - result: unknown; - files?: BrowserProxyFile[]; + errorEnvelope?: unknown; }; const BROWSER_PROXY_MAX_FILE_BYTES = 10 * 1024 * 1024; @@ -317,8 +313,11 @@ export async function runBrowserProxyCommand(paramsJSON?: string | null): Promis ); } if (response.status >= 400) { - // node.invoke preserves only Error.message; keep the status there so gateway - // retry classifiers see the same error shape as direct browser requests. + if (params.errorEnvelope === BROWSER_PROXY_ERROR_ENVELOPE) { + // New callers opt into the closed envelope; older Gateways retain the + // shipped status-prefixed node error during rolling upgrades. + return JSON.stringify(createBrowserProxyFailure(response.status, response.body)); + } const detail = response.body && typeof response.body === "object" && "error" in response.body ? String((response.body as { error?: unknown }).error).trim() @@ -363,6 +362,6 @@ export async function runBrowserProxyCommand(paramsJSON?: string | null): Promis } } - const payload: BrowserProxyResult = files ? { result, files } : { result }; + const payload: BrowserProxyEnvelope = files ? { result, files } : { result }; return JSON.stringify(payload); }