Set anonymous Sentry user so alerts report users affected#2840
Conversation
Sentry alerts showed "Users: 0" because no user context was ever
attached to events. On @sentry/browser v9, sendDefaultPii defaults to
false (no IP), and the extension never called Sentry.setUser, so every
event arrived with an empty user and could not be attributed.
Attach the existing persisted anonymous analytics id (the same one
Amplitude uses) via Sentry.setUser, gated on the existing data-sharing
consent. No PII or wallet address — just an opaque per-install id, so
Sentry and Amplitude user counts stay aligned. Mirrors freighter-mobile.
Also harden generateRandomUserId: Math.random() can yield values < 1e-6
(exponential notation) or exactly 0, where split(".")[1] is undefined;
fall back to "0" so we never hand Sentry/Amplitude an undefined id.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 459c3afc47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-2ff63a035bea038feea7 (SDF collaborators only — install instructions in the release description) |
There was a problem hiding this comment.
Pull request overview
This PR updates the extension’s Sentry initialization to attach a stable, anonymous per-install user identifier to error events (only when data sharing is enabled), allowing Sentry alerts to correctly show “users affected” counts.
Changes:
- Set a Sentry user
{ id }during Sentry initialization using the existing persisted analytics user ID. - Export
getUserIdfromhelpers/metricsso it can be reused by error tracking. - Harden the random ID generator to avoid producing
undefinedin edge cases (Math.random()exponential/zero formatting).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extension/src/popup/components/ErrorTracking/index.tsx | Sets an anonymous Sentry user ID during init so Sentry can compute “users affected”. |
| extension/src/helpers/metrics.ts | Exports getUserId and hardens ID generation to avoid undefined IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…mock - Clear the per-install user id (Sentry.setUser(null)) before Sentry.close on data-sharing opt-out. Sentry.close may still report (anonymized) until the next refresh; without this those reports would keep the id attached. - Add getUserId to the helpers/metrics jest mock in setupTests so rendering ErrorTracking under test doesn't call an undefined mock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TL;DR
Sentry alerts for the Freighter extension all showed "Users: 0", so we couldn't tell how many people a given error actually affected. The cause: error events were never tagged with a user, so Sentry had no one to count. This attaches a stable, anonymous per-install identifier to Sentry events (only when error reporting is already enabled), so alerts now show real "users affected" counts. No personal data or wallet addresses are involved — it's the same opaque ID analytics already uses, so the two stay in agreement. This matches how the mobile app already works.
Implementation details (for agents)
Root cause:
@sentry/browseris on v9, wheresendDefaultPiidefaults tofalse(no IP capture), andSentry.initwas never followed by aSentry.setUsercall. Every event therefore arrived with an emptyuser, and Sentry's "users affected" counts distinct user identifiers → 0.What changed:
Attach an anonymous user id during Sentry init, inside the existing
SENTRY_KEY && isDataSharingAllowedconsent gate, reusing the persisted analytics id (same one Amplitude uses, so counts stay aligned):freighter/extension/src/popup/components/ErrorTracking/index.tsx
Lines 48 to 55 in 459c3af
Export the existing
getUserIdhelper so the Sentry init can reuse it rather than minting a separate id:freighter/extension/src/helpers/metrics.ts
Line 128 in 459c3af
Harden the id generator:
Math.random()can yield values< 1e-6(rendered in exponential notation, e.g."4e-7") or exactly0("0"), wheresplit(".")[1]isundefined. Thatundefinedwould flow intoSentry.setUser({ id: undefined })andamplitude.setUserId(undefined), reproducing "Users: 0" for that install. Fall back to"0":freighter/extension/src/helpers/metrics.ts
Lines 113 to 117 in 459c3af
Design notes:
Sentry.close()on opt-out.metrics_user_idlocalStorage key as Amplitude, so Sentry and Amplitude user counts agree.Sentry.setUser({ id: await getUserId() })insrc/components/App.tsxoff the same sharedgetUserId.Verification:
tsc --noEmit -p extension/tsconfig.json→ exit 0, 0 errors.Follow-up / out of scope:
generateRandomUserIdundefinededge case exists in freighter-mobile's generator; worth the same one-line?? "0"hardening in that repo as a separate PR.