Repository navigation
fix: do not retry navigations in the expectation loop - #280
Conversation
navigate(), refresh(), back() and forward() ran inside waitForExpectation(), whose one-second attempts re-sent a navigation that took longer than a second, while the first load could still be in progress: a slow page was requested twice, or the call failed with "interrupted by another navigation". Run them once, like click() and the other actions (pestphp#269); Playwright waits for the load state within the full timeout.
|
Independent reproduction from an application test suite, on v5.1.2 (the current A test that edits a page and then calls and sometimes: These are the Same machine, same run, the test body repeated 18 times under 6 workers:
Posted by Claude on behalf of @CharlieEtienne. |
|
thanks |
navigate(),refresh(),back()andforward()go throughExecution::waitForExpectation(). When the configured timeout is above 1000 ms, that loop sets the client timeout to 1000 ms for each attempt and retries everyExpectationFailedException; after the loop it makes one final call with the restored timeout.Client::execute()turns every Playwright error into that exception, including the timeout of an attempt. So when a page takes more than a second to load, the navigation is sent again while the first load may still be in progress. This PR runs these four methods once, as #269 already does forclick(),press()and the other actions.Example
On
5.x, the firstgoto('/slow')times out after one second and the loop calls it again, so/slowis served twice for onenavigate()call. In an application's CI this showed up as:Playwright raises this when a different document commits before the one its
gotois waiting for. The retry loop can issue both navigations from one call. A retriedback()is worse: in the test below it does not land within the 5 s timeout (Timeout 5000ms exceeded). With this change,/slowis served once.Why one attempt
Retrying repeats a state-changing command. A repeated
goto/reloadstarts another load, and a repeatedgoBack/goForwardcan traverse another history entry. Playwright already waits for the requested load state:goto()andreload()sendwaitUntil: 'load', andgoBack/goForwarddefault toload. That wait happens within the timeout it is given, so a single attempt with the full timeout is the right shape, as for clicks. One behaviour changes on purpose: a navigation that fails is no longer re-sent automatically.Tests
tests/Browser/Webpage/SlowNavigationTest.phpraises the timeout to 5 s inside the file and restores it afterwards. Each slow response sleeps 1.5 s; the initial loads are fast.navigate()serves the slow page once and shows it.refresh()reloads once.back()andforward()each request the page once and land on the expected history entry. The responses sendCache-Control: no-storeto avoid HTTP-cache reuse; Playwright's default Chromium launch disables the back/forward cache.Local results:
5.xsource, the three tests failed in 3 of 3 runs:Failed asserting that 2 is identical to 1;3 is identical to 2;Timeout 5000ms exceeded.composer test(exit 0): 349 passed, 27 skipped (the usual platform skips); Rector, Pint, profanity, type coverage (100 %) and PHPStan are clean.These are local observations; the 1.5 s sleep leaves headroom within the 5 s timeout, but runner scheduling can never be fully ruled out.
Not covered
withinFrame()still retries its whole callback with a one-second client timeout, so a navigation inside a frame callback can still be repeated with it. That is callback replay, a separate change.Related PRs
goto()andreload()wait for the response to their own command rather than the first matching load event. That is a separate defect inClient::execute(), in a different file. Until it lands, a navigation can still return early on an unrelated load event.withKeyDown; it does not touch navigations.🤖 Generated with Claude Code