diff --git a/packages/core/src/browser/playwrightBrowser.ts b/packages/core/src/browser/playwrightBrowser.ts index 2ef96910..daffab40 100644 --- a/packages/core/src/browser/playwrightBrowser.ts +++ b/packages/core/src/browser/playwrightBrowser.ts @@ -76,6 +76,14 @@ export interface PlaywrightBrowserOptions { navigationRetry?: Partial; /** Timeout for page load and element actions in milliseconds (default: 30000) */ actionTimeoutMs?: number; + /** + * Timeout for evaluating the aria-tree script in a single frame, in milliseconds + * (default: 5000). Bounds an otherwise unbounded Playwright `evaluate` so an + * unresponsive frame can't hang the snapshot. A child frame that exceeds it is skipped; + * a main-frame timeout surfaces as an error (the page's own document is unusable). + * Primarily a testability seam; rarely needs overriding in production. + */ + ariaFrameEvaluateTimeoutMs?: number; } export interface ExtendedPlaywrightBrowserOptions extends PlaywrightBrowserOptions { @@ -93,6 +101,30 @@ export interface ExtendedPlaywrightBrowserOptions extends PlaywrightBrowserOptio /** Timeout per CDP endpoint connection attempt in milliseconds */ const CDP_CONNECTION_TIMEOUT_MS = 5_000; +/** + * Timeout for evaluating the aria-tree script in a single frame, in milliseconds. + * + * Playwright's `evaluate` has no timeout option and is not governed by + * `setDefaultTimeout`. A hostile or unresponsive iframe (e.g. a busy ad frame whose + * main thread never yields) can leave `frame.evaluate` pending forever — neither + * resolving nor rejecting — which hangs the page snapshot and, in turn, the whole agent. + * Bounding each evaluate lets us skip such a frame instead of freezing. + */ +const ARIA_FRAME_EVALUATE_TIMEOUT_MS = 5_000; + +/** + * Race a promise against a timeout. Rejects with `Error(message)` if `ms` elapses before + * `promise` settles. The timer is always cleared, so no timer handle leaks on the happy + * path. + */ +function withTimeout(promise: Promise, ms: number, message: string): Promise { + let timer!: ReturnType; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(message)), ms); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); +} + export class PlaywrightBrowser implements AriaBrowser { public browserName: string; public channel: string | undefined; @@ -104,6 +136,9 @@ export class PlaywrightBrowser implements AriaBrowser { // Timeout for page load and element actions (configurable) private readonly actionTimeoutMs: number; + // Timeout for the aria-tree evaluate in a single frame (configurable) + private readonly ariaFrameEvaluateTimeoutMs: number; + // Navigation retry configuration private readonly navigationConfig: NavigationRetryConfig; @@ -123,6 +158,10 @@ export class PlaywrightBrowser implements AriaBrowser { // Initialize action timeout from options or use default this.actionTimeoutMs = options.actionTimeoutMs ?? getConfigDefaults().action_timeout_ms; + // Initialize per-frame aria-tree evaluate timeout from options or use default + this.ariaFrameEvaluateTimeoutMs = + options.ariaFrameEvaluateTimeoutMs ?? ARIA_FRAME_EVALUATE_TIMEOUT_MS; + // Initialize navigation retry config with defaults and overrides // Uses createNavigationRetryConfig which safely handles undefined values this.navigationConfig = createNavigationRetryConfig(options.navigationRetry); @@ -604,21 +643,27 @@ export class PlaywrightBrowser implements AriaBrowser { } private async getTreeWithRefsImpl(): Promise { - // Inject the ariaTree bundle and generate snapshot in the main frame - const mainResult = await this.page!.evaluate( - ({ script }) => { - // Idempotent injection guard - const win = window as any; - if (!win.__piloAriaTree) { - const fn = new Function(script); - fn(); - win.__piloAriaTree = (globalThis as any).__piloAriaTree; - } - const counter = { value: 0 }; - const yaml: string = win.__piloAriaTree.generateAndRenderAriaTree(document.body, counter); - return { yaml, counterValue: counter.value }; - }, - { script: ARIA_TREE_SCRIPT }, + // Inject the ariaTree bundle and generate snapshot in the main frame. + // Bounded by a timeout so an unresponsive page can't hang the snapshot forever; a + // main-frame timeout is a real failure and propagates to getTreeWithRefs's wrapper. + const mainResult = await withTimeout( + this.page!.evaluate( + ({ script }) => { + // Idempotent injection guard + const win = window as any; + if (!win.__piloAriaTree) { + const fn = new Function(script); + fn(); + win.__piloAriaTree = (globalThis as any).__piloAriaTree; + } + const counter = { value: 0 }; + const yaml: string = win.__piloAriaTree.generateAndRenderAriaTree(document.body, counter); + return { yaml, counterValue: counter.value }; + }, + { script: ARIA_TREE_SCRIPT }, + ), + this.ariaFrameEvaluateTimeoutMs, + `aria-tree main-frame evaluate timed out after ${this.ariaFrameEvaluateTimeoutMs}ms`, ); // Handle cross-origin iframes via Playwright's frame API @@ -631,29 +676,35 @@ export class PlaywrightBrowser implements AriaBrowser { if (frame === this.page!.mainFrame()) continue; try { - const frameResult = await frame.evaluate( - ({ script, counterStart }) => { - const win = window as any; - if (!win.__piloAriaTree) { - const fn = new Function(script); - fn(); - win.__piloAriaTree = (globalThis as any).__piloAriaTree; - } - const counter = { value: counterStart }; - const yaml: string = win.__piloAriaTree.generateAndRenderAriaTree( - document.body, - counter, - ); - return { yaml, counterValue: counter.value }; - }, - { script: ARIA_TREE_SCRIPT, counterStart: counter }, + // Bounded by a timeout: a busy/unresponsive ad iframe can otherwise leave + // frame.evaluate pending forever (Playwright's evaluate has no timeout). + const frameResult = await withTimeout( + frame.evaluate( + ({ script, counterStart }) => { + const win = window as any; + if (!win.__piloAriaTree) { + const fn = new Function(script); + fn(); + win.__piloAriaTree = (globalThis as any).__piloAriaTree; + } + const counter = { value: counterStart }; + const yaml: string = win.__piloAriaTree.generateAndRenderAriaTree( + document.body, + counter, + ); + return { yaml, counterValue: counter.value }; + }, + { script: ARIA_TREE_SCRIPT, counterStart: counter }, + ), + this.ariaFrameEvaluateTimeoutMs, + `aria-tree frame evaluate timed out after ${this.ariaFrameEvaluateTimeoutMs}ms`, ); if (frameResult.yaml) { childYamls.push(frameResult.yaml); counter = frameResult.counterValue; } } catch { - // Cross-origin or detached frame, skip + // Cross-origin frame, detached frame, or an evaluate that timed out — skip it. } } diff --git a/packages/core/test/playwrightBrowser.test.ts b/packages/core/test/playwrightBrowser.test.ts index e3f47d39..08ba15eb 100644 --- a/packages/core/test/playwrightBrowser.test.ts +++ b/packages/core/test/playwrightBrowser.test.ts @@ -1292,4 +1292,52 @@ describe("PlaywrightBrowser", () => { await expect(browser.getScreenshot()).rejects.not.toThrow(BrowserDisconnectedError); }); }); + + describe("getTreeWithRefs frame evaluate timeout", () => { + let browser: PlaywrightBrowser; + + beforeEach(() => { + // Tiny per-frame timeout so a non-responsive frame is skipped quickly under + // real timers (no fake-timer gymnastics around the sequential await loop). + browser = new PlaywrightBrowser({ browser: "chromium", ariaFrameEvaluateTimeoutMs: 20 }); + }); + + it("skips a child frame whose evaluate never resolves and still returns the responsive frames", async () => { + const mainFrame = { evaluate: vi.fn() }; + // Simulates a hostile/unresponsive ad iframe: evaluate never settles. + const hangingFrame = { evaluate: vi.fn().mockReturnValue(new Promise(() => {})) }; + const goodFrame = { + evaluate: vi.fn().mockResolvedValue({ yaml: "GOOD_FRAME_YAML", counterValue: 5 }), + }; + (browser as any).page = { + evaluate: vi.fn().mockResolvedValue({ yaml: "MAIN_YAML", counterValue: 1 }), + frames: vi.fn().mockReturnValue([mainFrame, hangingFrame, goodFrame]), + mainFrame: vi.fn().mockReturnValue(mainFrame), + }; + + const result = await browser.getTreeWithRefs(); + + expect(result).toContain("MAIN_YAML"); + expect(result).toContain("GOOD_FRAME_YAML"); + expect(hangingFrame.evaluate).toHaveBeenCalledTimes(1); + }); + + it("surfaces a main-frame evaluate timeout as an error (not a disconnect)", async () => { + (browser as any).page = { + evaluate: vi.fn().mockReturnValue(new Promise(() => {})), // never resolves + frames: vi.fn().mockReturnValue([]), + mainFrame: vi.fn(), + }; + + const error = await browser.getTreeWithRefs().then( + () => { + throw new Error("expected getTreeWithRefs to reject"); + }, + (e) => e, + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(/timed out/i); + expect(error).not.toBeInstanceOf(BrowserDisconnectedError); + }); + }); });