diff --git a/static/app/components/replays/replayContext.tsx b/static/app/components/replays/replayContext.tsx index 621e00095fba4c..633199e05397a1 100644 --- a/static/app/components/replays/replayContext.tsx +++ b/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. diff --git a/static/app/components/replays/videoReplayer.spec.tsx b/static/app/components/replays/videoReplayer.spec.tsx index 6d5b8c6c41f9e0..69bc7871586952 100644 --- a/static/app/components/replays/videoReplayer.spec.tsx +++ b/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); + }); +}); diff --git a/static/app/components/replays/videoReplayer.tsx b/static/app/components/replays/videoReplayer.tsx index f7a5f4f4b5dcd3..ff1aa889524f5a 100644 --- a/static/app/components/replays/videoReplayer.tsx +++ b/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