diff --git a/static/app/components/replays/replayContext.tsx b/static/app/components/replays/replayContext.tsx index 8b90d20f6571ab..633199e05397a1 100644 --- a/static/app/components/replays/replayContext.tsx +++ b/static/app/components/replays/replayContext.tsx @@ -495,6 +495,7 @@ function ProviderNonMemo({ setVideoBuffering(buffering); }, clipWindow, + durationMs, }); // `.current` is marked as readonly, but it's safe to set the value from // inside a `useEffect` hook. @@ -521,6 +522,7 @@ function ProviderNonMemo({ startTimestampMs, startTimeOffsetMs, clipWindow, + durationMs, ] ); diff --git a/static/app/components/replays/videoReplayer.spec.tsx b/static/app/components/replays/videoReplayer.spec.tsx index ad7363e904ae76..41e80e16547f03 100644 --- a/static/app/components/replays/videoReplayer.spec.tsx +++ b/static/app/components/replays/videoReplayer.spec.tsx @@ -87,6 +87,7 @@ describe('VideoReplayer - no starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); // @ts-expect-error private expect(inst._currentIndex).toEqual(0); @@ -112,6 +113,7 @@ describe('VideoReplayer - no starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); const playPromise = inst.play(18100); // @ts-expect-error private @@ -140,6 +142,7 @@ describe('VideoReplayer - no starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); const playPromise = inst.play(50000); // 15000 -> 20000 is a gap, so player should start playing @ index 3, from @@ -162,6 +165,7 @@ describe('VideoReplayer - no starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); const playPromise = inst.play(0); jest.advanceTimersByTime(2500); @@ -181,6 +185,7 @@ describe('VideoReplayer - no starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 50000, }); // play at segment 7 const playPromise = inst.play(45_003); @@ -215,6 +220,7 @@ describe('VideoReplayer - no starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 55000, }); // play at segment 7 const playPromise = inst.play(45_003); @@ -288,6 +294,7 @@ describe('VideoReplayer - with starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); // @ts-expect-error private expect(inst._currentIndex).toEqual(0); @@ -311,6 +318,7 @@ describe('VideoReplayer - with starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); const playPromise = inst.play(18100); // @ts-expect-error private @@ -339,6 +347,7 @@ describe('VideoReplayer - with starting gap', () => { onFinished: jest.fn(), onLoaded: jest.fn(), onBuffer: jest.fn(), + durationMs: 40000, }); const playPromise = inst.play(50000); // 15000 -> 20000 is a gap, so player should start playing @ index 3, from @@ -352,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(60000); + 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 at the end', 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(60000); + 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 48ddb6b0fbe906..cc6e877442ce01 100644 --- a/static/app/components/replays/videoReplayer.tsx +++ b/static/app/components/replays/videoReplayer.tsx @@ -14,6 +14,7 @@ interface OffsetOptions { } interface VideoReplayerOptions { + durationMs: number; onBuffer: (isBuffering: boolean) => void; onFinished: () => void; onLoaded: (event: any) => void; @@ -56,6 +57,7 @@ export class VideoReplayer { private _videos: Map; private _videoApiPrefix: string; private _clipDuration: number | undefined; + private _durationMs: number; public config: VideoReplayerConfig = { skipInactive: false, speed: 1.0, @@ -73,6 +75,7 @@ export class VideoReplayer { onFinished, onLoaded, clipWindow, + durationMs, }: VideoReplayerOptions ) { this._attachments = attachments; @@ -86,6 +89,7 @@ export class VideoReplayer { }; this._videos = new Map(); this._clipDuration = undefined; + this._durationMs = durationMs; this.wrapper = document.createElement('div'); if (root) { @@ -111,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 @@ -247,13 +255,14 @@ 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._timer.addNotificationAtTime( + this._clipDuration ? this._clipDuration : this._durationMs, + () => { this.stopReplay(); - }); - } + } + ); } /** @@ -297,8 +306,16 @@ export class VideoReplayer { // No more segments if (nextIndex >= this._attachments.length) { + 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(); - return; } // Final check in case replay was stopped immediately after a video @@ -598,8 +615,13 @@ export class VideoReplayer { const loadedSegmentIndex = await this.loadSegmentAtTime(videoOffsetMs); if (loadedSegmentIndex === undefined) { - // TODO: this shouldn't happen, loadSegment should load the previous - // segment until it's time to start the next segment + // If we end up here, we seeked into a gap + // at the end of the replay. + // This tells the timer to stop at the specified duration + // and prevents the timer from running infinitely. + this._timer.addNotificationAtTime(this._durationMs, () => { + this.stopReplay(); + }); return Promise.resolve(); }