From a5b796f6753c26af62e9b93caedf91fddfbb2d12 Mon Sep 17 00:00:00 2001 From: Les Orchard Date: Thu, 4 Jun 2026 11:59:37 -0700 Subject: [PATCH] fix(core): bound the domcontentloaded wait in ensureOptimizedPageLoad MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit waitForLoadState("domcontentloaded") was called with no timeout, while the "load" wait right below it is bounded by actionTimeoutMs. A navigation that commits to a page (or SPA soft-navigation) that never fires DOMContentLoaded as Playwright tracks it could hang this await indefinitely — the same class of freeze fixed in #511, on the path that runs after every navigation. Pass { timeout: this.actionTimeoutMs } to the domcontentloaded wait. It is already wrapped in a try/catch that continues on failure, so a timeout simply proceeds to the bounded "load" wait and the settle delay; no behavior change on healthy pages. Closes #514 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/browser/playwrightBrowser.ts | 9 +++++++-- packages/core/test/playwrightBrowser.test.ts | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/core/src/browser/playwrightBrowser.ts b/packages/core/src/browser/playwrightBrowser.ts index daffab40..8bb49cca 100644 --- a/packages/core/src/browser/playwrightBrowser.ts +++ b/packages/core/src/browser/playwrightBrowser.ts @@ -590,8 +590,13 @@ export class PlaywrightBrowser implements AriaBrowser { if (!this.page) throw new Error("Browser not started"); try { - // 1. Wait for DOM to be ready - this is critical for interactivity - await this.page.waitForLoadState("domcontentloaded"); + // 1. Wait for DOM to be ready - this is critical for interactivity. + // Bounded by actionTimeoutMs: a page (or SPA soft-nav) that never reaches + // domcontentloaded must not hang the agent. On timeout we continue, since the + // page may still be interactive and the load wait + settle below still run. + await this.page.waitForLoadState("domcontentloaded", { + timeout: this.actionTimeoutMs, + }); } catch (error) { // Still continue since we might be able to interact with what's loaded } diff --git a/packages/core/test/playwrightBrowser.test.ts b/packages/core/test/playwrightBrowser.test.ts index 08ba15eb..97444db1 100644 --- a/packages/core/test/playwrightBrowser.test.ts +++ b/packages/core/test/playwrightBrowser.test.ts @@ -1340,4 +1340,20 @@ describe("PlaywrightBrowser", () => { expect(error).not.toBeInstanceOf(BrowserDisconnectedError); }); }); + + describe("ensureOptimizedPageLoad", () => { + it("bounds the domcontentloaded wait with actionTimeoutMs so it can't hang indefinitely", async () => { + const browser = new PlaywrightBrowser({ browser: "chromium", actionTimeoutMs: 1234 }); + const waitForLoadState = vi.fn().mockResolvedValue(undefined); + (browser as any).page = { + waitForLoadState, + waitForTimeout: vi.fn().mockResolvedValue(undefined), + }; + + await (browser as any).ensureOptimizedPageLoad(); + + // The domcontentloaded wait must carry a timeout (previously unbounded). + expect(waitForLoadState).toHaveBeenCalledWith("domcontentloaded", { timeout: 1234 }); + }); + }); });