Skip to content

Commit

Permalink
feat(replay): Improve public Replay APIs (#13000)
Browse files Browse the repository at this point in the history
  • Loading branch information
chargome authored Jul 24, 2024
1 parent b2dded8 commit 4bdd979
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 8 deletions.
12 changes: 9 additions & 3 deletions packages/replay-internal/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -235,7 +235,6 @@ export class Replay implements Integration {
if (!this._replay) {
return;
}

this._replay.start();
}

Expand Down Expand Up @@ -265,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<void> {
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();
}

Expand Down
11 changes: 7 additions & 4 deletions packages/replay-internal/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 13 additions & 1 deletion packages/replay-internal/test/integration/flush.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,6 +34,7 @@ describe('Integration | flush', () => {

const { record: mockRecord } = mockRrweb();

let integration: Replay;
let replay: ReplayContainer;
let mockSendReplay: MockSendReplay;
let mockFlush: MockFlush;
Expand All @@ -45,7 +47,7 @@ describe('Integration | flush', () => {
domHandler = handler;
});

({ replay } = await mockSdk());
({ replay, integration } = await mockSdk());

mockSendReplay = vi.spyOn(SendReplay, 'sendReplay');
mockSendReplay.mockImplementation(
Expand Down Expand Up @@ -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);
});
});
20 changes: 20 additions & 0 deletions packages/replay-internal/test/integration/start.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,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);
});
});

0 comments on commit 4bdd979

Please sign in to comment.