Fix site failing to start when it has a PHP error - #2805
Conversation
…ialog When a site has a PHP fatal error (in functions.php, plugins, etc.), Studio now starts the site and displays the PHP error in the browser instead of showing an error dialog. This allows users to see exactly what's wrong. The fix intercepts Playground CLI's process.exit(1) call and captures the actual PHP error output. The orphaned HTTP server is repurposed to serve an error page. A file watcher watches for .php changes and automatically restarts the server when the error is fixed.
Move pure error-handling functions (isPhpUserError, parsePhpError, generateErrorPageHtml, serveErrorPage) to apps/cli/lib/php-error-handling.ts and simplify output capture by using the existing global console/stdout interceptors instead of duplicating them inside runCLIWithoutExit.
When Playground fails before creating an HTTP server, the orphaned server is null. Previously, the file watcher started unconditionally and the function returned success — leaving nothing on the port and silently retrying with no user feedback. Now the watcher only starts when there is an orphaned server to serve the error page, and the PHP error is sent as a console-message so the parent/UI can display it. Without an orphaned server, the error is rethrown so the parent shows a dialog.
- Remove double-capture of stdout (console.log already captures to capturedBootOutput; the process.stdout.write interceptor was duplicating every entry) - Clear lastCapturedOutput after successful server recovery to free stale error output from memory - Clean up abortControllers record after message handling to prevent unbounded growth over the process lifetime
The cleanup commit incorrectly removed the stdout capture line, but Playground outputs PHP errors to stdout, not console.log. Without this, parsePhpError falls back to the generic message.
📊 Performance Test ResultsComparing f0ed345 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
| retrying = false; | ||
| } | ||
| } )(); | ||
| }, 2000 ); |
There was a problem hiding this comment.
This could be potentially moved to the constant but it is a minor things. Can be left as is
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
I spent some time testing this and here are my observations:
- I think it is a nice UI improvement for the user when there is an error on the site start:
I found a message a bit confusing on the site itself because when I am done fixing the error and if I keep the site open in the browser, Studio does refresh the thumbnail but I see that my site in the browser does not restart. Perhaps, the message could say something regarding closing this tab and accessing the site again from Studio when the error is fixed.
- If the site is running and I break it in the meantime, the error display in this format:
I think this PR is not expected to fix that but I wanted to note it down here as well.
fredrikekelund
left a comment
fredrikekelund
left a comment
There was a problem hiding this comment.
Overriding process.exit and http.createServer strikes me as a somewhat fragile approach.
To me, the ideal outcome here would be that Playground CLI manages to start the site even if there are errors in the PHP code. If I add a nonexistent function to my WP theme after starting my site, Playground CLI displays the error I'd expect in my browser, so we should focus on the boot sequence.
Also, ideally, runCLI shouldn't call process.exit() – at least not unless we know Playground CLI is being invoked as a CLI. That's just not a safe operation for a function that's exposed in the API.
| process.exit = ( ( code?: number ) => { | ||
| if ( code !== 0 ) { | ||
| throw new Error( `WordPress server startup failed (exit code ${ code })` ); | ||
| } | ||
| return originalExit( code ); | ||
| } ) as typeof process.exit; |
There was a problem hiding this comment.
I didn't realize Playground CLI calls process.exit() internally. That's a very clear culprit.
There was a problem hiding this comment.
I experimented a bit with moving this logic to the main process, and letting the child process exit. Still, I agree that the cleanest solution would be for the Playground server to keep running and serve the WordPress error page.
Instead of overriding process.exit and http.createServer inside the child process (fragile), handle PHP boot errors in the main process: - When the child dies from a PHP fatal error, the main process reads PM2 logs, starts an error-page HTTP server on the same port, and watches for PHP file changes to auto-restart. - Emits a site-event so the renderer shows the site as "running". - Increases PM2 log read limit from 50 to 200 lines so the specific PHP error isn't lost behind WordPress's verbose error page HTML. - Removes all process.exit/http.createServer overrides from the child.
…ttic/studio into gcsecsey/fix-php-error-site-start
|
Thanks for the reviews @fredrikekelund and @katinthehatsite!
Yes, I agree. As an alternative to trying to patch this on our end, we could also open a Playground PR that updates
This is true, but as you also pointed out above, the process is exited from Playground, so we're unable to reuse the same server process if it fails during site startup. |
What I meant by this is that it seems achievable that Playground would start the site even if the PHP code contains an error, because it manages to display the error in the browser if it's added after the site was started. I've only skimmed your most recent changes, @gcsecsey, but moving the error recovery up into the process manager is a much cleaner solution, IMO. Still, it'd be even better if Playground could just start the site and display the error in the browser, so I'd argue we could take a stab at solving this directly in Playground. The Orbit team should be able to help us with directions or, indeed, even take on the implementation. |
…x-php-error-site-start
|
@gcsecsey, can we mark this PR as a draft until further notice, to avoid it being included in the daily Slack ping? |
Sure, I converted it to draft for the time being. 👍 |
|
I raised WordPress/wordpress-playground#3478 in Playground to start the server even when there are errors, and also to throw errors instead of calling |
|
The native PHP runtime fixes the underlying issue here (i.e., it can start even if the PHP code contains errors). I still think this behavior should change in Playground, and if WordPress/wordpress-playground#3478 can address that, that's great. However, in the interest of cleaning up the PR backlog in this repo, I'm closing this PR. I believe it would be more straightforward to open a new PR if and when WordPress/wordpress-playground#3478 is merged and released, but feel free to reopen this PR later if needed, @gcsecsey |
Thanks for following up on this @fredrikekelund. Also, thanks for closing this, I don't think these changes are needed even after WordPress/wordpress-playground#3478 is merged. I'll follow up on that PR as it might have missed some reviews due to RSM. |
## Motivation for the change, related issues ### Summary `@wp-playground/cli` can be used in two different ways: 1. As the `wp-playground-cli` command that a person runs in a terminal. 2. As a JavaScript library that another application imports and calls. Those two uses need different error behavior. A standalone command may finish by calling `process.exit()`. An imported library function should not: doing so stops the entire Node.js process, including the application that called the library. The caller cannot catch the error, retry the operation, or finish its own cleanup. This happened in WordPress Studio. Studio imports `runCLI()` and uses it to start Playground in a child process. When Playground startup failed, `runCLI()` called `process.exit(1)` before Studio could handle the failure. This PR separates those responsibilities: - Functions exported as the library API return results or throw errors. - The standalone entry point in `cli.ts` decides when the command-line process should exit. Fixes #3520. Supersedes #3774, which proposes the same core `runCLI()` error-propagation fix but does not include the broader entry-point separation and cleanup work here. Related to Automattic/studio#2805 and its [review discussion](Automattic/studio#2805 (review)). ### Behavior before and after this PR | Situation | Before | After | | --- | --- | --- | | An application calls `runCLI()` and startup fails | Playground prints the error and calls `process.exit(1)`. The application cannot catch it. | `runCLI()` attempts cleanup and rejects its promise (an asynchronous error that normal `try`/`catch` can handle). The application can retry or report it. | | `parseOptionsAndRunCLI()` finishes a one-shot command or reaches an explicitly handled validation exit | It calls `process.exit()`. | It returns `{ exitCode }`, and the caller decides what to do with that code. | | A person runs the standalone CLI and startup fails | The process exits with code 1. | The process still exits with code 1. This responsibility now lives in `cli.ts`. | | A running standalone CLI receives `SIGINT` (for example, Ctrl+C) or `SIGTERM` (a shutdown request) | Signal handling is registered by the library-oriented `run-cli.ts` module. | `cli.ts` disposes the server and then exits. A library consumer controls its own signal policy. | | A library caller runs the `php` command | `runCLI()` calls `process.exit()` with PHP's exit code. | `runCLI()` returns the numeric exit code. The standalone CLI passes that code to `process.exit()`. | For example, a library consumer can now handle a startup failure normally: ```ts try { await using server = await runCLI({ command: 'server', port: 9400, }); // Use the server... } catch (error) { // Report the error, choose another port, retry, etc. } ``` The `await using` declaration automatically disposes the server when execution leaves the `try` block. ## Implementation details ### Keep process termination in the standalone entry point - Removed executable `process.exit()` calls from `run-cli.ts` library paths. - `cli.ts` now translates library results into command-line behavior: - `{ exitCode }` results become the process exit code. - Unexpected errors exit with code 1 after the error is reported. - `SIGINT` and `SIGTERM` dispose a running server before exiting with code 0. - `runCLI({ command: 'php' })` returns PHP's exit code instead of terminating the caller's process. `parseOptionsAndRunCLI()` now returns a `ParseCLIResult`. There are two possible result shapes: - `CLIExitResult`: `{ exitCode: number }` for validation failures and commands that finish without leaving a server running. - `CLIServerResult`: an `AsyncDisposable` handle for a running server. `AsyncDisposable` means the result has an asynchronous cleanup method at `result[Symbol.asyncDispose]()`. Callers may invoke it directly or use `await using` where that syntax is supported. Explicitly handled validation failures use `CLIArgsValidationError`. This prevents an ordinary user mistake from being printed as an unexpected stack trace. `parseOptionsAndRunCLI()` converts the error into `{ exitCode }`. `CLIArgsValidationError` is exported so a direct `runCLI()` caller can identify it with `instanceof` and inspect its `exitCode`. For example, a direct caller may receive this error when `start --reset` targets a site that Playground CLI does not manage. ### Clean up resources before returning an error Removing `process.exit()` exposed an important second problem: a rejected promise is not enough if workers or sockets are still open. Those resources can keep Node.js running even after the caller has handled the error. On each covered boot-failure path, `runCLI()` calls its shared cleanup routine. That routine attempts to: - Terminate spawned worker threads when boot fails, including the normal non-debug path. - Close the HTTP server and all active connections. - Remove the temporary Playground directory even when setup fails before WordPress boots. `startServer()` also closes a socket that has already started listening when `onBind` throws. If cleanup itself fails, the failure is logged without replacing the original startup error; the caller still receives the startup error. Separately, mount failures now include the host path, virtual filesystem path, and root cause. This keeps the message useful to callers that only display `error.message`. ### Keep command-line errors readable For normal CLI use, an unexpected error now prints one concise line, for example: ```text Error: listen EADDRINUSE: address already in use :::12345 ``` The raw error and full debug details remain available with `--debug`. Resetting a site that is not managed by Playground CLI is treated as an expected validation failure. The user sees the existing guidance, while a library caller receives a catchable error with an exit code. ## Breaking changes for library consumers > [!IMPORTANT] > These are public API changes for applications that import > `@wp-playground/cli`. People who only run the `wp-playground-cli` command are > not required to change how they invoke the command. - `runCLI({ command: 'php' })` now resolves to `number` instead of `void`. The number is PHP's exit code. - The general `runCLI()` return type now includes `number`. - A startup failure from `runCLI()` rejects after attempting cleanup instead of terminating the process. - `parseOptionsAndRunCLI()` returns `ParseCLIResult`. Errors that are not converted into explicit exit results are reported and rethrown instead of causing an internal `process.exit()`. - `parseOptionsAndRunCLI()` no longer installs `SIGINT` or `SIGTERM` handlers. A library application that starts a server is responsible for deciding how its process handles signals and when it disposes the server. The successful result from `runCLI({ command: 'server' | 'start' })` is unchanged: it still exposes `.server`, `.serverUrl`, `.playground`, and `[Symbol.asyncDispose]`. ### Compatibility review In July 2026, I searched GitHub for known public consumers of the library API. This is a point-in-time lower bound because private repositories do not appear in public code search. That review did not identify a public consumer expected to break at its currently declared dependency version. <details> <summary>Known public consumers checked</summary> | Consumer | Installed version | Expected effect | | --- | --- | --- | | `Automattic/wp-codebox` | `^3.1.35` | Improvement. It already replaces `process.exit()` with a thrown error. This change makes that workaround unnecessary and allows its `EADDRINUSE` retry logic to work. | | `bgrgicak/wp-tester` | `3.0.15` exactly | Unaffected at its current version. After an upgrade, startup failures become catchable instead of terminating the test process. | | `juanma-wp/users-headless-nextjs-playground` | `1.1.2` exactly | Unaffected at its current version. Its existing `try`/`catch` will work as intended after an upgrade. | | `n3f/link-preview-cards` | `^2.0.4` | Unaffected at its current major version. After an upgrade, an unhandled startup rejection would still end the process with a failure, but callers should add explicit error handling. | | `pfaciana/wp-now-playwright-testing` | `^1.2.2` | Unaffected at its current major version. After an upgrade, it must own graceful signal cleanup if it relies on that behavior. | | `siteorigin/siteorigin-tests-common` | `^2.0.16` | Unaffected at its current major version. Its existing `try`/`finally` structure already expects the call to throw. | All six consumers use the server command, so the `php` return-type change does not affect them. Coordinate separately with private consumers that are not visible through GitHub search. </details> ## Testing Instructions (or ideally a Blueprint) Automated checks run for this change: ```bash npm exec -- nx run playground-cli:test-playground-cli npm exec -- nx run playground-cli:typecheck npm exec -- nx run playground-cli:lint ``` The CLI test target passes all 186 tests, and the PR checks pass on Linux, macOS, and Windows. Coverage includes: - Catchable startup and port-conflict errors. - Structured results for every command type. - Worker termination and socket cleanup after startup failures. - Concise normal error output and debug-only details. - `SIGINT` and `SIGTERM` cleanup in the standalone entry point. Manual checks: 1. Occupy a port, start the standalone CLI on that port, and confirm it prints one concise error and exits with code 1. Repeat with `--debug` and confirm the additional details are shown. 2. Import `runCLI()` from another Node.js program, trigger a startup failure, and confirm the program can catch the error and continue running. 3. Start the standalone CLI, send `SIGINT` or `SIGTERM`, and confirm it cleans up before exiting successfully. --------- Co-authored-by: Brandon Payton <brandon@happycode.net> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 participants
Related issues
How AI was used in this PR
AI assisted with drafting the implementation, exploring the Playground CLI internals, the feasibility of using
mount-onlymode, and reviewing the final code for reuse, quality, and efficiency issues.Proposed Changes
When a site has a PHP fatal error (eg.
this_function_does_not_exist()infunctions.php), the server child process crashes withprocess.exit(1)and the user sees a generic "Failed to start site" error dialog. This happens because Playground's internal error handler callsprocess.exit(1)directly in a.catch(), bypassing all try-catch blocks. The user has no way to know what went wrong or fix it without digging through logs.This PR catches PHP errors during startup and shows them in the browser instead:
runCLIWithoutExit()wrapper inwordpress-server-child.tsthat overridesprocess.exitto throw instead of exiting, so PHP fatal errors become catchable exceptions. It also interceptshttp.createServerto capture Playground's orphaned HTTP server, which is already bound to the port after a failed boot.watchForPhpChanges()file watcher that watches the site directory for.phpfile changes. When the user fixes the error, the watcher closes the orphaned server, attempts a full restart, and on success restores normal operation (blueprints applied, admin credentials set).apps/cli/lib/php-error-handling.tsmodule.uncaughtExceptionandunhandledRejectionhandlers to prevent stale async error callbacks from Playground's failed boot from crashing the child process.Other approaches I explored
I also investigated using Playground's
mount-onlymode as an alternative approach. The idea was to retry withmount-onlymode after a failed normal boot, skipping the WordPress boot (where the fatal error occurs) and starting the HTTP server with all site files mounted. Each browser request would then trigger fresh PHP execution, letting WordPress display errors naturally per-request, the same behavior you see when a fatal error occurs on an already-running server.However,
mount-onlyis a V2 runner concept only (worker-thread-v2.ts). The V1 worker that Studio currently uses silently ignores themodeparameter.This approach would be worth revisiting once the V2 runner ships and
mount-onlybecomes available.Testing Instructions
functions.phpfile and add an invalid function call likethis_function_does_not_exist();functions.phpCleanShot.2026-03-17.at.12.47.59.mp4
Pre-merge Checklist