getsentry / getsentry/sentry-react-native

Make getNewScreenTimeToDisplay race-free by moving it to the native store/pull model

Ouverte
#6,723 1 commentaire 0 réactions 1 personne assignée Voir sur GitHub

@antonis y travaille déjà.

Depuis le 14/9/2026.

Improvement React-Native
Langage dominant
TypeScript
Étoiles
1.8k
Forks
367
Merge moyen
1 j 6 h
PR mergées (30 j)
94

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.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.