getsentry / getsentry/sentry-react-native
Make getNewScreenTimeToDisplay race-free by moving it to the native store/pull model
- Dominant language
- TypeScript
- Stars
- 1.8k
- Forks
- 366
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 89
Description
### Background
#6722 fixes the fatal `JavaCallback was already settled` crash from `getNewScreenTimeToDisplay` under rapid navigation on Android (#6709). That fix makes losing the race **survivable** (crash → dropped measurement) but does not close the race window itself.
### Why the window is intrinsic to the current design
`RNSentryTimeToDisplay.getTimeToDisplay` schedules a deferred `Choreographer` frame callback that resolves a live `Promise` *after* the frame renders. The producer (native frame callback) and consumer (JS promise / backing `JavaCallback`) have independent lifetimes:
- Native gets **no teardown signal** when a navigation is superseded, so it can't cancel the pending frame callback.
- `com.facebook.react.bridge.Promise` exposes **no validity query**, and even a check would be TOCTOU.
- The deferred settle **is** the measurement (time to first frame after navigation), so we can't resolve early to dodge the race.
The only atomic option at that layer is attempt-and-catch — which is what #6722 does.
### Proposed race-free design
Move `getNewScreenTimeToDisplay` to the **store/pull model that the other TTID paths already use**: the frame callback writes the timestamp into the native `screenIdToRenderDuration` map (via `putTimeToDisplayFor`), and JS **pops** it later via `popTimeToDisplayFor` — same pattern as `ttid-`, `ttfd-`, and `ttid-navigation-` (see `timeToDisplayIntegration.ts`). A stored value has no long-lived bridge callback, so there is no "already settled" failure mode at all.
### Scope / considerations
- Bridge spec change (`NativeRNSentry.ts`) → **breaking**, needs a `@deprecated` migration path.
- Must be implemented on **both** platforms (Android + iOS — the iOS variant of the settle-after-teardown risk was deferred in #6722).
- Changes the fallback contract in `sentryeventemitterfallback.ts`, which currently races the native promise against `requestAnimationFrame`; a pull model changes that timing and needs care.
### Acceptance criteria
- [ ] `getNewScreenTimeToDisplay`'s deferred measurement no longer relies on resolving a long-lived promise from a frame callback.
- [ ] No behavioral regression in TTID measurements on either architecture (New + Old) or platform (iOS + Android).
- [ ] Deprecation path for the bridge API change.
Contributor guide
Assessment
This issue has not been assessed yet.