fix(init): release stdin handle to unblock event loop on wizard exit#831
Merged
fix(init): release stdin handle to unblock event loop on wizard exit#831
Conversation
At end-of-wizard, `sentry init` would hang until the user pressed a key. Root cause: our no-op patch of `process.stdin.pause` (installed to dodge Bun's fd-0 EINVAL on pause/resume transitions) silently swallowed clack's `rl.close() → input.pause()` call. Stdin stayed in flowing/ref'd mode from `createInterface`'s internal `input.resume()`, keeping the libuv event loop alive until any keypress delivered a `data` event. PRs #802/#824 fixed the `/dev/tty` ReadStream contribution and #825 fixed the MastraClient socket contribution, but the stdin flowing-mode state was never addressed because the patched `.pause()` made it invisible — clack kept calling `input.pause()` during prompt close, but it hit our no-op every time. Fix: invoke `original.pause.call(process.stdin)` in `closeFreshTtyForwarding` after restoring the original methods. Exactly what Node's own `rl.close()` would have called. Idempotent on already-paused streams. Wrapped in try/catch for defense on runtimes that throw after `destroy()`. Regression tests: - Unit: spy on pre-install pause, verify teardown invokes the restored spy exactly once (and that no-op patch swallows calls during the wizard — caught 0 calls mid-run). - Integration: put stdin into flowing mode via real `Readable.prototype.resume`, install+teardown, assert `readableFlowing !== true` afterwards.
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 1941 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 95.31% 95.30% -0.01%
==========================================
Files 284 284 —
Lines 41337 41340 +3
Branches 0 0 —
==========================================
+ Hits 39401 39399 -2
- Misses 1936 1941 +5
- Partials 0 0 —Generated by Codecov Action |
6 tasks
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary After "Sentry SDK installed successfully!", `sentry init` still hangs until a keypress despite #802/#824/#825/#831. Root cause is a Bun 1.3.11 libuv refcount bug that userland cannot fix. Restores PR #782's `process.exit` workaround, but properly wrapped in `setTimeout(..., 100).unref()` so it's transparent in the happy path and terminal only when the Bun bug bites. ## Root cause (verified) Opening our fresh `/dev/tty` ReadStream (the `curl | bash` TTY-delivery workaround in `stdin-reopen.ts`) combined with clack's internal `readline.createInterface(process.stdin)` leaks a libuv handle that NO userland cleanup releases. Verified by systematic matrix test against real `/dev/tty` under a pty: | Scenario | Result | |---------------------------------------|-----------| | `fresh` alone | FAST ✓ | | `readline.createInterface` alone | FAST ✓ | | `readline` + our pause/resume patch | FAST ✓ | | `fresh + readline` | HANG 7s | | `fresh + readline + fresh.destroy()` | HANG 7s | | `fresh + readline + rl.close()` | HANG 7s | | `fresh + readline + process.stdin.destroy()` | HANG 7s | | `fresh + readline + removeAllListeners` | HANG 7s | | `fresh + readline + setTimeout(exit, 100).unref()` | FAST (via forced exit) | `process.stdin.unref()` is `undefined` on Bun 1.3.11, so Node's canonical "let the process exit" escape hatch isn't available. ## Why PRs #802/#824/#825/#831 didn't fix it Each peeled off a **legitimate contributing cause** — all should stay: - #802: `/dev/tty` ReadStream being ref'd (explicit `fresh.destroy()`) - #824: hardened teardown via `using`/`Symbol.dispose` + termios restore - #825: MastraClient keep-alive sockets (AbortController) - #831: `process.stdin` flowing state (restored `pause()` call) But the libuv refcount bug is a Bun-internal issue, not a stream-state or socket issue. No amount of userland cleanup fixes it. ## Fix Restore a force-exit safety net in `src/commands/init.ts`, wrapped in `setTimeout(..., 100).unref()`: ```ts if (process.env.NODE_ENV !== "test") { setTimeout(() => { process.exit(process.exitCode ?? 0); }, 100).unref(); } ``` Properties: - **Transparent in the happy path** — when the loop drains naturally (future Bun versions that fix the refcount bug, non-TTY flows, `--yes` with no prompts), the `.unref()` timer doesn't hold the loop. Process exits before the timer fires. - **Terminal when needed** — when the Bun bug bites, the timer fires after a 100ms grace period. Imperceptible to the user. - **100ms grace period** — enough for Sentry telemetry flush and stdio buffer flush to complete first. Matches best practices for terminal commands. ## Test gate `NODE_ENV !== "test"` guard: `bun test` sets `NODE_ENV=test` automatically. Without this guard, each call to `initCommand.func` in tests would schedule an unref'd 100ms timer; accumulated timers fire across test files and terminate the test runner mid-suite. The guard avoids this while leaving the safety net active in all real-world invocations (including `bun run dev`, compiled binary, npm bundle). ## Test plan - [x] `bun test test/commands/init.test.ts test/lib/init/` — 193 pass - [x] `bun test --timeout 15000 test/lib test/commands test/types` — 5777 pass, 0 fail - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) - [x] Manual repro: the production scenario (real `/dev/tty` under a pty) hangs for 7s without this fix, exits in 286ms with it. - [ ] User validation via `curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash` after merge. ## Follow-ups - Exploration task: find an alternative to the fresh `/dev/tty` ReadStream approach for the `curl | bash` TTY-delivery workaround (the original bug #767 was fixing). If we can make that work without a second ReadStream on stdin, the Bun refcount bug is sidestepped entirely and the safety net becomes redundant. - File a Bun upstream issue with the systematic matrix repro. ## Risk Low. Single-file change. `.unref()` ensures the timer is transparent in healthy flows. Guarded against test-runner interference. All prior fixes remain in place because each addresses a legit cause.
6 tasks
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary PR #835 deleted `forwardFreshTtyToStdin` on the theory that Bun 1.3.11 fixes the inherited-TTY keystroke-delivery bug that the workaround was written for. That's true on **Linux** (verified via PTY harness). It's **NOT** true on **macOS**: `curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash` hangs at the first clack prompt — user can't type. Restore the workaround, gated on `process.platform === "darwin"`. ## User-reported reproduction ``` curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash -s -- --version nightly ``` After install, wizard shows: ``` sentry init EXPERIMENTAL: This feature is experimental and may modify your code. Continue? ● Yes / ○ No ``` Keystrokes (Y/N/Enter) don't reach clack. Prompt hangs indefinitely. Exactly what `forwardFreshTtyToStdin` was fixing originally. ## Fix ### `src/lib/init/wizard-runner.ts` Gate the `using _tty = forwardFreshTtyToStdin()` install on `process.platform === "darwin"`: ```ts using _tty = process.platform === "darwin" ? forwardFreshTtyToStdin() : { [Symbol.dispose]: (): void => { /* no-op on non-Darwin */ } }; ``` - **Linux / WSL / Windows**: no workaround installed, loop drains naturally. Keeps the performance/simplicity win from #835. - **macOS**: workaround installs, keystrokes deliver — original bug fixed again. ### `src/commands/init.ts` Restore the `setTimeout(process.exit, 100).unref()` safety net from #833, also gated to darwin: ```ts if (process.platform === "darwin" && process.env.NODE_ENV !== "test") { setTimeout(() => { process.exit(process.exitCode ?? 0); }, 100).unref(); } ``` The safety net is needed on macOS because the workaround opens a second `tty.ReadStream` which leaks a libuv handle (upstream [oven-sh/bun#29126](oven-sh/bun#29126)). On Linux that leak doesn't occur (no workaround installed). The `NODE_ENV=test` gate ensures `bun test` doesn't accumulate unref'd timers mid-suite. ### Restored (unchanged from pre-#835 state) - `src/lib/init/stdin-reopen.ts` (320 lines) — includes all hardening from #824 (`using`/Symbol.dispose, termios restore), #831 (`original.pause.call` in teardown), and #833-era CI fixes (`Object.defineProperty` for `isTTY`). - `test/lib/init/stdin-reopen.test.ts` (452 lines) — 15 tests covering install/teardown lifecycle via `/dev/ptmx` fixture. ## Test plan - [x] `bun test test/lib/init/ test/commands/init.test.ts` — 193 pass - [x] `bun test --timeout 15000 test/lib test/commands test/types` — 5777 pass, 0 fail - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) - [ ] **Required**: Manual verification on macOS via nightly: `curl -fsSL https://cli.sentry.dev/install | SENTRY_VERSION=nightly SENTRY_INIT=1 bash`. Expect: first clack prompt accepts keystrokes, wizard completes, shell returns promptly after "Setup complete". - [ ] Also verify on Linux (Ubuntu glibc + Alpine musl) and WSL that the workaround is NOT installed and exit is still clean. ## Risk Low. Platform-gated to darwin, so any bugs in the workaround only affect macOS users. Non-Darwin path unchanged from #835 (proven clean on Linux). If macOS has further issues, the revert is a single commit away. ## Follow-ups - File an upstream Bun bug specifically about the macOS compiled-binary keystroke-delivery issue (distinct from oven-sh/bun#29126 which is about `process.stdin`'s readable-listener polling). A minimal repro running the stock `readline` module on a redirected `/dev/tty` fd should be straightforward. - Revisit when Bun 1.3.12+ is available — if the macOS bug is fixed upstream, we can delete the workaround for good.
5 tasks
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
At the end of a
sentry initflow, the process hangs until the user presses a key. Follow-up to #802, #824, #825 — addresses the third and final contributor to the post-wizard hang.Root cause
Our no-op patch of
process.stdin.pausesilently swallowed clack'srl.close() → input.pause()call. Stdin stayed in flowing/ref'd mode fromreadline.createInterface()'s internalinput.resume(), keeping the libuv event loop alive until any keypress delivered adataevent.Why the patch was needed
stdin-reopen.tsreplacesprocess.stdin.pause/resumewith no-ops at install time to dodge Bun's fd-0EINVALon pause/resume transitions (see the comment at the install site). That fix is correct and must stay.Why the bug was invisible
rl.close()is the only place clack ever pauses stdin — it relies entirely on Node's standard readline cleanup discipline. Our no-op patch swallowed every call without any visible error, so there was no log line, no warning, no failed assertion.Why PRs #802/#824/#825 didn't catch it
/dev/ttyReadStream contribution (explicit.destroy()).using/Symbol.disposefor guaranteed teardown + termios restore.Post-teardown state after all three fixes:
/dev/ttyReadStream: destroyed ✓process.stdin: still ref'd and flowing ✗ ← the remaining anchorPR #782's original
process.exit(0)workaround masked this by killing the process unconditionally. Each subsequent PR peeled off one contributor; stdin was the last one standing.Fix
One call to the just-restored
original.pauseat the end ofcloseFreshTtyForwarding():Rationale for
.pause()over alternatives:rl.close()would have called — matches clack's implicit contract..destroy()); any future code that wanted to read stdin (none does ininit; it's a terminal command) could still resume.Regression tests
Two new tests in
test/lib/init/stdin-reopen.test.ts:Unit: teardown invokes restored pause exactly once
Replaces the beforeEach stub with a counting spy BEFORE install (so install captures the spy as
original.pause). Verifies:process.stdin.pauseis the patched no-op (not the spy).process.stdin.pauseis restored to the spy AND the spy was invoked exactly once.Integration: stdin is not flowing after teardown
Puts
process.stdininto flowing mode via realReadable.prototype.resume(simulating what clack does viareadline.createInterface). Runs install + teardown. Assertsprocess.stdin.readableFlowing !== trueafter disposal — without the fix, this assertion would fail because the no-op pause never actually pauses.Test plan
bun test test/lib/init/stdin-reopen.test.ts— 15 pass (13 existing + 2 new)bun test test/lib/init/ test/commands/init.test.ts— 193 passbun test --timeout 15000 test/lib test/commands test/types— 5777 pass, 0 failbun run typecheck— cleanbun run lint— clean (only pre-existing markdown.ts warning)curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash— shell prompt should return immediately after "Setup complete" without a keypress.Risk
Very low. Single call to a restored function (known state) guarded by try/catch. No API changes, no new dependencies, no test fixture churn. Two new tests exercise the exact regression.
Out of scope
pause/resumepatch is still necessary with current Bun (Bun's fd-0 EINVAL may be fixed — worth investigating later as a simplification, but not as part of this hot-fix).