Skip to content

feat(local): add --verify, --timeout, auto-detect dev script, post-init verification#998

Open
MathurAditya724 wants to merge 41 commits into
mainfrom
feat/local-run-verify
Open

feat(local): add --verify, --timeout, auto-detect dev script, post-init verification#998
MathurAditya724 wants to merge 41 commits into
mainfrom
feat/local-run-verify

Conversation

@MathurAditya724
Copy link
Copy Markdown
Member

Summary

Adds dev script auto-detection and post-init verification to sentry local run:

  • Auto-detect: when no command is provided, detects the dev script from package.json (dev > develop > serve > start), manage.py, app.py/main.py, go.mod, or docker-compose.yml
  • --verify flag: starts a local server, runs the app, waits for the first SDK envelope to arrive, then reports success/failure
  • --timeout flag: kills the child process after N seconds
  • Post-init verification: after sentry init succeeds, automatically runs the detected dev command with --verify --timeout 30 to confirm the SDK is sending events. On failure, captures a Sentry event with diagnostic context.

Testing

  • 21 tests pass across 3 files (dev-script unit + property, run command)
  • bun run typecheck, check:deps, check:errors, check:fragments all pass

…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)
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-998/

Built to branch gh-pages at 2026-05-26 15:48 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@MathurAditya724
Copy link
Copy Markdown
Member Author

fix-ci: attempt 1 — tests use /tmp/opencode for temp dirs which doesn't exist in CI. Switching to TEST_TMP_DIR from test/constants.ts (uses os.tmpdir()).

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.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

Codecov Results 📊

❌ Patch coverage is 46.53%. Project has 4449 uncovered lines.
❌ Project coverage is 81.5%. Comparing base (base) to head (head).

Files with missing lines (4)
File Patch % Lines
src/lib/init/verify-setup.ts 8.11% ⚠️ 136 Missing and 1 partials
src/commands/local/run.ts 79.63% ⚠️ 22 Missing and 7 partials
src/lib/dev-script.ts 89.74% ⚠️ 4 Missing and 1 partials
src/lib/init/wizard-runner.ts 100.00% ⚠️ 1 partials
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       +10

Generated by Codecov Action

Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts
…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
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/lib/init/wizard-runner.ts
- 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
Comment thread src/commands/local/run.ts Outdated
Comment thread src/lib/dev-script.ts
- 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
Comment thread src/commands/local/run.ts Outdated
Use named handler references and removeListener after the race
settles so SIGINT/SIGTERM aren't swallowed during teardown.
Comment thread src/lib/init/verify-setup.ts Outdated
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.
@MathurAditya724
Copy link
Copy Markdown
Member Author

fix-ci: attempt 1 — two lint errors (numeric separators 5_0005000) and a property test failure in sentryclirc-import.property.test.ts with counterexample "*", investigating

- Replace 5_000 with 5000 to satisfy biome useNumericSeparators rule
- Skip all-asterisk inputs in maskToken identity test (masking '*' correctly returns '*')
@MathurAditya724
Copy link
Copy Markdown
Member Author

pushed fix in 02b8918 — removed 5_000 numeric separators (biome lint) and narrowed the maskToken property test to skip all-asterisk inputs (masking "*" correctly returns "*", so the identity assertion was too broad).

Comment thread src/commands/local/run.ts Outdated
Store and clearTimeout the SIGKILL grace timer so it doesn't hold
the event loop alive for 5s after a cooperative child exit.
@MathurAditya724
Copy link
Copy Markdown
Member Author

fix-ci: attempt 3 — biome flagged 5_000 as an unnecessary numeric separator in verify-setup.ts:136. Changed to 5000.

Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts Outdated
Comment thread src/lib/init/verify-setup.ts Outdated
…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
Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts Outdated
github-actions Bot and others added 2 commits May 21, 2026 16:13
Move removeListener calls after the child kill/await so SIGINT
during the 5s grace period still forwards to the child.
Comment thread src/commands/local/run.ts Outdated
…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.
@MathurAditya724 MathurAditya724 marked this pull request as ready for review May 26, 2026 09:34
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.
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/lib/init/wizard-runner.ts
…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).
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/lib/init/verify-setup.ts
Comment thread src/lib/init/verify-setup.ts
…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'.
Comment thread src/lib/init/verify-setup.ts
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/commands/local/run.ts
- 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.
Comment thread src/commands/local/run.ts Outdated
Comment thread src/lib/init/verify-setup.ts Outdated
- 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).
Comment thread src/lib/init/verify-setup.ts
Comment thread src/lib/init/wizard-runner.ts
Comment thread src/lib/init/wizard-runner.ts
- 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.
Comment thread src/lib/dev-script.ts
- 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.
Copy link
Copy Markdown

@sentry-warden sentry-warden Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts attaches child.on('close') via childExited (and indirectly via watchChildOutput), but grep finds zero child.on("error") registrations in the file.
  • watchChildOutput only registers child.stdout/stderr data and child.on('close') listeners, leaving the error event unhandled.
  • On Linux, a missing executable does not always throw synchronously from spawn(); the error surface is equivalent to the same bug in run.ts:runWithVerify.
  • Node.js EventEmitter promotes an unhandled error event to an uncaught exception.

Identified by Warden find-bugs

Comment thread src/commands/local/run.ts
Comment thread src/lib/dev-script.ts Outdated
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.
Comment thread src/lib/dev-script.ts
Comment thread src/lib/init/verify-setup.ts
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.
Comment thread src/lib/dev-script.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant