Skip to content

fix: fail a request when the Playwright connection closes - #277

Merged
MrPunyapal merged 2 commits into
pestphp:5.xfrom
joshdaugherty:fix/closed-connection-spin
Oct 25, 2026
Merged

MrPunyapal merged 2 commits into
pestphp:5.xfrom
joshdaugherty:fix/closed-connection-spin

Conversation

@joshdaugherty

Copy link
Copy Markdown
Contributor

What happens

When the Playwright server's websocket closes (the server is killed, or crashes), WebsocketConnection::receive() returns null. Client::fetch() casts that to an empty string:

return (string) $client->receive()?->read();

An empty response has neither an error nor an id, so execute() never takes its error branch or its exit, and asks the closed connection for another response forever, at full CPU. Reported as pestphp/pest#1801.

The fix

fetch() throws a RuntimeException naming the closed connection when receive() returns no message. A normal response is read as before.

This is one half of #241, credited as prior art. That pull request was closed by its author for lack of activity, not rejected. Its metadata-timeout half has since landed on 5.x as e84a76d, in v5.0.1. The rest of its remaining half, a per-request TimeoutCancellation plus a wall-clock deadline, is left out on purpose. In a suite where the browser, the in-process HTTP server and the test share one event loop, a burst of server work can delay response handling past such a deadline with nothing wrong. It was measured failing a passing control-panel smoke test at 144.3s against 102.6s without it. The closed-connection check has no deadline, so it cannot fire that way.

Tests

tests/Unit/Playwright/ClientTest.php, with a stubbed websocket connection:

  • A connection that returns no message fails with the closed-connection message.
  • A request whose response carries the request's own id still completes, with its result returned.

Against unchanged 5.x source, the first test never finishes. It calls receive() until PHP runs out of memory at 2 GB, which is the same loop pestphp/pest#1801 describes at full CPU.

The full suite on this branch passes 341 tests with 27 skipped, including the 2 new ones. Three tests fail: two iframe tests and "it may visit external URLs". All three load external URLs and fail the same way on unchanged 5.x on this machine. PHPStan, type coverage (100%), Rector and Pint on the changed files are clean. Run on Windows 11 with PHP 8.4 and Playwright 1.62.1.

pestphp/pest#1781

Not changed here, because it no longer reproduces on 5.x. With Playwright 1.62.1, a click on a locator that matches nothing fails with Timeout 2000ms exceeded after 2.4 to 2.8s, in 3 of 3 runs, with the suite's setTimeout(2000). That is e84a76d's metadata timeout at work, so #1781 can likely be closed as fixed by it.

Closes pestphp/pest#1801.

Once the websocket closes, receive() returns null. fetch() cast that to
an empty string, and a response with neither an `error` nor an `id`
never ends execute()'s loop, so it kept asking a closed connection for
a response at full CPU instead of failing.

fetch() now throws when there is no message, naming the closed
connection. A normal response is unchanged.
@MrPunyapal
MrPunyapal merged commit 6d92b09 into pestphp:5.x Oct 25, 2026
3 of 4 checks passed
@MrPunyapal

Copy link
Copy Markdown
Member

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Browser plugin — Client::execute() spins at 100% CPU forever when the Playwright connection closes

2 participants