diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index ad9e20ef11b3c..6537576c89a73 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -2626,7 +2626,11 @@ const make = Effect.gen(function* () { return loadError ? { ...snapshot, url: loadError.url, loadError } : snapshot; } case "click": { - const clickInput = input as PreviewAutomationClickInput; + // Time spent queued behind other work on this tab counts against the broker's deadline. + const clickInput = { + ...(input as PreviewAutomationClickInput), + timeoutMs: Math.max(1, Math.floor(deadlineMs - performance.now())), + }; await recordAction(tab, "click", () => ServerBrowserPage.click(tab.page, clickInput, pointerFor(tab)), ); diff --git a/apps/server/src/preview/ServerBrowserPage.test.ts b/apps/server/src/preview/ServerBrowserPage.test.ts index 74da1e3825716..550943312c2f1 100644 --- a/apps/server/src/preview/ServerBrowserPage.test.ts +++ b/apps/server/src/preview/ServerBrowserPage.test.ts @@ -173,10 +173,22 @@ describe("server browser element refs", () => { const hidden = await failure("button"); expect(hidden.tag).toBe("PreviewAutomationTimeoutError"); expect(hidden.message).toBe( - 'The click may have happened; check the page before retrying. "button" still matches an element after the timeout.', + 'The click may have happened; check the page before retrying. "button" still matches an element after the timeout. Playwright last saw: element is not visible.', ); }); + it("gives a late target only the click's remaining time, not a fresh timeout", async () => { + await page.setContent(``); + await expect( + ServerBrowserPage.click(page, { selector: "#save", timeoutMs: 2_000 }), + ).rejects.toThrow(); + expect(await page.locator("body").getAttribute("data-clicked")).toBeNull(); + }); + it("does not retarget a removed ref to a replacement node", async () => { await page.setContent(""); const locator = buttonLocator((await takeSnapshot()).accessibilityTree, "original"); diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index 20df4457643eb..3354554f1dd90 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -22,6 +22,7 @@ import { constVoid } from "effect/Function"; import * as Schema from "effect/Schema"; import type { CDPSession, ConsoleMessage, Frame, Locator, Page } from "playwright-core"; import * as NodeCrypto from "node:crypto"; +import * as NodeUtil from "node:util"; import { BrowserControlInterrupted } from "./SessionControl.ts"; const isBrowserControlInterrupted = Schema.is(BrowserControlInterrupted); @@ -62,7 +63,7 @@ export const toOperationError = ( if (cause instanceof Error && cause.name === "TimeoutError") { return new ServerBrowserOperationError({ tag: "PreviewAutomationTimeoutError", - message: firstLine, + message: withActionabilityReason(firstLine, cause), detail: timeoutMs === undefined ? undefined : { timeoutMs }, }); } @@ -84,6 +85,17 @@ export const toOperationError = ( }); }; +/** Playwright puts why an element was not actionable in its call log, below the first line. */ +const withActionabilityReason = (message: string, cause: Error) => { + // Playwright dims call-log lines with ANSI escapes when colour is on. + const reason = NodeUtil.stripVTControlCharacters(cause.message) + .split("\n") + .map((line) => line.trim()) + .findLast((line) => line.startsWith("- element is ")) + ?.slice(2); + return reason === undefined ? message : `${message} Playwright last saw: ${reason}.`; +}; + const DEFAULT_TIMEOUT_MS = 15_000; const TARGET_COUNT_MS = 250; const MAX_TARGET_CHARS = 200; @@ -168,11 +180,11 @@ const targetPoint = async ( page: Page, locator: Locator | null, input: { readonly x?: number | undefined; readonly y?: number | undefined }, - timeout: number, + timeout: () => number, ) => { if (locator === null) return { x: input.x ?? 0, y: input.y ?? 0 }; - await scrollIntoView(locator, timeout); - const box = await locator.boundingBox({ timeout }); + await scrollIntoView(locator, timeout()); + const box = await locator.boundingBox({ timeout: timeout() }); if (box) return { x: box.x + box.width / 2, y: box.y + box.height / 2 }; const viewport = page.viewportSize(); return { x: (viewport?.width ?? 0) / 2, y: (viewport?.height ?? 0) / 2 }; @@ -711,10 +723,12 @@ export const click = async ( input: PreviewAutomationClickInput, pointer: PointerReporter = noPointer, ): Promise<{ readonly x: number; readonly y: number }> => { - const timeout = input.timeoutMs ?? DEFAULT_TIMEOUT_MS; + // One deadline across steps keeps the reply inside the broker's grace; Playwright reads 0 as none. + const deadline = Date.now() + (input.timeoutMs ?? DEFAULT_TIMEOUT_MS); + const remaining = () => Math.max(1, deadline - Date.now()); const locator = targetLocator(page, input); try { - return await clickTarget(page, locator, input, timeout, pointer); + return await clickTarget(page, locator, input, remaining, pointer); } catch (cause) { throw locator === null ? cause : await namedTargetTimeout(locator, input, cause); } @@ -750,10 +764,11 @@ const namedTargetTimeout = async ( : matches === 0 ? `The click may have happened and removed its target or navigated; check the page before retrying. No element matches ${target} after the timeout; take a fresh snapshot and use its locator.` : `The click may have happened; check the page before retrying. ${target} still matches an element after the timeout.`; + const message = withActionabilityReason(reason, cause); return new ServerBrowserOperationError({ tag: "PreviewAutomationTimeoutError", - message: reason, - detail: { reason }, + message, + detail: { reason: message }, cause, }); }; @@ -762,7 +777,7 @@ const clickTarget = async ( page: Page, locator: Locator | null, input: PreviewAutomationClickInput, - timeout: number, + timeout: () => number, pointer: PointerReporter, ) => { const point = await targetPoint(page, locator, input, timeout); @@ -771,7 +786,7 @@ const clickTarget = async ( const clicked = locator === null ? page.mouse.click(point.x, point.y, options) - : locator.click({ ...options, timeout }); + : locator.click({ ...options, timeout: timeout() }); let onDialog = constVoid; const dialogOpened = new Promise<"dialog">((resolve) => { onDialog = () => resolve("dialog"); @@ -852,7 +867,7 @@ export const hover = async ( ) => { const timeout = input.timeoutMs ?? DEFAULT_TIMEOUT_MS; const locator = targetLocator(page, input); - await pointer(await targetPoint(page, locator, input, timeout), "move"); + await pointer(await targetPoint(page, locator, input, () => timeout), "move"); if (locator === null) { await page.mouse.move(input.x ?? 0, input.y ?? 0); return; @@ -903,7 +918,7 @@ export const drag = async ( const timeout = input.timeoutMs ?? DEFAULT_TIMEOUT_MS; const source = targetLocator(page, { locator: input.source })!; const target = targetLocator(page, { locator: input.target })!; - await pointer(await targetPoint(page, source, {}, timeout), "move"); + await pointer(await targetPoint(page, source, {}, () => timeout), "move"); // Observe drag rejection immediately; an unhandled rejection during cursor movement exits the server. // Chromium delivers a single move before dragstart; steps give the page mousemoves during the drag. const dragFailure = source.dragTo(target, { timeout, steps: 5 }).then(