fix: flaky navigation timeouts caused by a lost wakeup in WaitForLoadState and WaitForURL

[myrjola]

Thank you for this awesome project! Here's a fix to a flaky navigation timeout caused by a subtle concurrency bug in the client which causes WaitForURL, ExpectNavigation and Goto to occasionally fail even when the page is already at the wanted URL and fully loaded. This failure became quite common in the CI pipelines of my projects but they have been running fine after taking the fix into use. I published a minimal reproduction here https://github.com/myrjola/playwright-go-lost-wakeup. I've verified the reproduction and fix on my Linux server and Macbook.

I did not find a prior issue for this; happy to open one if you want it tracked separately. I'm also happy to put in the work to to improve this PR.

Click to open sequence diagrams for understanding the bug and fix.

sequenceDiagram
    participant W as Wait goroutine<br/>(waitForLoadStateImpl)
    participant F as frameImpl<br/>(loadStates, listeners)
    participant D as Dispatch goroutine<br/>(connection)

    W->>F: read loadStates
    F-->>W: "load" absent
    W->>F: setNavigationWaiter(): start timeout timer,<br/>register close / crash / framedetached listeners

    rect rgba(220, 38, 38, 0.15)
        Note over F,D: gap: nobody is subscribed to "loadstate" yet
        D->>F: onLoadState("load")
        Note over F: loadStates += "load"
        Note over F: emit "loadstate" to zero listeners,<br/>never replayed
    end

    W->>F: On("loadstate", handler)
    Note over W: nothing re-reads loadStates
    W->>W: block on errChan
    W-->>W: ErrTimeout after the full timeout,<br/>page is at the URL and fully loaded

Loading

And the same interleaving after the fix:

sequenceDiagram
    participant W as Wait goroutine<br/>(waitForLoadStateImpl)
    participant F as frameImpl<br/>(loadStates, listeners)
    participant D as Dispatch goroutine<br/>(connection)

    W->>F: read loadStates
    F-->>W: "load" absent
    W->>W: setNavigationWaiter()
    D->>F: onLoadState("load"): loadStates += "load",<br/>emit to zero listeners
    W->>F: On("loadstate", handler)
    W->>F: re-check loadStates
    F-->>W: "load" present
    W->>W: waiter.dispose(): stop timer, remove listeners
    W-->>W: return nil

Loading

Click to open AI generated report below with more details.

It shows up as a rare, load-correlated timeout whose failure screenshot shows the right page. One instance from the reproduction below, with `framenavigated` and `load` listeners attached before the click:

iter 3 LOST: wait returned after 3.000555759s; framenavigated(http://127.0.0.1:41031/a) at -2.482707ms after the call; load at 262.241µs after the call; page.URL() now http://127.0.0.1:41031/a; err: timeout:Timeout 3000.00ms exceeded.

Cause. waitForLoadStateImpl reads f.loadStates, then builds its waiter (setNavigationWaiter: a timer goroutine and four listener registrations), and only then subscribes to "loadstate". Events arrive on the connection's single dispatch goroutine, which can run between that read and the subscription. A loadstate delivered in the gap is emitted to no listener and is never replayed; loadStates is updated but nothing re-reads it, so the wait sleeps out its whole timeout. WaitForURL has the same shape around "navigated": it checks f.URL(), then calls ExpectNavigation, which subscribes several registrations later.

Upstream. client/frame.ts waitForLoadState builds the waiter first, checks _loadStates, and calls waiter.dispose() when the state is already there; run-to-completion makes that check atomic with the subscription. The Go port checks first and builds the waiter afterwards, and the dispatch goroutine can interleave.

Fix. Bring the port to upstream's shape:

• waiter.dispose() (new, after upstream's Waiter.dispose()): marks the waiter fulfilled so a racing handler drops its event, cancels the timeout goroutine, removes every listener.
• waitForLoadStateImpl: keep the cheap first check (the common already-loaded case builds no waiter), subscribe, re-check loadStates, dispose on a hit. onLoadState adds to the set before emitting, so a re-check after subscribing cannot miss an event that fired before it.
• WaitForURL: the not-yet-matching path goes through an internal expectNavigation(cb, alreadyNavigated, ...) that re-checks the URL once the "navigated" listener is attached. ExpectNavigation passes nil and is unchanged. On a hit only the load state is awaited and no Response is returned, which WaitForURL discards anyway.

Evidence. Reproduction with both runs' full output: https://github.com/myrjola/playwright-go-lost-wakeup. Six Chromium pages click between two pages whose stylesheet and script make load trail the commit by a few ms; a timeout is LOST if the page is at the wanted URL and load was delivered well inside the budget, otherwise slow. Same box, same knobs, -race:

Library	ok	LOST	slow	load avg during run
v0.6201.1	1794	6	0	~7
this branch	1800	0	0	~50

Every loss has the same shape: the navigation committed a few ms before the call, load arrived 0.2 to 0.4 ms after it, and the wait ran its full 3 s.

Tests.

• frame_wait_race_test.go (new, no browser): a frameImpl from the existing newTimeoutSemanticsFixture; a goroutine fires onLoadState / onFrameNavigated the moment the wait's "framedetached" rejection appears on the page, which is the registration just before the wait subscribes to its own event. Both tests fail within a few rounds on v0.6201.1 and pass here; they raise GOMAXPROCS to 2 when they find 1, since at 1 the waiting goroutine never yields before it subscribes, and they check no listener is left on the frame or page.
• TestWaiterDisposeRemovesEveryListener (new).
• Root package under -race, and the full tests/ suite on Chromium under -race: pass. golangci-lint with the repo config: 0 issues. gofumpt clean. Driver version untouched.

Written with Claude (Fable 5.1) in Claude Code; the measurements are from real runs on my machine.