feat(local): add --verify, --timeout, auto-detect dev script, post-init verification#998
feat(local): add --verify, --timeout, auto-detect dev script, post-init verification#998MathurAditya724 wants to merge 41 commits into
Conversation
…it verification - Add src/lib/dev-script.ts: auto-detects dev command from package.json (dev > develop > serve > start), manage.py, app.py, main.py, go.mod, docker-compose.yml/compose.yml - Update sentry local run: when no command args provided, auto-detect from the project. Add --verify flag (wait for first SDK event, then exit) and --timeout flag (kill child after N seconds) - Add src/lib/init/verify-setup.ts: after successful sentry init, run the detected dev command with --verify to confirm SDK sends events. On failure, capture a Sentry event with diagnostic context. - Wire verify-setup into wizard-runner.ts handleFinalResult() - 21 tests across 3 files (dev-script unit + property, run command)
|
|
fix-ci: attempt 1 — tests use |
Tests were using /tmp/opencode which only exists in the local dev environment. CI runners don't have this directory, causing mkdtemp to fail. Switch to TEST_TMP_DIR from test/constants.ts which uses os.tmpdir() and is worker-scoped for parallel test isolation.
Codecov Results 📊❌ Patch coverage is 46.53%. Project has 4449 uncovered lines. Files with missing lines (4)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 81.95% 81.50% -0.45%
==========================================
Files 329 331 +2
Lines 23749 24046 +297
Branches 15502 15638 +136
==========================================
+ Hits 19463 19597 +134
- Misses 4286 4449 +163
- Partials 1643 1653 +10Generated by Codecov Action |
…p scripts with env vars - Store and clearTimeout after Promise.race resolves in runWithVerify and verifySetup to prevent holding the event loop alive - Add SIGINT/SIGTERM forwarding in runWithVerify so Ctrl-C kills the child instead of orphaning it - Detect shell features (env-var prefixes, pipes, operators) in package.json scripts and run them via sh -c instead of naive whitespace splitting
- Register SIGINT/SIGTERM handlers in verifySetup so Ctrl-C during post-init verification kills the child instead of orphaning it - Await child.exited after SIGTERM in both verifySetup and runWithVerify (envelope/timeout branches) so the child releases its port before the function returns
- Always add a timeout racer in runWithVerify — defaults to 30s when no explicit --timeout is given, preventing indefinite hangs - Redact KEY=VALUE env-var assignments in the detectedCommand telemetry field to avoid leaking secrets from package.json scripts
Use named handler references and removeListener after the race settles so SIGINT/SIGTERM aren't swallowed during teardown.
Prevents indefinite hangs when the dev server doesn't respond to SIGTERM. Extracted gracefulKill helper in run.ts; inlined the same pattern in verify-setup.ts.
|
fix-ci: attempt 1 — two lint errors (numeric separators |
- Replace 5_000 with 5000 to satisfy biome useNumericSeparators rule - Skip all-asterisk inputs in maskToken identity test (masking '*' correctly returns '*')
|
pushed fix in 02b8918 — removed |
Store and clearTimeout the SIGKILL grace timer so it doesn't hold the event loop alive for 5s after a cooperative child exit.
|
fix-ci: attempt 3 — biome flagged |
…n env redaction - Add $ and lowercase letters to SHELL_FEATURES_RE so scripts with variable references or mixed-case env assignments route through sh -c - Wrap gracefulKill's initial SIGTERM in try/catch so an already-exited child doesn't skip shutdownServer - Broaden telemetry redaction regex to match mixed-case env var names
Move removeListener calls after the child kill/await so SIGINT during the 5s grace period still forwards to the child.
…y output - Check child.exitCode before cleanup block to avoid registering close listeners on an already-exited process (would hang for 5s on the grace timer). Addresses Warden finding about verifySetup hanging when the child exits before timeout. - Scrub absolute paths and env-var values from dev-server error lines before forwarding to Sentry telemetry. Addresses Warden finding about unscrubbed output in captureException extras.
buildChildEnv and verify-setup both hardcoded SENTRY_TRACES_SAMPLE_RATE to '1', silently overriding any user-configured value. Use nullish coalescing to fall back to '1' only when the user hasn't set it.
…ng error message - Wrap child.kill() in signal handlers with try/catch to handle the race where the child exits between the race resolution and signal delivery. - Guard SIGKILL with exitCode check and try/catch to prevent throwing on an already-exited child. Extract cleanup into cleanupChild(). - Fix 'errored' outcome message: show the actual startup error line instead of the unrelated 'Sentry.init() call found' message. - Extract buildVerifyEnv() and cleanupChild() from verifySetup() to reduce cognitive complexity from 16 to within the 15 limit. - Fix biome formatting (single-line SENTRY_TRACES_SAMPLE_RATE).
…ptions
- Split the 'timeout or silent' telemetry tag into separate values
('silent' vs 'timeout') so monitoring can distinguish between a
process that produced no output and one that timed out.
- Capture the subscription ID from buffer.subscribe() and call
buffer.unsubscribe() during cleanup in both verify-setup.ts and
run.ts to prevent resource leaks when the race resolves early.
…ing server - Add URI_USERINFO_RE to scrubOutputLine() to redact user:password@ from connection strings (e.g. postgresql://admin:pw@host) before sending error lines to Sentry telemetry. - After Promise.race settles, check child.exitCode — if the child crashed with non-zero exit but stdout had no fatal patterns, correct the outcome from 'started' to 'exited' so the crash is reported instead of a false 'Verified — app started successfully'.
- Clear the watchChildOutput timer when settle() is called from any path (fatal error, close event), preventing a 15s stall on exit. - Broaden ENV_VAR_RE to KEY_VALUE_RE to also catch --flag=value CLI arguments (e.g. --api-key=secret) in telemetry redaction. - Fix URI_USERINFO_RE to handle empty-username URIs like redis://:password@host by allowing zero chars before the colon. - Reuse scrubOutputLine() for detectedCommand telemetry field instead of a separate inline regex.
- Add exitCode check before awaiting close after SIGKILL in gracefulKill() to prevent indefinite hang if the close event fires between the kill call and listener attachment. - Scrub the error line shown to the user via ui.log.warn with scrubOutputLine() to redact credentials that may appear in startup errors (e.g. database connection strings in CI logs).
- cleanupChild now awaits the close event after SIGKILL so child.exitCode is populated before the crash detection check in verifySetup. Without this, a force-killed process would have exitCode=null, bypassing the started→exited correction. - Fix biome formatting (single-line if condition).
- Read child.exitCode before cleanupChild() so the crash detection sees the natural exit code, not 143/137 from our SIGTERM/SIGKILL. Fixes false failures where a healthy server killed by cleanup was reported as crashed. - Wrap verifySetup() call in wizard-runner with try-catch so unexpected throws from third-party calls (createSpotlightBuffer, buildApp) don't leave the spinner running or skip formatResult.
- Trim package.json script value before testing against SHELL_FEATURES_RE so leading whitespace doesn't prevent detection of env-var assignments (e.g. ' NODE_OPTIONS=... tsx dev'). - Remove unused biome-ignore suppression for useMaxParams in verify-setup.ts (reportOutcome now takes 2 params via ctx object). - Remove unused biome-ignore suppression for noControlCharactersInRegex in local.ts (biome no longer flags this regex). - Fix import sort order in wizard-runner.ts after adding logger.
There was a problem hiding this comment.
Unhandled child process error event in verifySetup causes process crash
In src/lib/init/verify-setup.ts, the spawned child process has no error event listener; an async spawn error (ENOENT, EACCES, etc.) will become an uncaught exception and crash the CLI. Add a child.on('error', ...) handler before entering the Promise.race.
Evidence
verify-setup.tsattacheschild.on('close')viachildExited(and indirectly viawatchChildOutput), but grep finds zerochild.on("error")registrations in the file.watchChildOutputonly registerschild.stdout/stderr dataandchild.on('close')listeners, leaving theerrorevent unhandled.- On Linux, a missing executable does not always throw synchronously from
spawn(); the error surface is equivalent to the same bug inrun.ts:runWithVerify. - Node.js
EventEmitterpromotes an unhandlederrorevent to an uncaught exception.
Identified by Warden find-bugs
Change ^[A-Za-z_]+= to ^[A-Za-z_]\w*= so scripts like E2E_BASE_URL=... or API_V2_KEY=... are correctly detected as needing shell execution instead of being whitespace-split.
Extend the post-race exit code correction to also cover the 'silent' outcome. A child that crashes immediately with no output would resolve as 'silent' (from watchChildOutput's close handler) instead of 'exited'. Now both 'started' and 'silent' are corrected to 'exited' when preCleanupExitCode is non-zero.
Summary
Adds dev script auto-detection and post-init verification to
sentry local run:dev>develop>serve>start), manage.py, app.py/main.py, go.mod, or docker-compose.yml--verifyflag: starts a local server, runs the app, waits for the first SDK envelope to arrive, then reports success/failure--timeoutflag: kills the child process after N secondssentry initsucceeds, automatically runs the detected dev command with--verify --timeout 30to confirm the SDK is sending events. On failure, captures a Sentry event with diagnostic context.Testing
bun run typecheck,check:deps,check:errors,check:fragmentsall pass