From e34a40e4dcd5a7433dad56afe51ce17a1cf9c3ff Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 22 Jul 2024 11:41:03 +0200 Subject: [PATCH 1/3] feat: modify start() and startBuffering() behaviour --- packages/replay-internal/src/integration.ts | 3 +-- packages/replay-internal/src/replay.ts | 11 ++++++---- .../test/integration/start.test.ts | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/packages/replay-internal/src/integration.ts b/packages/replay-internal/src/integration.ts index 2b1ef71287ae..da089b8ea113 100644 --- a/packages/replay-internal/src/integration.ts +++ b/packages/replay-internal/src/integration.ts @@ -226,7 +226,7 @@ export class Replay implements Integration { /** * Start a replay regardless of sampling rate. Calling this will always - * create a new session. Will throw an error if replay is already in progress. + * create a new session. Will log a message if replay is already in progress. * * Creates or loads a session, attaches listeners to varying events (DOM, * PerformanceObserver, Recording, Sentry SDK, etc) @@ -235,7 +235,6 @@ export class Replay implements Integration { if (!this._replay) { return; } - this._replay.start(); } diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index 1d3b1ef340c4..a0ef13276e1a 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -288,18 +288,20 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Start a replay regardless of sampling rate. Calling this will always - * create a new session. Will throw an error if replay is already in progress. + * create a new session. Will log a message if replay is already in progress. * * Creates or loads a session, attaches listeners to varying events (DOM, * _performanceObserver, Recording, Sentry SDK, etc) */ public start(): void { if (this._isEnabled && this.recordingMode === 'session') { - throw new Error('Replay recording is already in progress'); + DEBUG_BUILD && logger.info('[Replay] Recording is already in progress'); + return; } if (this._isEnabled && this.recordingMode === 'buffer') { - throw new Error('Replay buffering is in progress, call `flush()` to save the replay'); + DEBUG_BUILD && logger.info('[Replay] Buffering is in progress, call `flush()` to save the replay'); + return; } logInfoNextTick('[Replay] Starting replay in session mode', this._options._experiments.traceInternals); @@ -335,7 +337,8 @@ export class ReplayContainer implements ReplayContainerInterface { */ public startBuffering(): void { if (this._isEnabled) { - throw new Error('Replay recording is already in progress'); + DEBUG_BUILD && logger.info('[Replay] Buffering is in progress, call `flush()` to save the replay'); + return; } logInfoNextTick('[Replay] Starting replay in buffer mode', this._options._experiments.traceInternals); diff --git a/packages/replay-internal/test/integration/start.test.ts b/packages/replay-internal/test/integration/start.test.ts index dff5df38b53d..349f3e1b14d9 100644 --- a/packages/replay-internal/test/integration/start.test.ts +++ b/packages/replay-internal/test/integration/start.test.ts @@ -3,6 +3,7 @@ import { vi } from 'vitest'; import { getClient } from '@sentry/core'; import type { Transport } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_EXPIRE_DURATION } from '../../src/constants'; import type { Replay } from '../../src/integration'; import type { ReplayContainer } from '../../src/replay'; @@ -49,4 +50,24 @@ describe('Integration | start', () => { recordingPayloadHeader: { segment_id: 0 }, }); }); + + it('does not start recording once replay is already in progress', async () => { + const startRecordingSpy = vi.spyOn(replay, 'startRecording').mockImplementation(() => undefined); + + integration.start(); + replay.start(); + replay.start(); + + expect(startRecordingSpy).toHaveBeenCalledTimes(1); + }); + + it('does not start buffering once replay is already in progress', async () => { + const startRecordingSpy = vi.spyOn(replay, 'startRecording').mockImplementation(() => undefined); + + integration.startBuffering(); + replay.startBuffering(); + replay.startBuffering(); + + expect(startRecordingSpy).toHaveBeenCalledTimes(1); + }); }); From 8c83f4de38424c9089cc3fe02216d10f96a076b5 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 22 Jul 2024 12:01:57 +0200 Subject: [PATCH 2/3] chore: lint --- packages/replay-internal/test/integration/start.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/replay-internal/test/integration/start.test.ts b/packages/replay-internal/test/integration/start.test.ts index 349f3e1b14d9..063dc5babc7a 100644 --- a/packages/replay-internal/test/integration/start.test.ts +++ b/packages/replay-internal/test/integration/start.test.ts @@ -3,7 +3,6 @@ import { vi } from 'vitest'; import { getClient } from '@sentry/core'; import type { Transport } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_EXPIRE_DURATION } from '../../src/constants'; import type { Replay } from '../../src/integration'; import type { ReplayContainer } from '../../src/replay'; From e5e70f93ccd71c400ecab91653a7eeb3a3fa5cde Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 22 Jul 2024 12:31:02 +0200 Subject: [PATCH 3/3] feat: start recording when calling flush with replay disabled --- packages/replay-internal/src/integration.ts | 9 ++++++++- .../replay-internal/test/integration/flush.test.ts | 14 +++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/replay-internal/src/integration.ts b/packages/replay-internal/src/integration.ts index da089b8ea113..8f70e3099a97 100644 --- a/packages/replay-internal/src/integration.ts +++ b/packages/replay-internal/src/integration.ts @@ -264,13 +264,20 @@ export class Replay implements Integration { /** * If not in "session" recording mode, flush event buffer which will create a new replay. + * If replay is not enabled, a new session replay is started. * Unless `continueRecording` is false, the replay will continue to record and * behave as a "session"-based replay. * * Otherwise, queue up a flush. */ public flush(options?: SendBufferedReplayOptions): Promise { - if (!this._replay || !this._replay.isEnabled()) { + if (!this._replay) { + return Promise.resolve(); + } + + // assuming a session should be recorded in this case + if (!this._replay.isEnabled()) { + this._replay.start(); return Promise.resolve(); } diff --git a/packages/replay-internal/test/integration/flush.test.ts b/packages/replay-internal/test/integration/flush.test.ts index 31fd8a91a5e2..03dc3563e292 100644 --- a/packages/replay-internal/test/integration/flush.test.ts +++ b/packages/replay-internal/test/integration/flush.test.ts @@ -9,6 +9,7 @@ import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import * as SentryUtils from '@sentry/utils'; import { DEFAULT_FLUSH_MIN_DELAY, MAX_REPLAY_DURATION, WINDOW } from '../../src/constants'; +import type { Replay } from '../../src/integration'; import type { ReplayContainer } from '../../src/replay'; import { clearSession } from '../../src/session/clearSession'; import type { EventBuffer } from '../../src/types'; @@ -33,6 +34,7 @@ describe('Integration | flush', () => { const { record: mockRecord } = mockRrweb(); + let integration: Replay; let replay: ReplayContainer; let mockSendReplay: MockSendReplay; let mockFlush: MockFlush; @@ -45,7 +47,7 @@ describe('Integration | flush', () => { domHandler = handler; }); - ({ replay } = await mockSdk()); + ({ replay, integration } = await mockSdk()); mockSendReplay = vi.spyOn(SendReplay, 'sendReplay'); mockSendReplay.mockImplementation( @@ -484,4 +486,14 @@ describe('Integration | flush', () => { // Start again for following tests await replay.start(); }); + + /** + * Assuming the user wants to record a session + * when calling flush() without replay being enabled + */ + it('starts recording a session when replay is not enabled', () => { + integration.stop(); + integration.flush(); + expect(replay.isEnabled()).toBe(true); + }); });