Record more error details on CLI failure

[fredrikekelund]

Related issues

• Related to STU-1260

Proposed Changes

After the 1.7.0 launch, we've seen an increased number of Failed to start server and Failed to create server errors. We know this stems from the CLI launch, and the errors are captured in Sentry, but we don't record enough details to know exactly what's going on.

This PR fixes that by:

1. Changing the CliCommandEventEmitter's failure event to contain a new custom error class in the payload.
2. This custom error class (CliCommandError) captures the last error message sent over IPC (probably the most vital piece of information), stderr, stdout, the process's exit code, or the type of signal that interrupted the process.
3. All site CLI commands now pass the output: 'capture' config to executeCliCommand. If something unexpected is printed to stderr or stdout in the child process, we will now capture it, too.

Testing Instructions

1. Add a line somewhere in cli/commands/site/start.ts that just throws an error, like throw new Error( 'TEST ERROR' );
2. Run npm start
3. Start a server
4. Ensure that you see the following in your terminal:

Sentry Logger [log]: Captured error event `[Last error message] Failed to start WordPress server: TEST ERROR

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[wpmobilebot]

📊 Performance Test Results

Comparing 771f78e vs trunk

site-editor

Metric	trunk	771f78e	Diff	Change
load	2894.00 ms	2899.00 ms	+5.00 ms	🔴 0.2%

site-startup

Metric	trunk	771f78e	Diff	Change
siteCreation	8064.00 ms	7091.00 ms	-973.00 ms	🟢 -12.1%
siteStartup	3925.00 ms	3922.00 ms	-3.00 ms	🟢 -0.1%

---

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change

[fredrikekelund]

I removed the exit event handler, because it's redundant. The close event already provides the same params as the exit event.

[fredrikekelund]

This is unlikely to have any discernible effect, but it still makes sense in principle: this function runs before we quit the app. It kills the child process with a SIGTERM signal. If we initiated the process shutdown and the app is quitting anyway, then we don't need to listen for any more events on child

[sejas]

I edited stop.ts to throw an error, tested it, and confirmed that I see the error message in the Node console.

I also tested importing a SQL file. And the error appeared in the console and in the dialog.

[sejas]

Renaming the trashFiles parameter seems not related to this PR but it's ok to keep it.

[fredrikekelund]

Yeah, I should've commented on that, but that was intentional on my part. Just clarifies what the param does 👍

[gcsecsey]

I added the test error, and it's shown both in the alert box and in the console. 👍

[fredrikekelund]

As a general FYI, here's from the Node.js docs:

If the process exited, code is the final exit code of the process, otherwise null. If the process terminated due to receipt of a signal, signal is the string name of the signal, otherwise null. One of the two will always be non-null.

[fredrikekelund]

The value of exitCode is now implicit based on whether we emit a success or failure event.