Skip to content

Consolidate Sentry initialization: single init, opt-in DSN, URL scrubbing - #16526

Open
bodhish wants to merge 16 commits into
issues/16507/vitest-typecheck-cifrom
issues/16508/sentry-init-consolidation
Open

Consolidate Sentry initialization: single init, opt-in DSN, URL scrubbing#16526
bodhish wants to merge 16 commits into
issues/16507/vitest-typecheck-cifrom
issues/16508/sentry-init-consolidation

Conversation

@bodhish

@bodhish bodhish commented Jul 8, 2026

Copy link
Copy Markdown
Member

Closes #16508

Stacked on #16514 (uses the Vitest harness for the new tests). GitHub will retarget this to develop when that merges.

What

  • care.config.ts: sentry.dsn no longer falls back to a hardcoded external DSN — telemetry is now opt-in (REACT_SENTRY_DSN unset → Sentry disabled).
  • src/index.tsx: removes the eager production Sentry.init (and the @sentry/browser import from the eager bundle). Previously both this and the Integrations/Sentry component initialized Sentry in production.
  • src/Integrations/Sentry.tsx is now the single init, configured with sendDefaultPii: false and a beforeSend hook that strips query strings, normalizes UUID path segments to <id> in event/breadcrumb URLs, and drops request headers/cookies. The unset-DSN case is a quiet no-op instead of a console.error, since that's now a supported state.

Verification

  • npm run typecheck → exit 0
  • npm run test:unit → 8/8 (3 new scrubUrl tests + seed suite)
  • grep -rn "Sentry.init" src/ care.config.ts → exactly one site
  • No hardcoded DSN remains in the codebase

Deployment note

Deployments that relied on the old implicit default DSN stop reporting after this — set REACT_SENTRY_DSN explicitly where telemetry is wanted.

🤖 Generated with Claude Code

rithviknishad and others added 3 commits July 8, 2026 16:07
Removes the duplicate eager Sentry.init from src/index.tsx (the
lazy init in src/Integrations/Sentry.tsx now runs exactly once),
drops the hardcoded fallback DSN so telemetry is off unless
REACT_SENTRY_DSN is explicitly set, and adds a beforeSend hook
that scrubs query strings and internal record identifiers from
event/breadcrumb URLs before events leave the browser.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bodhish
bodhish requested a review from a team as a code owner July 8, 2026 12:09
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 81b6ba10-4701-4e75-95a0-43f197579fb9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issues/16508/sentry-init-consolidation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

⚠️ Merge Checklist Incomplete

Thank you for your contribution! To help us review your PR efficiently, please complete the merge checklist in your PR description.

Your PR will be reviewed once you have marked the appropriate checklist items.

To update the checklist:

  • Change - [ ] to - [x] for completed items
  • Only check items that are relevant to your PR
  • Leave items unchecked if they don't apply

The checklist helps ensure code quality, testing coverage, and documentation are properly addressed.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 8, 2026

Copy link
Copy Markdown

Deploying care-preview with  Cloudflare Pages  Cloudflare Pages

Latest commit: 41e49ed
Status: ✅  Deploy successful!
Preview URL: https://595ce966.care-preview-a7w.pages.dev
Branch Preview URL: https://issues-16508-sentry-init-con.care-preview-a7w.pages.dev

View logs

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR consolidates Sentry initialization to a single site (src/Integrations/Sentry.tsx), removes the hardcoded fallback DSN that was silently sending telemetry by default, and adds a beforeSend hook that scrubs query strings, normalizes UUID path segments to <id>, and strips request headers and cookies before events are sent.

  • care.config.ts: sentry.dsn now resolves to undefined when REACT_SENTRY_DSN is unset, making telemetry fully opt-in; deployments that relied on the implicit default DSN will stop reporting until the env var is set explicitly.
  • src/index.tsx: Removes the eager production Sentry.init with the hardcoded DSN and the @sentry/browser import from the main bundle, eliminating the double-init that could occur in production.
  • src/Integrations/Sentry.tsx: New scrubUrl helper and beforeSend hook added with sendDefaultPii: false; the unset-DSN path is now a silent no-op instead of a console.error.
  • src/Integrations/Sentry.test.ts: Four unit tests for scrubUrl covering query stripping, UUID replacement, the combined case, and clean pass-through URLs.

Confidence Score: 5/5

Safe to merge; the change correctly removes a hardcoded DSN and adds privacy-preserving URL scrubbing, with one breadcrumb-scrubbing detail worth verifying against the Sentry v9 runtime event shape.

The consolidation is clean, the opt-in DSN change is straightforward, and the beforeSend logic is well-structured. The only unverified detail is whether event.breadcrumbs in Sentry v9's beforeSend is a flat array or { values: Breadcrumb[] } — if it's the latter, the breadcrumb URL scrubbing is silently skipped rather than breaking anything, since Sentry catches errors in beforeSend. All other privacy scrubbing (URL query strings, headers, cookies) is unaffected.

The breadcrumb-scrubbing block in src/Integrations/Sentry.tsx (lines 32–43) is worth a quick sanity check against the actual beforeSend event shape in @sentry/browser v9 before merging.

Important Files Changed

Filename Overview
care.config.ts Removes the hardcoded fallback DSN from sentry.dsn, making telemetry opt-in via REACT_SENTRY_DSN env var
src/Integrations/Sentry.tsx Adds scrubUrl helper and a beforeSend hook that strips query strings, replaces UUID segments, and deletes request headers/cookies; removes old console.error for unconfigured DSN
src/Integrations/Sentry.test.ts New unit tests for scrubUrl covering query-strip, UUID replacement, combined case, and clean URLs — all four cases look correct
src/index.tsx Removes the eager, production-only Sentry.init with the hardcoded DSN and the @sentry/browser eager import, leaving a single init site in the Sentry integration component

Reviews (2): Last reviewed commit: "test: add combined UUID-path + query-str..." | Re-trigger Greptile

Comment thread src/Integrations/Sentry.tsx
Comment thread src/Integrations/Sentry.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c84f51ddf6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Integrations/Sentry.tsx
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bodhish and others added 6 commits July 8, 2026 19:21
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: abhimanyurajeesh <abhimanyurajeesh@gmail.com>
Co-authored-by: rithviknishad <mail@rithviknishad.dev>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Rithvik Nishad <mail@rithviknishad.dev>
@github-actions github-actions Bot added the Merge Conflict pull requests with merge conflict label Jul 14, 2026
@github-actions

Copy link
Copy Markdown

Conflicts have been detected against the base branch. Please merge the base branch into your branch.
cc: @bodhish

See: https://docs.ohc.network/docs/contributing#how-to-resolve-merge-conflicts

@nihal467 nihal467 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot fix the merge conflict

Co-authored-by: nihal467 <57055998+nihal467@users.noreply.github.com>
Copilot AI requested a review from a team as a code owner July 16, 2026 14:42
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown

Too many files changed for review. (224 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

Co-authored-by: nihal467 <57055998+nihal467@users.noreply.github.com>

Copilot AI commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@copilot fix the merge conflict

Resolved the merge conflicts in commit 41e49ed:

  • .github/workflows/linter.yml: Updated to Node 24, kept typecheck and test:unit steps, replaced unimported with knip.
  • package.json: Kept vitest-based test:unit/typecheck scripts; adopted develop's newer package versions (vite ^8, typescript ^6, tsx ^4.22.3, etc.); removed ts-node, unimported, and vite-plugin-static-copy (dropped in develop).
  • src/index.tsx: Kept the vite:preloadError handler from this PR; added the initApp() function from develop; removed the old hardcoded Sentry.init block.
  • package-lock.json: Regenerated after resolving the above.

Copilot AI requested a review from nihal467 July 16, 2026 14:45
@github-actions github-actions Bot added the stale label Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changes Required Merge Conflict pull requests with merge conflict stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants