P-2199: Test and improve mobile SDK#50
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Fix three mobile analytics gaps found while testing the RN SDK against the ingestion pipeline (the SDK was already ~80% complete; the real issues were at the SDK <-> Tinybird seams): - session_id: the SDK sent none, so the pipeline (which groups sessions by session_id with no fallback) collapsed all mobile users of a project into one session. Now generated with a 30-min inactivity timeout. - user_agent: empty on the Expo path, so device/OS classification (done purely from the UA string) resolved to "unknown". Now synthesized from the device info the SDK already collects. - screen page_url: was app://<name>, making the screen name the origin (fragmenting sessions) and leaving page_path empty. Now app://<bundleId>/<name> - stable origin per app, real path per screen. Add per-event debug logging and 3 test suites pinning each fix to the pipeline's classifier. 226 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4d9527d to
f4e858a
Compare
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
session_id/anonymous_id now use Web Crypto (getRandomValues/randomUUID) when available, addressing the CodeQL js/insecure-randomness alert. Falls back to Math.random only on runtimes without Web Crypto so the SDK never throws; these IDs are analytics identifiers, not security tokens. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend overrides to bump the fixable transitive deps (shell-quote, tmp, ws@6/7, ws@8, js-yaml@3) and document-ignore the 3 residuals with no clean fix (@babel/core, ws, js-yaml) - all in react-native/wagmi build tooling, never shipped in the SDK runtime. pnpm audit --prod now passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the no-Web-Crypto fallback (previously Math.random) with a monotonic-counter + timestamp hashed via SHA-256, so there is no PRNG in the code at all — clearing the residual CodeQL js/insecure-randomness alert. Secure path (Web Crypto) unchanged; fallback stays crash-free and collision-free within a process. Adds tests exercising the fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
note that the 2nd half of this happens on tb / backend (https://linear.app/getformo/issue/P-2070/update-tinybird-to-handle-mobile-sdk-page-events and https://github.com/getformo/formono/pull/2033) if there are anything we should do on the backend side to support mobile analytics that is missing, please mention it |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8353b8571
ℹ️ 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".
| uuidFallbackCounter = (uuidFallbackCounter + 1) >>> 0; | ||
| return formatUuidV4(sha256(utf8ToBytes(`${Date.now()}-${uuidFallbackCounter}`))); |
There was a problem hiding this comment.
In supported React Native installations that do not polyfill Web Crypto, this fallback is deterministic across app processes: two fresh processes whose first call occurs in the same millisecond both hash the identical ${Date.now()}-1 input and therefore emit the same UUID. Because this function now supplies both persisted anonymous_id and session_id, those users' events can be merged downstream. The SDK does not install or initialize a Web Crypto polyfill, so retain a random source in this branch (or require/import a secure RNG) rather than deriving IDs solely from time and a process-local counter.
Useful? React with 👍 / 👎.
| '@babel/plugin-transform-modules-systemjs@8.0.1': | ||
| resolution: {integrity: sha512-0NEHanXmnFEnfT2dLKTXnu7m8GXFsnxRgteBC2aH21hYMBwAgxu5dcTdi/Eg+ToI1HbZe0CHwz4XRLgRNQhYoQ==} | ||
| engines: {node: ^22.18.0 || >=24.11.0} | ||
| peerDependencies: | ||
| '@babel/core': ^7.0.0-0 | ||
| '@babel/core': ^8.0.0 |
There was a problem hiding this comment.
This lockfile now resolves the SystemJS plugin to Babel 8 even though the same snapshot wires it to @babel/core@7.29.7 (lines 4542–4546), despite the package declaring a ^8.0.0 core peer. It is a dependency of the @babel/preset-env used by babel.config.js, so builds/tests load an incompatible plugin; additionally its declared Node range starts at 22.18 while every CI job uses Node 22.14. Pin the override below 8 (for example >=7.29.4 <8) or upgrade the entire Babel/Node toolchain together.
Useful? React with 👍 / 👎.
- Pin @babel/plugin-transform-modules-systemjs override below v8. The lockfile regen let it float to 8.0.1 (peer @babel/core@^8, Node >=22.18) while the tree uses @babel/core@7 and CI runs Node 22.14 (P1). - Restore Math.random in the no-Web-Crypto UUID fallback for cross-process entropy; two processes starting in the same millisecond otherwise collide (P2). Math.random is fine here (analytics IDs, not security tokens); the CodeQL insecure-randomness alert on this line is dismissed as a false positive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bundle-id-host format (app://<bundleId>/<name>) conflicts with the Tinybird mobile page-event handling in formono#2033, whose page_path parser strips the app:// scheme and would leak the bundle id into the path (/com.formo.app/home instead of /home). The backend derives origin from the app identifier in context and page_path from the URL, so the SDK emits the screen name as-is. Test updated to guard against re-encoding a host. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
yosriady
left a comment
yosriady
left a comment
There was a problem hiding this comment.
LGTM, please resolve the codex review in the other PR
|
Also can you comment a list of sample events emitted by the SDK (with full payloads for each unique event type) for both ios and android (including user agents) I need it to update the dashboard / tinybird to add mobile support |
|
Also, in the test mobile project please let me know if there ar eany charts / pages that aren't working properly yet when using the mobile SDK (since the dashboard is designed for web SDK only at the moment) |
Sample events emitted by the RN SDK (iOS + Android)All payloads below are dumped from the actual SDK ( What the SDK sends vs. what gets added later
The
|
| Case | user_agent value |
|---|---|
Expo / no react-native-device-info (most common) |
synthesized: Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) FormoAnalytics/ReactNative or Mozilla/5.0 (Linux; Android 14; Pixel 8) Mobile FormoAnalytics/ReactNative |
Bare RN with react-native-device-info |
the native WebView UA (a real Safari/Chrome UA string) |
| Older SDK (≤0.1.6, pre-this-PR) | empty string — the case #2033 handles via context |
Tablets emit iPad / Tablet instead of iPhone / Mobile. device_type is always mobile or tablet.
Every event type — verified properties
Everything is snake_case on the wire (the SDK runs toSnakeCase over the event) — with one exception noted below.
type |
event |
address |
properties (verified) |
|---|---|---|---|
page |
null |
null |
{ name, category? } |
track |
custom name | null |
arbitrary; revenue/points/volume coerced to Number |
identify |
null |
wallet | { provider_name, rdns } |
detect |
null |
null |
{ provider_name, rdns } |
connect |
null |
wallet | { chain_id } |
disconnect |
null |
wallet | { chain_id } |
chain |
null |
wallet | { chain_id } |
signature |
null |
wallet | { status, chain_id, message } — status: requested/confirmed/rejected |
transaction |
null |
wallet | { status, chain_id, data, to, value, transaction_hash, function_name, function_args } |
Notes:
- The
userIdpassed toidentify()populates the top-leveluser_idfield, notproperties. properties.function_argskeeps its original camelCase inner keys (they're ABI parameter names, deliberately exempt from snake_casing) — e.g.{ "amountIn": "1", "tokenId": "7" }. Everything else is snake_case.
Lifecycle events are type: "track" with reserved names (Segment/RudderStack spec):
{ "type": "track", "event": "Application Installed", "properties": { "version": "1.1.0", "build": "42", "referrer": "example.com", "utm_source": "example.com" } }
{ "type": "track", "event": "Application Opened", "properties": { "version": "1.1.0", "build": "42", "from_background": false, "url": "myapp://Home" } }
{ "type": "track", "event": "Application Updated", "properties": { "version": "1.1.0", "build": "42", "previous_version": "1.0.0", "previous_build": "41" } }
{ "type": "track", "event": "Application Backgrounded", "properties": { "version": "1.1.0", "build": "42" } }Application Installed carries referrer/utm_* on Android only (Play Install Referrer) — see #51 (P-2207). iOS has no install referrer.
Dashboard panels — what I saw on a real mobile test project
Observed on a live iOS run against a test project (before #2033):
| Panel | Status |
|---|---|
| Pages | ❌ "No data found" — mobile screens absent → #2033 fixes |
| Devices / Browsers / OS | ❌ showed Others / unknown → #2033 fixes. Note browser will always be unknown for native apps — worth hiding or labelling that tab for channel=mobile |
| Custom events | ✅ Application Installed / Application Opened showed correctly |
| Countries | ✅ worked, but see the location caveat above — timezone-derived and incomplete |
| Channels / Referrers | ✅ populated |
| Wallets / Chains | ✅ worked (wallet events are channel-agnostic) |
| Sessions / Duration / Bounce |
Still open after #2033 (backend, not covered there):
- Install attribution in session/source rollups — the referrer on
Application Installedistype=track, whichprocess_sessions/process_sourcesskip (type='page'filter). It reaches per-user attribution and flows via screen events, so it's partial. - Lifecycle as metrics — no installs-over-time / D1–D7 retention off the lifecycle events.
session_id — resolved: the mobile SDK owns it
Decision: the mobile SDK sends its own session_id (a UUID that rotates after 30 minutes of inactivity), and ingestion honours it via the existing obj?.session_id || session_id path in handlerV0.ts#L153-L159. No backend change is needed — getformo/formono#2057 is closed.
Why not let ingestion derive it: the authorizer computes hash(dailySalt + domain + sourceIp + userAgent), which is a web design. For a native app all three inputs degenerate at once — no Origin header (so domain is the constant 'unknown'), the HTTP User-Agent is the client library's (okhttp/…, CFNetwork/… Darwin/…) and near-identical across users on the same app build, and carrier CGNAT shares IPs — so unrelated mobile users collapse into one session.
For ingestion, this means: session_id is present in the mobile payload (shown in the sample above) and should be taken from the body, not overwritten. Web is unchanged — it still has no session_id in the body and gets the authorizer's daily-rotating value.
Note this differs in semantics from web: mobile sessions are activity-based (30-min inactivity), web sessions are daily-rotating. data/events/common.mdx has been updated to describe both.
# Conflicts: # pnpm-lock.yaml
The events-gateway authorizer computes session_id as hash(dailySalt + domain + sourceIp + userAgent) — a privacy-friendly, daily-rotating identifier (Plausible-style, see apps/aws functions/events-gateway/authorizer.ts). handlerV0 then applies `obj?.session_id || session_id`, so a body-provided session_id overrides it; that escape hatch exists for server-side SDKs whose request IP/UA would otherwise collapse every call into one session. Emitting session_id from the mobile SDK hijacked that escape hatch and replaced the daily-rotating hash with a client UUID, giving web and mobile two different definitions of a session. Reverted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Team decision: the ingestion edge's session derivation is a web design. The authorizer computes hash(dailySalt + domain + sourceIp + userAgent), and for a native app all three inputs degenerate at once — no Origin header (domain is the constant 'unknown'), the HTTP User-Agent is the client library's and near-identical across users on the same app build, and carrier CGNAT shares IPs — so unrelated mobile users collapse into a single session. handlerV0 already honours a body-provided session_id (obj?.session_id || session_id), so the mobile SDK supplies its own with a 30-minute inactivity window. Reverts 3847a8a and corrects the rationale comment, which previously described the edge as cookie-based. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
let me know after you merge on main and I'll publish to https://npmjs.com/package/@formo/analytics-react-native and please do a final test after that by installing from npm |
#50 landed the installReferrer module with the dead iOS AdServices path still in it (react-native-ad-services-attribution does not exist on npm; the require is try/caught so it only ever no-ops). Resolved the storage.ts comment conflict in favour of this branch, which removes that path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3 participants
mobile.mp4
Summary
Tested the React Native SDK end-to-end against the ingestion pipeline and fixed the mobile analytics gaps at the SDK ↔ Tinybird seams, where mobile events were quietly producing wrong analytics. The SDK was already ~80% complete (lifecycle events, deep-link + install-referrer capture, device context).
Changes
1. Session tracking (
session_id) — the SDK sent nosession_id, so the pipeline (which groups sessions bysession_idwith no fallback) collapsed every mobile user of a project into one session, breaking Sessions / Bounce / Duration. The SDK now generates asession_idwith a 30-min inactivity timeout (persists across restarts, cleared byreset()). No backend equivalent — this is SDK-owned.2. User-agent detection — on the Expo path the SDK sent an empty
user_agent, so UA-based device/OS classification returnedunknown. The SDK now synthesizes a UA from the device info it already collects when the native one is absent. This is complementary to the backend's context-based device/OS derivation in #2033 (either path classifies mobile correctly); kept SDK-side so device/OS works independent of the backend rollout timing.3. Screen page paths — screen views are emitted as
page_url: app://<name>; the backend (#2033) derivesorigin(from the app identifier in context) andpage_path(by stripping theapp://scheme). An earlier SDK-side attempt to encode the bundle id in the URL host was reverted because it conflicts with that parser (it would leak the bundle id intopage_path).Also: crypto-secure UUID generation for
session_id/anonymous_id(Web Crypto, with a safe non-crashing fallback); per-event debug logging (gated behind debug logging); and repo-hygiene fixes surfaced once CI could run — regenerating a corruptedpnpm-lock.yamlonmain(385 duplicated keys) and patching transitive supply-chain advisories.Testing
pnpm test— 228 passing;pnpm typecheckclean;pnpm audit --prodclean.Application Installed/Application Opened) confirmed on the dashboard.Answers to the P-2199 questions
Related
docs.formo.soPR documentingsession_id,user_agent, and mobile screen events.P-2199
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.