Skip to content

fix(replay): Don't build a Replayer for loading or empty replays - #128811

Open
sentry-junior[bot] wants to merge 2 commits into
masterfrom
fix/replay-player-context-after-load
Open

sentry-junior[bot] wants to merge 2 commits into
masterfrom
fix/replay-player-context-after-load

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Replay details could crash with "Replayer need at least 2 events." after prev/next (or back/forward) navigation to a replay whose record was already cached.

The repro case for this is a bit complicated!

  1. Click on any replay in the Replay List view
  2. Wait for full load of the replay
  3. Click "Next" in the breadcrumb
  4. Wait for load of the replay header but not the load of the replay body
  5. Navigate back to the previous replay via the breadcrumb
  6. Navigate forward to the second replay via the breadcrumb

What happens is that the second navigation tries to get the second replay information for TanStack's cache, but there aren't any actual replay frames in there. The player crashes because the render chain is such that it doesn't check for fetching status of the replay when the navigation is updated.

The full fix here would be to reconsider the structure of the replay contexts and how they reload, but that's a much bigger task. For now, manually check for this case by avoiding inflating an invalid replay, and making sure the fetching status is passed to the page component (previously this was not needed because it only showed up when it was fully loaded)

Fixes JAVASCRIPT-3FVR
Fixes JAVASCRIPT-39VV
Fixes REPLAY-1021

via George Gritsouk.

--

View Junior Session [Sentry]

@linear-code

linear-code Bot commented Oct 26, 2026

Copy link
Copy Markdown

REPLAY-1021

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 26, 2026

@gggritso gggritso left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting start, a few questions

Comment thread static/app/views/explore/replays/detail/body/replayDetailsProviders.tsx Outdated
Comment thread static/app/views/explore/replays/detail/body/replayDetailsProviders.spec.tsx Outdated
Comment thread static/app/views/explore/replays/detail/body/replayDetailsProviders.tsx Outdated
ReplayDetailsProviders hard-coded isFetching={false}, so ReplayContext's
fetching guard in initRoot never fired. When navigating from a loaded replay
to one whose record was cached, the existing Replayer was rebuilt in place
from the new reader, which could still be loading or have fewer than two
frames. rrweb's Replayer throws in that case.

Pass the reader's real fetching state, and have initRoot tear down any
existing Replayer instead of constructing one when there are fewer than two
events.

Fixes JAVASCRIPT-3FVR

Fixes JAVASCRIPT-39VV

Fixes REPLAY-1021

Co-Authored-By: George Gritsouk <george.gritsouk@sentry.io>
@sentry-junior
sentry-junior Bot force-pushed the fix/replay-player-context-after-load branch from 68cb55a to 08d7463 Compare October 26, 2026 23:13
@sentry-junior sentry-junior Bot changed the title fix(replay): Mount player contexts only once the replay is playable fix(replay): Don't build a Replayer for loading or empty replays Oct 26, 2026

@gggritso gggritso left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty good just one small ask

Comment thread static/app/components/replays/replayContext.tsx Outdated
Comment thread static/app/views/explore/replays/details.tsx Outdated

Copy link
Copy Markdown
Member

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 82c1c71. Configure here.

@gggritso
gggritso marked this pull request as ready for review October 28, 2026 01:36
@gggritso
gggritso requested a review from a team as a code owner October 28, 2026 01:36

This branch was successfully deployed

1 active deployment
Preview — 82c1c713 Deployed Oct 28, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant