fix(cli): propagate runCLI() startup errors instead of swallowing them via process.exit(1)#3774
Open
amitraj2203 wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR changes runCLI() startup failure behavior so errors from startServer() propagate to library callers (instead of being swallowed via process.exit(1)), while preserving standalone CLI exit behavior via the outer wrapper.
Changes:
- Removed the internal
.catch()inrunCLI()sostartServer()rejections rejectrunCLI(). - Updated the “port in use” test to assert
runCLI()rejects withEADDRINUSEand thatprocess.exitis not called.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/playground/cli/src/run-cli.ts | Stops swallowing startup errors so callers can handle failures instead of an unconditional process exit. |
| packages/playground/cli/tests/run-cli.spec.ts | Adjusts expectations for port-conflict behavior to match the new error propagation semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2137
to
2139
| exitSpy.mockRestore(); | ||
| blockingServer.close(); | ||
| } |
Comment on lines
+2128
to
+2133
| await expect( | ||
| runCLI({ | ||
| command: 'server', | ||
| port, | ||
| }) | ||
| ).rejects.toThrow(/EADDRINUSE/); |
Comment on lines
1830
to
1832
| return response; | ||
| }, | ||
| }).catch((error) => { | ||
| cliOutput.printError(describeError(error)); | ||
| process.exit(1); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for the change, related issues
runCLI()inpackages/playground/cli/src/run-cli.tshad a.catch()on the internalstartServer()call that swallowed startup errors by printing them and callingprocess.exit(1):This means library callers of
runCLI()— such as WordPress Studio, which spawns it as a daemon — can never catch or handle startup errors. A non-fatal error (e.g. a port conflict, a transient worker exit race) kills the entire process with no opportunity to retry or report it gracefully.Fixes #3520
Implementation details
Remove the
.catch()so rejections fromstartServer()propagate naturally throughawaitas a rejected promise fromrunCLI().The
process.exit(1)behavior for standalone CLI users is preserved unchanged:parseOptionsAndRunCLI()already wrapsrunCLI()in its owntry/catchthat formats the error and exits.The existing test
should error when explicit port is already in usewas asserting the old broken behavior (error printed to stdout +process.exitcalled). Updated it to assert the correct behavior:runCLI()rejects withEADDRINUSEandprocess.exitis not called.Breaking change: library callers that previously relied on
runCLI()never rejecting will now see unhandled rejections on startup failure. This is the correct behavior — callers should handle errors fromrunCLI().Testing Instructions (or ideally a Blueprint)
Run the CLI test suite and confirm all tests pass:
Confirm the standalone CLI still exits cleanly on a port conflict:
Confirm library callers can now catch errors: