fix(onboarding): register Replay integration during onboarding flow#114774
Merged
Conversation
useReplayForCriticalFlow's early return on Sentry.getReplay() being null was masking a real bug for SaaS users: the Replay integration is only registered by gsApp's useReplayInit, which mounts inside OrganizationHeader (under OrganizationLayout). Onboarding routes use OrganizationContainerRoute, so during the SCM onboarding flow the integration is never registered, getReplay() returns null, and the forced replay path no-ops. Move the implementation back behind a HookStore-registered gsApp impl that calls useReplayInit before tagging and flushing. useReplayInit now exposes a ready boolean so the critical-flow effect can wait for the dynamic import to resolve. The module-level singleton in useReplayInit makes the OrganizationHeader call later in the session a safe no-op, and the sample rates passed in match the existing defaults (isStaff ? 1.0 : 0.05, 1.0) so behavior outside onboarding is unchanged.
github-actions
Bot
added
the
Scope: Frontend
Contributor
📊 Type Coverage Diff✅ No new type safety issues introduced. Coverage: 93.46% |
Each useReplayInit call has its own setReady, and the init() closure inside its effect can call it directly. Concurrent callers each run init(), each await the cached import, and each fall through to their own setReady — the singleton replayRef guard still ensures only one registration. Late callers see replayRef !== null synchronously in the useState initializer.
Drops the previous simplification: when two useReplayInit callers are mounted around the same in-flight init, the initiator's local setReady only updates its own component. Without the listener Set, a parallel caller (e.g. OrganizationHeader mounting before onboarding's useReplayForCriticalFlow has resolved its dynamic import) would stay stuck on ready=false. Add comments explaining the listener pattern, the readyListeners Set's purpose, and why `replayRef !== null` is a valid initial-state check (replayRef is assigned synchronously immediately before client.addIntegration, with no await between them).
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Without an explicit `= null`, replayRef's runtime value is undefined, so `useState(() => replayRef !== null)` evaluates `undefined !== null` to true. The ready flag started true before the integration was registered, the critical-flow effect bailed out on the first run, and the later setReady(true) was a no-op so the effect never re-ran.
Verifies useReplayInit returns false on first render before the dynamic replay integration import resolves. This catches the regression where declaring `let replayRef: ... | null;` without an initializer caused `undefined !== null` to seed `ready` as true.
Use UseReplayForCriticalFlowOptions in the HookStore type map instead of inlining the option shape, matching the UseExperimentOptions precedent. The gsApp useReplayForCriticalFlow now calls out that useReplayInit runs unconditionally even when enabled or shouldForce is false: it is the same registration OrganizationHeader does post-onboarding, so running it early is benign and avoids a hooks-ordering violation. Update the replayRef invariant comment in useReplayInit to call out the absence of an `await` between the assignment and `client.addIntegration`, which is what makes the singleton check valid for late mounters.
evanpurkhiser
approved these changes
May 5, 2026
Member
evanpurkhiser
left a comment
evanpurkhiser
left a comment
There was a problem hiding this comment.
Looks good, but I think we probably should adjust the naming
Abdkhan14
reviewed
May 5, 2026
billyvg
approved these changes
May 5, 2026
Comment on lines
+14
to
+16
| // The Replay integration is normally registered by OrganizationHeader, | ||
| // which doesn't mount during /onboarding/* (those routes use | ||
| // OrganizationContainerRoute, not OrganizationLayout). Calling it here |
Member
There was a problem hiding this comment.
tbh I don't remember exactly how this works, but would it help if we registered it higher up in the tree? iirc useReplayInit used to depend on organization to be available as part of our rollout strategy, but that's no longer the case.
Move useReplayInit out of OrganizationHeader and into a new component:app-init hook mounted at the App root. OrganizationHeader only renders under OrganizationLayout, which doesn't cover /onboarding/* (those routes mount under OrganizationContainerRoute). With init at the App root the Replay integration registers for every route, so the comment block in useReplayForCriticalFlow is no longer describing a workaround. Switch the per-mount sampling gate from useState lazy-init to useMemo with an empty dep array. Two reviewers read the useState form as something that could update; useMemo signals "computed once" more clearly. Self-hosted is unchanged: no gsApp hook registered, so the App-level mount renders nothing and Replay stays off. Refs VDY-48
…layInit Rename the gsApp App-root component AppInit -> ReplayInit (and the hook key component:app-init -> component:replay-init) since Replay is all it drives. Split a new useReplayReady out of useReplayInit. It subscribes to the "integration registered" listener set without trying to drive init. useReplayForCriticalFlow uses it so the call site reads as "wait for Replay" rather than "redundantly initialize Replay" — the App-root ReplayInit is the only initializer now. The subscriber is still load-bearing: registration is async (dynamic import of @sentry/react), so a fresh load straight into a critical-flow route can fire the consumer's effect before the integration resolves. useReplayReady flipping triggers a re-run. Refs VDY-48
useMemo with empty deps is a perf hint React reserves the right to discard (e.g. offscreen prerendering). A recalc would re-roll Math.random() and could flip the sampling decision mid-session. useState's lazy initializer is guaranteed to run exactly once per mount. Comment added at the call site so the next reader doesn't try the same useMemo swap. Refs VDY-48
billyvg
approved these changes
May 11, 2026
## TL;DR Bump the forced replay sample rate for SCM onboarding from 30% to 50%, now that replays actually start in this flow. ## Stack - [PR 1](#114774): fix Replay integration not registering during onboarding - **PR 2 (this):** bump sample rate to 50% ## Details The prior PR fixes the bug where the Replay integration wasn't registered on `/onboarding/*` routes, so the 30% `sampleRate` we configured was effectively 0%. With registration working, 50% gives a faster signal on the SCM onboarding funnel without doubling baseline replay storage. Easy to dial back if volume is too high.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eb6f2a8. Configure here.
The gsApp implementation calls hooks internally, so the local variable must start with 'use' for eslint-plugin-react-hooks to enforce rules-of-hooks on the call site.
nikkikapadia
pushed a commit
that referenced
this pull request
May 12, 2026
…114774) ## TL;DR Forced replays for SCM onboarding never start because the Replay integration isn't registered on `/onboarding/*` routes (those mount under `OrganizationContainerRoute`, not `OrganizationLayout`). Hoist `useReplayInit` to the App root so registration covers every route. ## Stack - **PR 1 (this):** fix Replay integration not registering during onboarding - [PR 2](#114795): bump sample rate to 50% ## Details The forced replay path for SCM onboarding (`useReplayForCriticalFlow` at `static/app/views/onboarding/onboarding.tsx:201`) is silently no-op'ing for SaaS users. The hook bails out when `Sentry.getReplay()` returns null, and during onboarding it always does: gsApp's `useReplayInit` was mounted inside `OrganizationHeader`, which only renders under `OrganizationLayout`. The onboarding routes use `OrganizationContainerRoute` instead, so the Replay integration was never registered. Recording only kicked in once the user navigated into the org, which matches the symptom we observed (no replays tagged `critical_flow:scm_onboarding` over 30d, and tag-less recordings starting after the user leaves `/onboarding/`). Move `useReplayInit` to the App root via a new `component:replay-init` hook. The new gsApp `ReplayInit` component is mounted in `App` (sibling of all route trees), so registration happens once and covers `OrganizationContainerRoute` and `OrganizationLayout` alike. `useReplayInit` no longer mounts under `OrganizationHeader`. Registration is async (dynamic import of `@sentry/react`), so on a fresh load straight into `/onboarding/welcome/` the consumer's effect can still fire before the integration resolves. To handle that without making the consumer drive init, `useReplayReady` is split out of `useReplayInit` — it subscribes to a module-level listener set that flips when registration completes. `useReplayForCriticalFlow` calls `useReplayReady` instead of `useReplayInit`, so the call site reads as "wait for Replay" rather than "redundantly initialize Replay." Per-mount sample gate switched from `useState` lazy-init to `useMemo` with empty deps; the value never updates, and `useMemo` signals that more clearly. Same sample rates pass through (`isStaff ? 1.0 : 0.05`, `1.0`), so behavior outside onboarding is unchanged. Self-hosted falls through to the OSS noop (no gsApp hook registered), matching the prior behavior that motivated 48ee2e9.
3 tasks
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
4 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.
TL;DR
Forced replays for SCM onboarding never start because the Replay integration isn't registered on
/onboarding/*routes (those mount underOrganizationContainerRoute, notOrganizationLayout). HoistuseReplayInitto the App root so registration covers every route.Stack
Details
The forced replay path for SCM onboarding (
useReplayForCriticalFlowatstatic/app/views/onboarding/onboarding.tsx:201) is silently no-op'ing for SaaS users. The hook bails out whenSentry.getReplay()returns null, and during onboarding it always does: gsApp'suseReplayInitwas mounted insideOrganizationHeader, which only renders underOrganizationLayout. The onboarding routes useOrganizationContainerRouteinstead, so the Replay integration was never registered. Recording only kicked in once the user navigated into the org, which matches the symptom we observed (no replays taggedcritical_flow:scm_onboardingover 30d, and tag-less recordings starting after the user leaves/onboarding/).Move
useReplayInitto the App root via a newcomponent:replay-inithook. The new gsAppReplayInitcomponent is mounted inApp(sibling of all route trees), so registration happens once and coversOrganizationContainerRouteandOrganizationLayoutalike.useReplayInitno longer mounts underOrganizationHeader.Registration is async (dynamic import of
@sentry/react), so on a fresh load straight into/onboarding/welcome/the consumer's effect can still fire before the integration resolves. To handle that without making the consumer drive init,useReplayReadyis split out ofuseReplayInit— it subscribes to a module-level listener set that flips when registration completes.useReplayForCriticalFlowcallsuseReplayReadyinstead ofuseReplayInit, so the call site reads as "wait for Replay" rather than "redundantly initialize Replay."Per-mount sample gate switched from
useStatelazy-init touseMemowith empty deps; the value never updates, anduseMemosignals that more clearly.Same sample rates pass through (
isStaff ? 1.0 : 0.05,1.0), so behavior outside onboarding is unchanged. Self-hosted falls through to the OSS noop (no gsApp hook registered), matching the prior behavior that motivated 48ee2e9.