Skip to content

fix: stop the playwright server process and keep body-less responses off keep-alive connections - #254

Open
cyppe wants to merge 1 commit into
pestphp:5.xfrom
cyppe:fix/server-process-signal-and-bodyless-responses
Open

cyppe wants to merge 1 commit into
pestphp:5.xfrom
cyppe:fix/server-process-signal-and-bodyless-responses

Conversation

@cyppe

@cyppe cyppe commented Sep 3, 2026

Copy link
Copy Markdown

Two independent server-process bugs that together made browser suites flaky on Linux. Both were traced on a real project (Laravel 13, Debian 13 in a container, Playwright 1.62.1) and are reproduced by the two tests added here; each test fails on 5.x and passes with the change. Supersedes #169 and #211 (both target 4.x).

1. playwright run-server outlives the run

PlaywrightNpmServer::start() uses Process::fromShellCommandline() without an exec prefix. PHP's proc_open runs the string as sh -c "…", and dash (Debian's /bin/sh) forks node as a child instead of replacing itself, so stop() only ever signals the shell:

how the server is started process tree after stop()
current (sh -c ./node_modules/.bin/playwright run-server …) sh → node sh exits, node re-parented to PID 1, keeps running with its browsers
with exec node is the direct child node exits, browsers exit with it

Every browser run leaked one server plus a headless Chromium tree; after a day of local runs a host held 65 of them (about 8 GB) and had swapped 50 GB. #169 fixes the same thing by passing an array command (Symfony adds exec itself in that case); this PR keeps the shell command line and adds the prefix on POSIX, which also keeps the relative ./node_modules/.bin/playwright resolution unchanged. stop() now gives node 2 s to close its browsers before Symfony escalates to SIGKILL, which is what #211 asked for.

Test: tests/Unit/Playwright/Servers/PlaywrightNpmServerTest.php starts a server through the class, asserts the process it owns is node … (not sh -c …), and asserts that nothing listening on that port survives stop(). Skipped on macOS/Windows because it reads /proc.

2. Body-less responses corrupt keep-alive connections

LaravelHttpServer hands Symfony's headers to amphp unchanged. Symfony strips Content-Length from 1xx/204/304 responses, and amphp's Http1Driver chunk-encodes every HTTP/1.1 response that has no Content-Length, so a response()->noContent() goes out as

HTTP/1.1 204 No Content
transfer-encoding: chunked

0\r\n\r\n

RFC 7230 §3.3 forbids a body (and Transfer-Encoding) on 204/304. Chromium treats the response as complete after the headers and returns the socket to its pool with the five stray bytes unread; the next request on that socket fails with net::ERR_INVALID_HTTP_RESPONSE (captured with --log-net-log). In the real project the casualty was the Livewire runtime script after a POST …/impression → 204 beacon, so pages silently never booted and only the assertions that depended on JavaScript failed, intermittently, depending on which socket Chromium picked.

handleRequest() now sends informational and empty responses with Content-Length: 0 and no body, which is what every production web server does. amphp itself should special-case 204/304 the way it already special-cases HEAD (reported separately), but the plugin should not depend on that.

Test: tests/Browser/Server/BodylessResponseTest.php posts to a 204/304 route with keepalive and then loads a script; without the fix the script request fails on the reused connection.

Checks

  • vendor/bin/pest tests/Unit/Playwright/Servers/PlaywrightNpmServerTest.php tests/Browser/Server/BodylessResponseTest.php: fail on 5.x, 7 passed with the change
  • full vendor/bin/pest on Linux: see comment below
  • phpstan: no errors; rector and pint clean for the touched files

🤖 Generated with Claude Code

…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 pestphp#169 and pestphp#211.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@cyppe

cyppe commented Sep 3, 2026

Copy link
Copy Markdown
Author

Full vendor/bin/pest on Linux (Debian 13 container, PHP 8.4.24, Playwright 1.62.1, Chromium headless shell 151): 337 passed, 27 skipped (the macOS/CI-only screenshot cases). phpstan clean. The amphp side of issue 2 is reported at amphp/http-server#393.

@DodecaDev-LLC

Copy link
Copy Markdown

+1, confirming the run-server part of this fixes the orphaned Playwright server for us.

Setup: v5.0.1, Symfony Process 8.1.7, PHP 8.5, Playwright 1.63.0, Debian trixie devcontainer (/bin/sh is dash).

Without the fix, the process tree during a run is php → sh -c ./node_modules/.bin/playwright run-server … → node → Chromium. After the suite passes, node is re-parented to PID 1 and keeps the test run's stdout/stderr open. So anything that captures the output (a pipe, timeout, a CI or agent runner) waits forever even though the tests finished in about 7 seconds.

With just the PlaywrightNpmServer changes from this PR applied as a Composer patch, php artisan test --testsuite=Browser | tail returns in about 10 seconds and no node process is left behind.

It would be great to see this merged and tagged.

@likemusic

Copy link
Copy Markdown
Contributor

Verified on a third environment class, and the exec prefix does fix it there.

Same test file, same three runs, same outcome each run (1 failed, 26 skipped, 2 passed — the failure is the external-URL test, unrelated), counting playwright run-server processes left behind after each:

run stock 5.x (c98e8a5) this PR (4a18a88)
1 1 orphan 0
2 2 orphans 0
3 3 orphans 0

Each orphan holds ~182 MB RSS. On WSL2 they are reparented to /init, which is WSL's PID-1 equivalent, so the ps picture matches the Ubuntu and Sail reports in pestphp/pest#1754 with a different parent name. Environment: WSL2 (Ubuntu, /bin/sh → dash), Linux 6.18 kernel on a Windows host, PHP 8.4.24, Playwright 1.62.1.

Worth noting for anyone reading the issue: a green run leaks too, and the leak is silent. Five ordinary runs earlier today left five servers and ~730 MB behind before I went looking for them.

I have not exercised the keep-alive half of this PR, so the table above speaks only to the process fix.

@tibbsa

tibbsa commented Oct 2, 2026

Copy link
Copy Markdown

+1, we ran into the 204 No Content issue and it took a lot of trial and error to figure out what was going on.

@el-schneider

Copy link
Copy Markdown

+1 ran into the same issue.

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.

5 participants