fix(init): force exit on failure paths so stdin closes cleanly#802
Draft
fix(init): force exit on failure paths so stdin closes cleanly#802
Conversation
The init command calls `process.exit` after the wizard succeeds to release Bun's fetch keep-alive sockets and the forwarded /dev/tty stream — without it, the libuv loop stays alive and the shell hangs. Error paths (WizardError from `runWizard`, ApiError from `findProjectsBySlug`, ValidationError/ContextError from arg parsing) skipped that call, so a failed `sentry init` would leave the terminal stuck. Wrap the command body in try/catch/finally: the finally block always calls `process.exit` with the appropriate code. The catch reports to Sentry via `reportCliError` (normally done by Stricli's handler, which we bypass by exiting early) and renders the error to stderr — skipped for `WizardError.rendered=true` since clack already displayed it. Fixes #798 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Contributor
Codecov Results 📊✅ 138 passed | Total: 138 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1754 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.65% 95.65% —%
==========================================
Files 280 280 —
Lines 40273 40284 +11
Branches 0 0 —
==========================================
+ Hits 38518 38530 +12
- Misses 1755 1754 -1
- Partials 0 0 —Generated by Codecov Action |
…it via setImmediate The prior commit wrapped the init body in try/catch so `process.exit` fired on every path, but that duplicated Stricli's existing error handling (`src/app.ts:295`): manual `reportCliError`, manual `WizardError.rendered` check, manual "Error: …" render. No other command in `src/commands/` does this — they all let errors propagate. Use `try/finally` instead and schedule `process.exit` via `setImmediate`. The error still propagates to Stricli, which reports to Sentry + renders + sets `process.exitCode` as usual. Only after that chain unwinds does the scheduled callback fire and terminate the process with the code Stricli set. Net effect: the bug fix shrinks from ~80 to ~24 lines, the four pre-existing "rejects.toThrow" tests return to their original form (since errors now propagate), and init no longer imports `reportCliError` or the `CliError`/`WizardError` classes directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
Don't think this is the right approach. We should identify the dangling refs. Closing |
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.
Summary
sentry initcallsprocess.exitafter success to release Bun's fetch keep-alive sockets and the forwarded/dev/ttystream. Error paths skipped that call, so a failing init (e.g.Forbiddenduring project analysis) left the terminal hung.try/catch/finallysoprocess.exitfires on every path — success,WizardError, or errors from arg parsing /findProjectsBySlug.Context
Closes #798. The existing success-path
process.exit(process.exitCode ?? 0)was added in #782 with the rationale that the libuv loop stays alive otherwise (Bun's global fetch dispatcher + the fresh/dev/ttyforwarded byforwardFreshTtyToStdin). That rationale applies equally to error paths —runWizardthrowsWizardErrorfrom preflight, connection, and step-loop failures, and none of those hit the exit call.Notes on the approach:
catchcallsreportCliErrordirectly. Normally Stricli'sexceptionWhileRunningCommandhandler (src/app.ts:327) reports to Sentry, but since weprocess.exitinfinallythe error never propagates to it — so we own reporting here.WizardError.rendered=truemeans clack already displayed the message; skipping the stderr write avoids double-printing. Other errors get anError: …line matchingsrc/app.ts:335.autoAuthMiddlewareno longer retries onAuthErrorthrown from init's pre-wizard steps. In practice the wizard owns its own auth via preflight; the only reachable path wasfindProjectsBySlugfor theinit <bare-slug>shape. Users hit with an auth error there will see a rendered message and cansentry login+ retry.Test plan
bun test test/commands/init.test.ts— 42/42 pass (added 6 new error-path regressions; updated 4 pre-existing "throws" tests to the new "force-exits" behavior).bun test test/commands test/lib— 5453/5453 pass.bunx tsc --noEmit— no new errors (only pre-existing unrelatedpicomatchtypes issue).bunx ultracite check src/commands/init.ts test/commands/init.test.ts— clean.sentry initin a project that triggers a preflight or workflow failure (e.g. revoked token → 403 on project analysis). Before: shell hangs after "Setup failed". After: shell returns to prompt with non-zero exit code.