Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(replay): Improve public Replay APIs #13000

Merged
merged 6 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
});
});
Loading