fix: refactor update animation to do less (#21901)

PostHog · Apr 26, 2024 · 226f768 · 226f768
1 parent f87b54c
commit 226f768
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 52 deletions.
diff --git a/frontend/src/scenes/session-recordings/player/controller/PlayerSeekbarPreview.tsx b/frontend/src/scenes/session-recordings/player/controller/PlayerSeekbarPreview.tsx
@@ -1,7 +1,7 @@
 import { BindLogic, useActions, useValues } from 'kea'
 import useIsHovering from 'lib/hooks/useIsHovering'
 import { colonDelimitedDuration } from 'lib/utils'
-import { MutableRefObject, useEffect, useRef, useState } from 'react'
+import { memo, MutableRefObject, useEffect, useRef, useState } from 'react'
 import { useDebouncedCallback } from 'use-debounce'
 
 import { PlayerFrame } from '../PlayerFrame'
@@ -28,7 +28,7 @@ const PlayerSeekbarPreviewFrame = ({
 }: { percentage: number; isVisible: boolean } & Omit<
     PlayerSeekbarPreviewProps,
     'seekBarRef' | 'activeMs'
->): JSX.Element => {
+>): JSX.Element | null => {
     const { sessionRecordingId, logicProps } = useValues(sessionRecordingPlayerLogic)
 
     const seekPlayerLogicProps: SessionRecordingPlayerLogicProps = {
@@ -66,7 +66,7 @@ const PlayerSeekbarPreviewFrame = ({
     )
 }
 
-export function PlayerSeekbarPreview({ minMs, maxMs, seekBarRef, activeMs }: PlayerSeekbarPreviewProps): JSX.Element {
+function _PlayerSeekbarPreview({ minMs, maxMs, seekBarRef, activeMs }: PlayerSeekbarPreviewProps): JSX.Element {
     const [percentage, setPercentage] = useState<number>(0)
     const ref = useRef<HTMLDivElement>(null)
     const fixedUnits = maxMs / 1000 > 3600 ? 3 : 2
@@ -125,3 +125,5 @@ export function PlayerSeekbarPreview({ minMs, maxMs, seekBarRef, activeMs }: Pla
         </div>
     )
 }
+
+export const PlayerSeekbarPreview = memo(_PlayerSeekbarPreview)
diff --git a/frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts b/frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts
@@ -352,6 +352,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
         logicProps: [() => [(_, props) => props], (props): SessionRecordingPlayerLogicProps => props],
         playlistLogic: [() => [(_, props) => props], (props) => props.playlistLogic],
 
+        roughAnimationFPS: [(s) => [s.playerSpeed], (playerSpeed) => playerSpeed * (1000 / 60)],
         currentPlayerState: [
             (s) => [
                 s.playingState,
@@ -813,47 +814,33 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
             const rrwebPlayerTime = values.player?.replayer?.getCurrentTime()
             let newTimestamp = values.fromRRWebPlayerTime(rrwebPlayerTime)
 
-            const skip = values.playerSpeed * (1000 / 60) // rough animation fps
-            if (
-                rrwebPlayerTime !== undefined &&
-                newTimestamp !== undefined &&
-                (values.currentPlayerState === SessionPlayerState.PLAY ||
-                    values.currentPlayerState === SessionPlayerState.SKIP) &&
-                values.timestampChangeTracking.timestampMatchesPrevious > 10
-            ) {
-                // NOTE: We should investigate if this is still happening - logging to posthog recording so we can find this in the future
-                posthog.sessionRecording?.log(
-                    'stuck session player detected - this indicates an issue with the segmenter',
-                    'warn'
-                )
-                cache.debug?.('stuck session player detected', {
-                    timestampChangeTracking: values.timestampChangeTracking,
-                    currentSegment: values.currentSegment,
-                    snapshots: values.sessionPlayerData.snapshotsByWindowId[values.currentSegment?.windowId ?? ''],
-                    player: values.player,
-                    meta: values.player?.replayer.getMetaData(),
-                    rrwebPlayerTime,
-                    segments: values.sessionPlayerData.segments,
-                    segmentIndex:
-                        values.currentSegment && values.sessionPlayerData.segments.indexOf(values.currentSegment),
-                })
-
-                actions.skipPlayerForward(rrwebPlayerTime, skip)
-                newTimestamp = newTimestamp + skip
-            }
-
             if (newTimestamp == undefined && values.currentTimestamp) {
                 // This can happen if the player is not loaded due to us being in a "gap" segment
                 // In this case, we should progress time forward manually
-
                 if (values.currentSegment?.kind === 'gap') {
                     cache.debug?.('gap segment: skipping forward')
-                    newTimestamp = values.currentTimestamp + skip
+                    newTimestamp = values.currentTimestamp + values.roughAnimationFPS
                 }
             }
 
+            // If we're beyond buffered position, set to buffering
+            if (values.currentSegment?.kind === 'buffer') {
+                // Pause only the animation, not our player, so it will restart
+                // when the buffering progresses
+                values.player?.replayer?.pause()
+                actions.startBuffer()
+                actions.setErrorPlayerState(false)
+                cache.debug('buffering')
+                return
+            }
+
+            if (newTimestamp == undefined) {
+                // no newTimestamp is unexpected, bail out
+                return
+            }
+
             // If we are beyond the current segment then move to the next one
-            if (newTimestamp && values.currentSegment && newTimestamp > values.currentSegment.endTimestamp) {
+            if (values.currentSegment && newTimestamp > values.currentSegment.endTimestamp) {
                 const nextSegment = values.segmentForTimestamp(newTimestamp)
 
                 if (nextSegment) {
@@ -873,22 +860,9 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
                 return
             }
 
-            // If we're beyond buffered position, set to buffering
-            if (values.currentSegment?.kind === 'buffer') {
-                // Pause only the animation, not our player, so it will restart
-                // when the buffering progresses
-                values.player?.replayer?.pause()
-                actions.startBuffer()
-                actions.setErrorPlayerState(false)
-                cache.debug('buffering')
-                return
-            }
-
-            if (newTimestamp !== undefined) {
-                // The normal loop. Progress the player position and continue the loop
-                actions.setCurrentTimestamp(newTimestamp)
-                cache.timer = requestAnimationFrame(actions.updateAnimation)
-            }
+            // The normal loop. Progress the player position and continue the loop
+            actions.setCurrentTimestamp(newTimestamp)
+            cache.timer = requestAnimationFrame(actions.updateAnimation)
         },
         stopAnimation: () => {
             if (cache.timer) {
@@ -1002,7 +976,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
         },
     })),
 
-    subscriptions(({ actions }) => ({
+    subscriptions(({ actions, values }) => ({
         sessionPlayerData: (next, prev) => {
             const hasSnapshotChanges = next?.snapshotsByWindowId !== prev?.snapshotsByWindowId
 
@@ -1012,6 +986,17 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
                 actions.syncSnapshotsWithPlayer()
             }
         },
+        timestampChangeTracking: (next) => {
+            if (next.timestampMatchesPrevious < 10) {
+                return
+            }
+
+            const rrwebPlayerTime = values.player?.replayer?.getCurrentTime()
+
+            if (rrwebPlayerTime !== undefined && values.currentPlayerState === SessionPlayerState.PLAY) {
+                actions.skipPlayerForward(rrwebPlayerTime, values.roughAnimationFPS)
+            }
+        },
     })),
 
     beforeUnmount(({ values, actions, cache, props }) => {