[Bug]: Browser plugin — evaluate()/script() can return another call's result (processResultResponse ignores the request id)
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 11.7k
- Forks
- 538
- Avg merge
- 4d 11h
- Merged PRs (30d)
- 8
Description
What Happened
Webpage::script() / Page::evaluate() can return the result of an earlier, unrelated call, because the value is picked by shape instead of by request id.
Client::execute() yields every websocket frame it reads until it sees one whose id matches its own $requestId:
// src/Playwright/Client.php
$requestId = uniqid();
// ...
while (true) {
$response = json_decode($this->fetch($this->websocketConnection), true);
// ...
yield $response; // <- every frame is yielded
if (isset($response['id']) && $response['id'] === $requestId) {
break;
}
}
processResultResponse() then consumes that generator and returns the first frame carrying result.value, without ever comparing id:
// src/Playwright/Concerns/InteractsWithPlaywright.php
private function processResultResponse(Generator $response): mixed
{
foreach ($response as $message) {
if (isset($message['result']['value'])) {
return JavaScriptSerializer::parseValue($message['result']['value']);
}
}
return null;
}
So if a frame belonging to a previous request is still in the stream when the next evaluate() runs, that frame's value is returned as this call's result. The call succeeds and returns plausible data — it is simply the wrong data, which makes it very hard to spot.
Observed in CI. A helper that evaluates window.__pestBrowser.jsErrors returned {text: "…", count: 2} — the return value of the previous script() call in the same test, which read a card's text. Nothing was wrong with the page.
The same root cause surfaces as a hard crash through assertNoJavaScriptErrors():
// src/Api/Concerns/MakesConsoleAssertions.php
expect($javaScriptErrors)->toBeEmpty(sprintf(
'…: %s',
// …
implode(', ', array_map(fn (array $log) => $log['message'], $javaScriptErrors)),
));
Two problems compound here: javaScriptErrors() may receive some other call's value (not a list of array{message: string}), and the failure message is built by sprintf(...) before the expectation runs, so array_map(fn (array $log) => …) throws
TypeError: {closure:…assertNoJavaScriptErrors():86}(): Argument #1 ($log) must be of type array, string given
even when the assertion would have passed. The real value is swallowed and the test reports a TypeError in plugin internals.
How to Reproduce
Any browser test that makes two script() calls in a row can hit it; it is a race, so it reproduces intermittently and much more often on a loaded machine (a shared CI runner). The shape is:
$page = visit('/some-page');
$first = $page->script('(() => ({ text: "hello", count: 2 }))()');
$second = $page->script('(() => ["a", "b"])()'); // may return {text: "hello", count: 2}
We hit it on a self-hosted runner in roughly 1 of 3 runs of a 164-test browser suite; on a developer laptop it is rare.
Suggested fixes:
processResultResponse()should only accept the frame whoseidmatches the request that produced the generator (the id is known inClient::execute(), so either filter there or pass it down).- Independently, build the failure message lazily (or defensively) in
assertNoJavaScriptErrors(), so a malformed entry cannot turn a passing assertion into aTypeErrorthat hides the value.
Happy to send a PR if the direction above looks right.
Pest Version
pestphp/pest v5.1.3, pestphp/pest-plugin-browser v5.0.1
PHP Version
8.4.23
Operating System
macOS (developer machines) and Linux (CI runner, container image)
Notes
Laravel 13 app, LaravelHttpServer driver, playwright 1.62.1.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/Playwright/Client.php and src/Playwright/Concerns/InteractsWithPlaywright.php, then inspect assertNoJavaScriptErrors() in src/Api/Concerns/MakesConsoleAssertions.php. Reproduce the issue with consecutive script() calls and a loaded browser test environment. Done means responses are matched to the originating request and assertion failures do not raise the reported TypeError, with regression coverage for both cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100