fix(sentry): turn Sentry fully off when data sharing is off (main backport of #955) - #958
Open
piyalbasu wants to merge 5 commits into
Open
fix(sentry): turn Sentry fully off when data sharing is off (main backport of #955)#958piyalbasu wants to merge 5 commits into
piyalbasu wants to merge 5 commits into
Conversation
…xtension) Previously mobile initialized Sentry unconditionally and kept sending error/crash events (with reduced context) even when the user turned data sharing off — unlike the extension, which never initializes Sentry when sharing is disabled. Make the data-sharing toggle the master switch for Sentry: (1) initializeSentry() no-ops when sharing is off (no client on a cold start); (2) beforeSend hard-drops every event while off (covers the runtime toggle-off window and any lingering/native client); (3) a new syncSentryEnablement(), called from the analytics-store subscription, (re)initializes on toggle-on and clears the user + closes the client on toggle-off. App.tsx already consent-gates the startup setUser. Caveat (documented): a JS-side close() cannot fully tear down native crash handlers mid-session, so a native crash between a runtime toggle-off and the next app launch could still be captured natively; on the next launch with sharing off, init is skipped entirely. Mirrors the extension's own 'close isn't complete until refresh' behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Make initializeSentry() idempotent (guard on isSentryInitialized) so the App startup effect and the analytics-store subscription can't double-init regardless of ordering. - Defer App's cold-start Sentry setup until the persisted data-sharing preference has hydrated from AsyncStorage. Zustand's pre-hydration default is `true`, so an un-deferred init could briefly initialize Sentry for a returning opted-out user. Mirrors services/analytics/core.ts. - Reset module init state before each sentryConfig test so the new idempotency guard doesn't suppress init in later tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Copilot's remaining (low-confidence) review point: Sentry.close() always does a full-drain flush before disabling, so toggling data sharing off could push out events buffered under prior consent (e.g. from an offline window) — contradicting "off means off". Go through the client directly with a 0ms flush timeout: still sets enabled=false (no future capture/send), but does not actively drain the backlog. beforeSend continues to hard-drop anything captured in the gap before close resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hydration Addresses the cloud review's blocking finding plus two 75-scored near-misses: - BLOCKING: client.close(0) did NOT skip the transport drain. In @sentry/core@10.18.0, PromiseBuffer.drain treats a falsy timeout (0 or undefined) as "wait until the whole queue drains", and RN's Sentry.close() passes no timeout at all — so opt-out still full-flushed events buffered under prior consent, the exact behavior the code claimed to avoid. Disable by flipping getOptions().enabled = false directly (what close() does after its flush): captureEvent's _isEnabled() guard blocks all future sends, no drain. - syncSentryEnablement() read isEnabled with no hasHydrated() guard. Called from the analytics-store subscription, which can fire pre-hydration (setUserId during identify) when the Android default is `true` — initializing Sentry for a returning opted-out user. Guard on persist.hasHydrated(), mirroring syncIdentifyTraits; App's startup effect owns the initial post-hydration reconcile. - Updated initializeSentry() JSDoc to document its three no-op cases (e2e / already-initialized / sharing-off), per anti-patterns.md. Tests: assert enabled flips to false (not close()); add a pre-hydration no-op case. 49/49 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Backports consent-based Sentry enablement to main, making data sharing the intended master switch.
Changes:
- Gates initialization and event delivery on consent.
- Synchronizes runtime Sentry state with analytics preferences.
- Defers startup until preference hydration and adds tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/config/sentryConfig.ts |
Implements consent-aware Sentry lifecycle. |
src/components/App.tsx |
Defers initialization until hydration. |
src/services/analytics/core.ts |
Synchronizes Sentry when preferences change. |
__tests__/config/sentryConfig.test.ts |
Tests initialization, event filtering, and toggling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+393
to
+395
| if (isEnabled && !isSentryInitialized) { | ||
| initializeSentry(); | ||
| } else if (!isEnabled && isSentryInitialized) { |
Comment on lines
+407
to
+410
| const client = Sentry.getClient(); | ||
| if (client) { | ||
| client.getOptions().enabled = false; | ||
| } |
Contributor
|
iOS Simulator preview build is ready: https://github.com/stellar/freighter-mobile/releases/tag/untagged-d5c967e21f85515f76ed (SDF collaborators only — install instructions in the release description) |
CassioMG
approved these changes
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Trunk backport of #955 (Sentry fully off when data sharing is off). #955 itself was retargeted onto the in-flight
v1.21.27release branch so it ships in the current beta; this PR lands the identical change onmainso it isn't lost when the release branch is deleted after prod.Implementation details (for agents)
What changed: same 4 files as #955 —
src/config/sentryConfig.ts,src/components/App.tsx,src/services/analytics/core.ts,__tests__/config/sentryConfig.test.ts. Consent gates Sentry init / event delivery / runtime shutdown;syncSentryEnablement()disables viaenabled=false(notclose(), which full-drains) and is guarded onpersist.hasHydrated().Relationship to #955: #955 → base
v1.21.27(beta); this PR → basemain(trunk). Content is identical (the 4 touched files are byte-identical onv1.21.27andmain).Verification: full jest suite green on the release-branch build (pre-commit).