From 4a18a88f151c4ef559dcfc01cb05a0b4d30aeb28 Mon Sep 17 00:00:00 2001 From: cyppe Date: Thu, 3 Sep 2026 22:43:13 +0200 Subject: [PATCH] fix: stop the playwright server process and keep body-less responses off keep-alive connections Two server-side bugs behind flaky browser suites on Linux: 1. PlaywrightNpmServer started 'playwright run-server' through Process::fromShellCommandline() without an exec prefix. PHP runs the string as sh -c, dash forks node instead of replacing itself, and stop() only signals the shell: node and its headless browsers are re-parented to PID 1 after every run. Prefix the command with exec on POSIX and give node two seconds to close its browsers before Symfony escalates to SIGKILL. 2. LaravelHttpServer passed Symfony's headers to amphp unchanged. Symfony strips Content-Length from 1xx/204/304 responses and amphp chunk-encodes any HTTP/1.1 response without one, so a 204 went out with a terminating chunk. Browsers treat a 204 as complete after the headers, leaving the stray bytes on the keep-alive socket; the next request on it fails with ERR_INVALID_HTTP_RESPONSE. Send such responses with Content-Length: 0 and no body. Each change comes with a test that fails without it. Supersedes #169 and #211. Co-Authored-By: Claude Fable 5.1 --- src/Drivers/LaravelHttpServer.php | 15 ++++++- .../Servers/PlaywrightNpmServer.php | 17 ++++++- tests/Browser/Server/BodylessResponseTest.php | 33 ++++++++++++++ .../Servers/PlaywrightNpmServerTest.php | 45 +++++++++++++++++++ 4 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 tests/Browser/Server/BodylessResponseTest.php create mode 100644 tests/Unit/Playwright/Servers/PlaywrightNpmServerTest.php diff --git a/src/Drivers/LaravelHttpServer.php b/src/Drivers/LaravelHttpServer.php index ffa12a6c..6ecdce76 100644 --- a/src/Drivers/LaravelHttpServer.php +++ b/src/Drivers/LaravelHttpServer.php @@ -305,9 +305,22 @@ private function handleRequest(AmpRequest $request): Response } } + $headers = $response->headers->all(); + + if ($response->isInformational() || $response->isEmpty()) { + // Symfony strips Content-Length from 1xx/204/304 responses and amphp + // chunk-encodes any HTTP/1.1 response without one, which appends a + // terminating chunk to a response that must not carry a body. Browsers + // consider such a response complete after the headers, so the stray + // bytes corrupt the next request on the same keep-alive connection. + unset($headers['transfer-encoding']); + $headers['content-length'] = ['0']; + $content = ''; + } + return new Response( $response->getStatusCode(), - $response->headers->all(), // @phpstan-ignore-line + $headers, // @phpstan-ignore-line $content, ); } diff --git a/src/Playwright/Servers/PlaywrightNpmServer.php b/src/Playwright/Servers/PlaywrightNpmServer.php index 1afa6647..fa657876 100644 --- a/src/Playwright/Servers/PlaywrightNpmServer.php +++ b/src/Playwright/Servers/PlaywrightNpmServer.php @@ -60,7 +60,10 @@ public function start(): void return; } - $this->systemProcess = SystemProcess::fromShellCommandline(sprintf( + // `exec` makes the shell replace itself with node, so stop() signals the + // server itself. Without it, PHP's `sh -c` wrapper receives the signal and + // node is re-parented to PID 1 together with the browsers it launched. + $this->systemProcess = SystemProcess::fromShellCommandline($this->execPrefix().sprintf( $this->command, $this->host, $this->port, @@ -94,7 +97,9 @@ public function start(): void public function stop(): void { if ($this->systemProcess instanceof SystemProcess && $this->isRunning()) { - $this->systemProcess->stop(timeout: 0.1); + // Give node a moment to close its browsers on SIGTERM before Symfony + // escalates to SIGKILL; the browsers exit on their own once node is gone. + $this->systemProcess->stop(timeout: 2.0); } $this->systemProcess = null; @@ -164,6 +169,14 @@ private function ensurePlaywrightIsInstalledAndVersionIsSupported(): void } } + /** + * The shell prefix that hands the process its own PID (POSIX only). + */ + private function execPrefix(): string + { + return DIRECTORY_SEPARATOR === '/' ? 'exec ' : ''; + } + /** * Checks */ diff --git a/tests/Browser/Server/BodylessResponseTest.php b/tests/Browser/Server/BodylessResponseTest.php new file mode 100644 index 00000000..573f8789 --- /dev/null +++ b/tests/Browser/Server/BodylessResponseTest.php @@ -0,0 +1,33 @@ + new Response('', $status)); + + Route::get('/after-beacon.js', fn (): Response => new Response('window.afterBeaconLoaded = true;', 200, [ + 'Content-Type' => 'text/javascript', + ])); + + Route::get('/', fn (): string => <<<'HTML' + + + + + + HTML); + + visit('/')->assertScript('window.afterBeaconLoaded === true || window.afterBeaconFailed === true'); + + visit('/')->assertScript('window.afterBeaconLoaded === true'); +})->with([204, 304])->repeat(3); diff --git a/tests/Unit/Playwright/Servers/PlaywrightNpmServerTest.php b/tests/Unit/Playwright/Servers/PlaywrightNpmServerTest.php new file mode 100644 index 00000000..718433ca --- /dev/null +++ b/tests/Unit/Playwright/Servers/PlaywrightNpmServerTest.php @@ -0,0 +1,45 @@ +start(); + + try { + $pid = (int) new ReflectionProperty($server, 'systemProcess')->getValue($server)->getPid(); + $commandLine = str_replace("\0", ' ', (string) file_get_contents("/proc/{$pid}/cmdline")); + + expect($commandLine)->toStartWith('node ') + ->and($commandLine)->toContain("--port {$port}"); + } finally { + $server->stop(); + AlreadyStartedPlaywrightServer::markAsStopped(); + } + + $survivors = []; + + foreach (glob('/proc/[0-9]*', GLOB_NOSORT) ?: [] as $directory) { + $candidate = str_replace("\0", ' ', (string) @file_get_contents($directory.'/cmdline')); + + if (str_contains($candidate, "run-server --host 127.0.0.1 --port {$port} ")) { + $survivors[] = basename($directory); + } + } + + expect($survivors)->toBe([]); +})->skipOnWindows()->skipOnMac();