fixes and add tests

Author: michellewzhang

static/app/components/replays/replayContext.tsx

@@ -495,7 +495,7 @@ function ProviderNonMemo({
           setVideoBuffering(buffering);
         },
         clipWindow,
-        durationMs: durationMs,
+        durationMs,
       });
       // `.current` is marked as readonly, but it's safe to set the value from
       // inside a `useEffect` hook.

static/app/components/replays/videoReplayer.spec.tsx

@@ -87,7 +87,7 @@ describe('VideoReplayer - no starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     // @ts-expect-error private
     expect(inst._currentIndex).toEqual(0);
@@ -113,7 +113,7 @@ describe('VideoReplayer - no starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     const playPromise = inst.play(18100);
     // @ts-expect-error private
@@ -142,7 +142,7 @@ describe('VideoReplayer - no starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     const playPromise = inst.play(50000);
     // 15000 -> 20000 is a gap, so player should start playing @ index 3, from
@@ -165,7 +165,7 @@ describe('VideoReplayer - no starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     const playPromise = inst.play(0);
     jest.advanceTimersByTime(2500);
@@ -185,7 +185,7 @@ describe('VideoReplayer - no starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 50,
+      durationMs: 50000,
     });
     // play at segment 7
     const playPromise = inst.play(45_003);
@@ -220,7 +220,7 @@ describe('VideoReplayer - no starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 60,
+      durationMs: 55000,
     });
     // play at segment 7
     const playPromise = inst.play(45_003);
@@ -294,7 +294,7 @@ describe('VideoReplayer - with starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     // @ts-expect-error private
     expect(inst._currentIndex).toEqual(0);
@@ -318,7 +318,7 @@ describe('VideoReplayer - with starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     const playPromise = inst.play(18100);
     // @ts-expect-error private
@@ -347,7 +347,7 @@ describe('VideoReplayer - with starting gap', () => {
       onFinished: jest.fn(),
       onLoaded: jest.fn(),
       onBuffer: jest.fn(),
-      durationMs: 40,
+      durationMs: 40000,
     });
     const playPromise = inst.play(50000);
     // 15000 -> 20000 is a gap, so player should start playing @ index 3, from
@@ -361,3 +361,115 @@ describe('VideoReplayer - with starting gap', () => {
     expect(inst.getVideo(inst._currentIndex)?.currentTime).toEqual(5);
   });
 });
+
+describe('VideoReplayer - with ending gap', () => {
+  beforeEach(() => {
+    jest.clearAllTimers();
+  });
+
+  const attachments = [
+    {
+      id: 0,
+      timestamp: 2500,
+      duration: 5000,
+    },
+    // no gap
+    {
+      id: 1,
+      timestamp: 5000,
+      duration: 5000,
+    },
+    {
+      id: 2,
+      timestamp: 10_001,
+      duration: 5000,
+    },
+    // 5 second gap
+    {
+      id: 3,
+      timestamp: 20_000,
+      duration: 5000,
+    },
+    // 5 second gap
+    {
+      id: 4,
+      timestamp: 30_000,
+      duration: 5000,
+    },
+    {
+      id: 5,
+      timestamp: 35_002,
+      duration: 5000,
+    },
+  ];
+
+  it('keeps playing until the end if there is an ending gap', async () => {
+    const root = document.createElement('div');
+    const inst = new VideoReplayer(attachments, {
+      videoApiPrefix: '/foo/',
+      root,
+      start: 0,
+      onFinished: jest.fn(),
+      onLoaded: jest.fn(),
+      onBuffer: jest.fn(),
+      durationMs: 50000,
+    });
+    // actual length of the segments is 40s
+    // 10s gap at the end
+
+    // play at the last segment
+    const playPromise = inst.play(36000);
+    await playPromise;
+    jest.advanceTimersByTime(4000);
+
+    // we're still within the last segment (5)
+    // @ts-expect-error private
+    expect(inst._currentIndex).toEqual(5);
+    expect(inst.getCurrentTime()).toEqual(40000);
+
+    // now we are in the gap
+    // timer should still be going since the duration is 50s
+    jest.advanceTimersByTime(5000);
+    // @ts-expect-error private
+    expect(inst._isPlaying).toEqual(true);
+
+    // a long time passes
+    // ensure the timer stops at the end duration (50s)
+    jest.advanceTimersByTime(50000);
+    expect(inst.getCurrentTime()).toEqual(50000);
+    // @ts-expect-error private
+    expect(inst._isPlaying).toEqual(false);
+  });
+
+  it('ends at the proper time if seeking into a gap', async () => {
+    const root = document.createElement('div');
+    const inst = new VideoReplayer(attachments, {
+      videoApiPrefix: '/foo/',
+      root,
+      start: 0,
+      onFinished: jest.fn(),
+      onLoaded: jest.fn(),
+      onBuffer: jest.fn(),
+      durationMs: 50000,
+    });
+    // actual length of the segments is 40s
+    // 10s gap at the end
+
+    // play at the gap
+    const playPromise = inst.play(40002);
+    await playPromise;
+    jest.advanceTimersByTime(4000);
+
+    // we should be still playing in the gap
+    expect(inst.getCurrentTime()).toEqual(44002);
+    // @ts-expect-error private
+    expect(inst._isPlaying).toEqual(true);
+
+    // a long time passes
+    // ensure the timer stops at the end duration (50s)
+    jest.advanceTimersByTime(50000);
+    expect(inst.getCurrentTime()).toBeLessThan(50100);
+    // @ts-expect-error private
+    expect(inst._isPlaying).toEqual(false);
+  });
+});

static/app/components/replays/videoReplayer.tsx

@@ -115,7 +115,11 @@ export class VideoReplayer {
         this.stopReplay();
       });
     } else {
-      // Otherwise, if there's no clip window set, we should
+      // Tell the timer to stop at the replay end
+      this._timer.addNotificationAtTime(this._durationMs, () => {
+        this.stopReplay();
+      });
+      // If there's no clip window set, we should
       // load the first segment by default so that users are not staring at a
       // blank replay. This initially caused some issues
       // (https://github.com/getsentry/sentry/pull/67911), but the problem was
@@ -251,12 +255,16 @@ export class VideoReplayer {
     this._isPlaying = true;
     this._timer.start(videoOffsetMs);
 
-    // This is used when a replay clip is restarted
+    // This is used when a replay is restarted
     // Add another stop notification so the timer doesn't run over
     if (this._clipDuration) {
       this._timer.addNotificationAtTime(this._clipDuration, () => {
         this.stopReplay();
       });
+    } else {
+      this._timer.addNotificationAtTime(this._durationMs, () => {
+        this.stopReplay();
+      });
     }
   }
 
@@ -301,13 +309,16 @@ export class VideoReplayer {
 
     // No more segments
     if (nextIndex >= this._attachments.length) {
-      // If we're at the end of a segment, but there's a gap
-      // at the end, force the replay to play until the end duration
-      // rather than stopping right away.
-      this._timer.addNotificationAtTime(this._durationMs, () => {
-        this.stopReplay();
-      });
-      return;
+      if (this.getCurrentTime() < this._durationMs) {
+        // If we're at the end of a segment, but there's a gap
+        // at the end, force the replay to play until the end duration
+        // rather than stopping right away.
+        this._timer.addNotificationAtTime(this._durationMs, () => {
+          this.stopReplay();
+        });
+        return;
+      }
+      this.stopReplay();
     }
 
     // Final check in case replay was stopped immediately after a video