Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Client::fetch()waits for the next websocket message withreceive()and no cancellation. When the Playwright server accepts a request and never answers it, the test run hangs forever, with no output and no timeout: the only bound in the stack is the action timeout the server itself enforces, and a server that does not answer does not enforce it either.That is the alive-socket half of pestphp/pest#1855. #277 covered the other half (a closed socket). We hit the alive case on Windows 11 with 5.0.1, twice over nine days, on full suite runs: the websocket to
playwright run-serverstayedESTABLISHED, the server process had no browser child left, the PHP thread sat in a synchronous wait with its CPU time frozen, and the run had to be killed by hand. One of the two started during a memory-exhaustion warning of Windows against thenode.exeof the server (RADAR_PRE_LEAK_64), so the browser launch likely died under pressure while the socket stayed open.Fix
fetch()now receives with aTimeoutCancellationof the action timeout plus a grace (60 s by default,setReplyGrace()to change it), and turns the cancellation into aRuntimeExceptionthat names the deadline. Playwright answers every request within its action timeout, with an error if need be, so a reply that has not come by then plus the grace is not coming. The grace covers the requests the action timeout does not bound, such as a browser launch on a loaded machine.Events keep flowing as before: each message gets its own deadline, so a navigation that waits on its load event is bounded per message, not as a whole.
Tests
tests/Unit/Playwright/ClientTest.php: a stubbed connection whosereceive()honours the cancellation and never produces a message. With a 100 ms action timeout and no grace,execute()throws the new exception within the deadline instead of blocking the test.vendor/bin/pint --test,vendor/bin/phpstan analyseand the unit suite pass.