Skip to content

feat(init): Verify Flutter and Expo setups with doctor commands - #1363

Open
MathurAditya724 wants to merge 1 commit into
mainfrom
feat/init-flutter-expo-doctor-verify
Open

feat(init): Verify Flutter and Expo setups with doctor commands#1363
MathurAditya724 wants to merge 1 commit into
mainfrom
feat/init-flutter-expo-doctor-verify

Conversation

@MathurAditya724

Copy link
Copy Markdown
Member

After a successful sentry init, embedded frameworks like Flutter and Expo cannot be validated by starting a short-lived Spotlight-backed dev server (the default post-init path). This routes those projects to their platform doctor commands instead, while keeping the same non-blocking failure telemetry.

Summary

  • Resolve a verification strategy from the wizard platform first, then filesystem markers (pubspec.yaml for Flutter; expo dependency / app config for Expo)
  • Run flutter doctor or npx expo doctor for those projects; keep the existing local Spotlight path for everything else
  • On doctor failure/timeout/missing tooling, warn the user and captureException with wizard.verify tags (doctor_failed, doctor_timeout, no_flutter, no_npx) — never block a successful init

Test plan

  • Unit tests: pnpm exec vitest run test/lib/init/verify-strategy.test.ts test/lib/init/verify-doctor.test.ts test/lib/init/verify-setup.test.ts
  • Manual: sentry init in a Flutter app → sees flutter doctor verification
  • Manual: sentry init in an Expo app → sees expo doctor verification
  • Manual: sentry init in a Next.js app → still uses local Spotlight verification

Made with Cursor

Embedded frameworks cannot be probed via the Spotlight-backed local
dev-server path used after init. Route Flutter and Expo projects to
`flutter doctor` / `npx expo doctor` instead, still reporting failures
to Sentry telemetry without blocking a successful init.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Aug 5, 2026
@MathurAditya724
MathurAditya724 requested a review from betegon August 5, 2026 06:24
@MathurAditya724 MathurAditya724 added jared Trigger the Jared agent to work on stuff and removed jared Trigger the Jared agent to work on stuff labels Aug 5, 2026

@jared-outpost jared-outpost Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed the doctor-based verification path. clean separation — strategy resolution, doctor runner, and the existing local path stay decoupled, and the scrub helpers moved to verify-doctor.ts without duplication. new unit tests pass locally (12/12) and cover the platform/filesystem precedence well.

a few non-blocking notes:

  • verify-doctor.tsno_npx message vs check. the expo branch does whichSync(cmd) where cmd is npx/npx.cmd, but the user-facing log is hardcoded "npx is not on PATH". fine today, just note the message won't track cmd if the resolution ever changes.

  • expo PATH augmentation. buildDoctorEnv prepends node_modules/.bin and sets npm_config_yes=true — good call for avoiding the interactive npx prompt hanging the wizard. worth a quick manual confirm that npx expo doctor in a real expo app doesn't still block on a first-run install prompt when the local cli is absent.

  • signal re-raise. process.kill(process.pid, signalReceived) after cleanup is the right pattern; just flagging it's exercised only in real runs, not in the mocked tests.

nothing here blocks. the two verify-setup.test.ts failures you may see locally are pre-existing on main (process-tree signal propagation), not from this change.

@MathurAditya724

Copy link
Copy Markdown
Member Author

jared resolve all the issues and make this ready to be merged. Do another through review and resolve all the issues if you find any

@jared-outpost

jared-outpost Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

thorough review done — all new doctor/strategy tests pass (13/13). the 2 failing tests in verify-setup.test.ts are pre-existing POSIX process-tree tests unrelated to this change. no blocking issues found; the implementation matches the stated intent exactly. ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jared Trigger the Jared agent to work on stuff risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant