fix(browser): keep upload errors specific (#101465)

This commit is contained in:
Peter Steinberger 2026-07-07 12:35:27 +01:00 committed by GitHub
parent fee997c8ad
commit bd7da9decd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 84 additions and 13 deletions

View file

@ -223,6 +223,7 @@ openclaw browser select 9 OptionA OptionB
openclaw browser download e12 report.pdf
openclaw browser waitfordownload report.pdf
openclaw browser upload /tmp/openclaw/uploads/file.pdf
openclaw browser upload /tmp/openclaw/uploads/file.pdf --ref e12
openclaw browser upload media://inbound/file.pdf
openclaw browser fill --fields '[{"ref":"1","type":"text","value":"Ada"}]'
openclaw browser dialog --accept
@ -269,7 +270,7 @@ Notes:
download URL, suggested filename, and guarded local path. Explicit download
interception is available for managed Playwright profiles; existing-session
profiles return an unsupported-operation error.
- `upload` and `dialog` are **arming** calls; run them before the click/press that triggers the chooser/dialog. If an action opens a modal, the action response includes `blockedByDialog` and `browserState.dialogs.pending`; pass that `dialogId` to respond directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`.
- Prefer atomic chooser uploads: pass the trigger `--ref` with the upload so OpenClaw arms and clicks in one request. Paths-only `upload` remains supported when a later trigger is intentional. Use `--input-ref` or `--element` to set a file input directly. `dialog` is an arming call; run it before the click/press that triggers the dialog. If an action opens a modal, the action response includes `blockedByDialog` and `browserState.dialogs.pending`; pass that `dialogId` to respond directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`.
- `click`/`type`/etc require a `ref` from `snapshot` (numeric `12`, role ref `e12`, or actionable ARIA ref `ax12`). CSS selectors are intentionally not supported for actions. Use `click-coords` when the visible viewport position is the only reliable target.
- Download and trace paths are constrained to OpenClaw temp roots: `/tmp/openclaw{,/downloads}` (fallback: `${os.tmpdir()}/openclaw/...`).
- `upload` accepts files from the OpenClaw temp uploads root and

View file

@ -507,6 +507,8 @@ describe("browser tool description", () => {
expect(tool.description).toContain("omit timeoutMs on act:type");
expect(tool.description).toContain("existing-session profiles");
expect(tool.description).toContain("browser-automation skill");
expect(tool.description).toContain("trigger ref with paths in the same upload call");
expect(tool.description).toContain("paths-only arming");
});
});

View file

@ -524,6 +524,7 @@ export function createBrowserTool(opts?: {
"For multi-step browser work, login checks, stale refs, duplicate tabs, or Google Meet flows, use the bundled browser-automation skill when it is available.",
'For stable, self-resolving refs across calls, use snapshot with refs="aria" (Playwright aria-ref ids). Default refs="role" are role+name-based.',
"Use snapshot+act for UI automation. Avoid act:wait by default; use only in exceptional cases when no reliable UI state exists.",
"For file chooser uploads, pass the trigger ref with paths in the same upload call when available; use paths-only arming only when a later trigger is intentional. Use inputRef or element to set a file input directly.",
`target selects browser location (sandbox|host|node). Default: ${targetDefault}.`,
hostHint,
].join(" "),

View file

@ -0,0 +1,69 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
const clientFetchMocks = vi.hoisted(() => ({
fetchBrowserJson: vi.fn(async (..._args: unknown[]) => ({ ok: true })),
}));
vi.mock("./client-fetch.js", () => clientFetchMocks);
import { browserArmDialog, browserArmFileChooser } from "./client-actions-core.js";
function lastFetchCall(): { url: string; options: { body?: string; timeoutMs?: number } } {
const call = clientFetchMocks.fetchBrowserJson.mock.calls.at(-1);
if (!call) {
throw new Error("fetchBrowserJson was not called");
}
return {
url: String(call[0]),
options: call[1] as { body?: string; timeoutMs?: number },
};
}
describe("browser hook client actions", () => {
beforeEach(() => vi.clearAllMocks());
it("adds transport slack to an atomic file chooser upload", async () => {
await browserArmFileChooser(undefined, {
paths: ["/tmp/openclaw/uploads/report.pdf"],
ref: "e12",
targetId: "tab-1",
timeoutMs: 30_000,
profile: "openclaw",
});
const call = lastFetchCall();
expect(call.url).toBe("/hooks/file-chooser?profile=openclaw");
expect(call.options.timeoutMs).toBe(35_000);
expect(JSON.parse(call.options.body ?? "{}")).toEqual({
paths: ["/tmp/openclaw/uploads/report.pdf"],
ref: "e12",
targetId: "tab-1",
timeoutMs: 30_000,
});
});
it("preserves paths-only arming with the advertised 120 second default", async () => {
await browserArmFileChooser(undefined, {
paths: ["/tmp/openclaw/uploads/report.pdf"],
});
const call = lastFetchCall();
expect(call.url).toBe("/hooks/file-chooser");
expect(call.options.timeoutMs).toBe(125_000);
expect(JSON.parse(call.options.body ?? "{}")).toEqual({
paths: ["/tmp/openclaw/uploads/report.pdf"],
});
});
it("keeps dialog hook requests alive past their operation timeout", async () => {
await browserArmDialog(undefined, { accept: true, timeoutMs: 45_000 });
const call = lastFetchCall();
expect(call.url).toBe("/hooks/dialog");
expect(call.options.timeoutMs).toBe(50_000);
expect(JSON.parse(call.options.body ?? "{}")).toEqual({
accept: true,
timeoutMs: 45_000,
});
});
});

View file

@ -38,8 +38,7 @@ type BrowserActResponse = {
downloads?: BrowserDownloadResult[];
};
const BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS = 5_000;
const BROWSER_DOWNLOAD_REQUEST_TIMEOUT_SLACK_MS = 5_000;
const BROWSER_REQUEST_TIMEOUT_SLACK_MS = 5_000;
type BrowserDownloadActionResult = BrowserActionTabResult & { download: BrowserDownloadResult };
@ -52,24 +51,23 @@ function resolveBrowserActRequestTimeoutMs(req: BrowserActRequest): number {
const candidateTimeouts =
explicitTimeout === undefined
? [DEFAULT_BROWSER_ACTION_TIMEOUT_MS]
: [addTimerTimeoutGraceMs(explicitTimeout, BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS) ?? 1];
: [addTimerTimeoutGraceMs(explicitTimeout, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1];
if (req.kind === "wait") {
const waitDuration = normalizePositiveTimeoutMs(req.timeMs);
if (waitDuration !== undefined) {
candidateTimeouts.push(
addTimerTimeoutGraceMs(waitDuration, BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS) ?? 1,
addTimerTimeoutGraceMs(waitDuration, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1,
);
}
}
return Math.max(...candidateTimeouts);
}
function resolveBrowserDownloadRequestTimeoutMs(timeoutMs: unknown): number {
const waitTimeoutMs =
function resolveBrowserOperationRequestTimeoutMs(timeoutMs: unknown): number {
const operationTimeoutMs =
normalizePositiveTimeoutMs(timeoutMs) ?? DEFAULT_BROWSER_DOWNLOAD_TIMEOUT_MS;
// Keep the HTTP client alive after the Playwright waiter expires so its
// timeout/error response reaches the caller instead of a transport abort.
return addTimerTimeoutGraceMs(waitTimeoutMs, BROWSER_DOWNLOAD_REQUEST_TIMEOUT_SLACK_MS) ?? 1;
// Let the browser operation report its own timeout/error before the client watchdog fires.
return addTimerTimeoutGraceMs(operationTimeoutMs, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1;
}
async function postDownloadRequest(
@ -84,7 +82,7 @@ async function postDownloadRequest(
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify(body),
timeoutMs: resolveBrowserDownloadRequestTimeoutMs(timeoutMs),
timeoutMs: resolveBrowserOperationRequestTimeoutMs(timeoutMs),
});
}
@ -129,7 +127,7 @@ export async function browserArmDialog(
targetId: opts.targetId,
timeoutMs: opts.timeoutMs,
}),
timeoutMs: 20000,
timeoutMs: resolveBrowserOperationRequestTimeoutMs(opts.timeoutMs),
});
}
@ -158,7 +156,7 @@ export async function browserArmFileChooser(
targetId: opts.targetId,
timeoutMs: opts.timeoutMs,
}),
timeoutMs: 20000,
timeoutMs: resolveBrowserOperationRequestTimeoutMs(opts.timeoutMs),
});
}