Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
fc286ca to
e792d88
Compare
|
I just updated the PR with my fork and you can check for yourself that with this PR, the e2e tests pass just fine: https://github.com/contao/contao/actions/runs/36156023262/job/108140649630?pr=10088 😊 |
|
Thanks @Toflar! Removing the extra PHP polling layer makes perfet sense 👍 I tested this with Chromium and found two "regressions":
And if that's ok for you, could we cover these two cases with regression tests too? Please tell me if you disagree here or if I missed something :) |
|
Thanks for testing this, and yes, I agree with both points. I restored the transport deadline for commands that do not have a native Playwright timeout, so an evaluation of a promise that never settles is bounded by the configured transport timeout again. Commands with native timeout semantics still defer to Playwright, which preserves page and context defaults as well as explicit operation timeouts. I also changed While auditing the surrounding timeout handling, I found two related issues and included fixes for them as well:
There is, however, a broader API consistency issue affecting all of Playwright PHP that I intentionally left out of this PR. Playwright itself supports timeout options on several other locator operations while our corresponding PHP methods currently have parameterless signatures. Exposing those options would require changing existing methods on The new |
Unfortunately, I have seen flaky failures on GitHub Actions jobs which seem limited to 0.25 CPU. The same test could alternately fail with either a JSON-RPC timeout or with
Element not actionable, even though the latter was not the underlying problem so this was a nightmare to debug.Turns out
click(),fill()anddragTo()performed PHP-side actionability polling before sending the actual command to Playwright. Each poll issued additionalisVisible()andisEnabled()JSON-RPC requests. Under constrained resources, a transport timeout during one of these requests could be caught by the polling loop and eventually replaced with the misleadingElement not actionableerror.This PR removes all of that redundant polling and sends locator actions directly to Playwright. Playwright already provides native auto-waiting and complete actionability checks for these operations, so it can determine when an element is ready and return its original error details and call log when an action fails.
The existing optional action timeout parameters are now nullable. That way, they remain available and are forwarded to the native Playwright operation. This preserves backward compatibility without retaining the PHP polling behavior.
Native locator waits also replace PHP polling where Playwright provides an equivalent operation. The remaining PHP condition polling no longer catches
PlaywrightException, ensuring JSON-RPC timeouts, strict-selector violations and other operational failures remain visible to the caller. So this should make debugging easier 😇