Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Replay Details - Bug] Seeking to a new location in replay can cause timer to be desynced #67727

Closed
Tracked by #63255
billyvg opened this issue Mar 26, 2024 · 0 comments · Fixed by #68212
Closed
Tracked by #63255

[Replay Details - Bug] Seeking to a new location in replay can cause timer to be desynced #67727

billyvg opened this issue Mar 26, 2024 · 0 comments · Fixed by #68212

Comments

@billyvg
Copy link
Member

billyvg commented Mar 26, 2024 •
edited by michellewzhang
Loading

This is a side affect of not eagerly loading all videos in #67657

To reproduce:

  • Load the replay
  • Open dev tools -> Network tab -> Set throttling to "Slow 3G"
  • Play the replay
  • Seek to near end of replay (where videos are not loaded)

The timer continues to tick while the video is loading, causing the timer to be desynced from video.

We should implement some sort of buffering/pausing of the timer while the video is loading to prevent this.

Related: #67712
@billyvg billyvg mentioned this issue Mar 26, 2024
Open
@michellewzhang michellewzhang mentioned this issue Mar 28, 2024
Merged
michellewzhang added a commit that referenced this issue Mar 28, 2024
@michellewzhang
fix(replay): when seeking while mobile replay is playing, pause old v…
66d3c86 
…ideo (#67841)

- for mobile replays, after pressing play, and then seeking to a new
later part of the video, the replay was still playing an "old" segment
and not the one we wanted, because we weren't pausing the old video.
this PR fixes this bug by pausing the old video before playing the next
(correct) video.
- fixes #67725
- there's still a bug where the later video is still buffering but the
timer/player keeps going, so we need to fix that:
  - #67712
  - #67727)


https://github.com/getsentry/sentry/assets/56095982/b3d4575c-739d-4249-8e6b-4d4c26847f2e
@michellewzhang michellewzhang changed the title [Replay Details] Seeking to a new location in replay can cause timer to be desynced [Replay Details - Bug] Seeking to a new location in replay can cause timer to be desynced Mar 28, 2024
This was referenced Apr 3, 2024
Merged
Closed
Closed
billyvg added a commit that referenced this issue Apr 4, 2024
@billyvg @michellewzhang
@ryan953 @getsantry
feat(replay): Add "buffering" state for mobile replays (#68212)
df944b2 
Adds a buffering state for mobile replays to show a "Buffering" messages
when it is busy. Also fixes some timer syncing issues when buffering/not
ready.

- Fixes #67727
- Fixes #67712
- Fixes #68111


To test locally: change `PRELOAD_BUFFER` in `videoReplayer` from "3" to
"1"
To test on preview deployment: change browser throttling to 3g *after*
Sentry UI has loaded and before you play the replay:


![image](https://github.com/getsentry/sentry/assets/79684/0b165bce-e2c3-4417-a6ef-e00887dd3c1f)


Preview (via removing preload):

[Screen Recording 2024-04-03 at
17.11.59.mov](https://github.com/getsentry/sentry/assets/79684/775f4006-8c6c-4acd-9e69-f833a29e43e9)

---------

Co-authored-by: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Co-authored-by: Ryan Albrecht <ryan.albrecht@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
shellmayr pushed a commit that referenced this issue Apr 10, 2024
@billyvg @michellewzhang
@ryan953 @getsantry @shellmayr
feat(replay): Add "buffering" state for mobile replays (#68212)
d20e50a 
Adds a buffering state for mobile replays to show a "Buffering" messages
when it is busy. Also fixes some timer syncing issues when buffering/not
ready.

- Fixes #67727
- Fixes #67712
- Fixes #68111


To test locally: change `PRELOAD_BUFFER` in `videoReplayer` from "3" to
"1"
To test on preview deployment: change browser throttling to 3g *after*
Sentry UI has loaded and before you play the replay:


![image](https://github.com/getsentry/sentry/assets/79684/0b165bce-e2c3-4417-a6ef-e00887dd3c1f)


Preview (via removing preload):

[Screen Recording 2024-04-03 at
17.11.59.mov](https://github.com/getsentry/sentry/assets/79684/775f4006-8c6c-4acd-9e69-f833a29e43e9)

---------

Co-authored-by: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Co-authored-by: Ryan Albrecht <ryan.albrecht@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(replay): Add "buffering" state for mobile replays getsentry/sentry
1 participant
@billyvg