fix: wallet telemetry identify#6258
Merged
DanielSinclair merged 4 commits intodevelopfrom Nov 22, 2024
Merged
Conversation
DanielSinclair
added
the
release
brunobar79
suggested changes
Nov 20, 2024
Contributor
brunobar79
left a comment
brunobar79
left a comment
There was a problem hiding this comment.
Looks great but there's a lint error. Will approve after that!
Contributor
Author
|
@brunobar79 Done ✅ |
DanielSinclair
force-pushed
the
@daniel/wallethash
branch
from
5860477 to
b052b22
Compare
DanielSinclair
force-pushed
the
@daniel/wallethash
branch
from
b052b22 to
bba7cc3
Compare
Contributor
brunobar79
approved these changes
Nov 22, 2024
BrodyHughes
approved these changes
Nov 22, 2024
Contributor
BrodyHughes
left a comment
BrodyHughes
left a comment
There was a problem hiding this comment.
Checked:
- importing and switching wallets
- connecting to a dapp
- accepting / rejecting txns
walletAddressHash and walletType both look consistent and the events seems to populate as expected!
olerass
added a commit
that referenced
this pull request
Mar 2, 2026
The `Analytics` class currently reads the device ID from MMKV in its field initializer (`device.get(['id'])`), but on a user's very first app open that value doesn't exist yet. Because `loadSettingsData()` calls `analytics.identify()` with `language`, `currency`, and `enabledTestnets` traits during the parallel app startup tasks, those calls hit the early-return guard in `identify()` and the traits are silently dropped. On subsequent app opens the device ID is already persisted in MMKV, so the issue is limited to the first session. In practice this means new users who open the app once and don't return have permanently empty trait profiles, and first-session analytics for all new users can't be segmented by these properties. The early identify calls generate ~200K Sentry warning events which is what prompted finding this issue to begin with ([example](https://rainbow-me.sentry.io/issues/6865136718/?project=1855565)). This was introduced in two stages: #6583 (May 2025) rewrote the Analytics class with the `device.get(['id'])` field initializer and `!deviceId` guard, which was safe at the time since `identify()` was only called after `setDeviceId()`. Then #6835 (Sep 2025) moved `loadSettingsData()` into the `Promise.all` block before `getOrCreateDeviceId()`, creating the code path where identify fires before the device ID exists. Sentry confirms the issue started Sep 8, 2025. The underlying issue is that the `Analytics` class starts the Rudderstack client in its constructor (a side effect of module import), but the device ID has to be set separately later because it isn't available at import time. This forces a split between "client starts" and "device ID is ready." This change replaces the constructor side effect with an explicit `init()` method that accepts the device ID and starts the client together. The startup sequence now retrieves the device ID and calls `analytics.init()` before the parallel init tasks, so all `identify()` calls during settings loading succeed on every app open, including the first. The redundant workarounds that were patching around this (a bare `identify()` with empty metadata, and a defensive `setDeviceId` in wallet init added in #6258) are cleaned up. As a side benefit, `Sentry.setUser` also runs earlier now, so crashes during migrations or remote config fetch get attributed to a user. The analytics unit tests have also been unskipped and fixed. They were disabled since Jun 2024 (#5874) because the constructor side effect made the Rudderstack mock very hard to wire up correctly. With init now explicit, the tests can control the lifecycle properly. Fixes FEPLAT-38.
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
3 participants
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.
Fixes APP-1956
What changed (plus any additional context for devs)
walletTypeparam toidentifyand Sentry useruseInitializeWalletto absorb wallet switchesgetWalletContextutil to prepare hashes and wallet types consistently throughout the codebasewalletContextoverrides intrackandscreento support scenarios where the actioned wallet is different than the global selected walletScreen recordings / screenshots
What to test
walletTypeappears to be undefined directly after import because of the following snippet; alternative approach may be necessary