From bf5d4f1bdb3b470a78ef65033d3c1de2269589e1 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 4 Apr 2023 16:07:24 -0400 Subject: [PATCH 1/8] ref(replay): Add `capture()` API to record current event buffer This adds a public API: `capture` that will record the current event buffer and by default, convert the replay type to "session" and continue recording. We have extracted the logic that was used for "onError" capturing and made it a public API. --- .../src/coreHandlers/handleAfterSendEvent.ts | 16 ++------- packages/replay/src/integration.ts | 13 ++++++++ packages/replay/src/replay.ts | 33 +++++++++++++++++-- packages/replay/src/types.ts | 3 +- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts index f3a531f3ffdc..5a31b17737e4 100644 --- a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -53,19 +53,9 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal event.exception && event.message !== UNABLE_TO_SEND_REPLAY // ignore this error because otherwise we could loop indefinitely with trying to capture replay and failing ) { - setTimeout(async () => { - // Allow flush to complete before resuming as a session recording, otherwise - // the checkout from `startRecording` may be included in the payload. - // Prefer to keep the error replay as a separate (and smaller) segment - // than the session replay. - await replay.flushImmediate(); - - if (replay.stopRecording()) { - // Reset all "capture on error" configuration before - // starting a new recording - replay.recordingMode = 'session'; - replay.startRecording(); - } + setTimeout(() => { + // Capture current event buffer as new replay + void replay.capture(); }); } }; diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index c08eb0431cac..6392a1f2a88f 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -226,6 +226,19 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, return this._replay.flushImmediate(); } + /** + * If not in "session" recording mode, flush event buffer (i.e. creates a new replay). + * Unless `continueRecording` is false, the replay will continue to record and + * behave as a "session"-based replay. + */ + public async capture(continueRecording: boolean = true): Promise { + if (!this._replay) { + return; + } + + return this._replay.capture(continueRecording); + } + /** Setup the integration. */ private _setup(): void { // Client is not available in constructor, so we need to wait until setupOnce diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 8bb426874442..0f80fd99867f 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -223,10 +223,9 @@ export class ReplayContainer implements ReplayContainerInterface { if (this._stopRecording) { this._stopRecording(); this._stopRecording = undefined; - return true; } - return false; + return true; } catch (err) { this._handleException(err); return false; @@ -289,6 +288,36 @@ export class ReplayContainer implements ReplayContainerInterface { this.startRecording(); } + /** + * If not in "session" recording mode, flush event buffer (i.e. creates a new replay). + * Unless `continueRecording` is false, the replay will continue to record and + * behave as a "session"-based replay. + */ + public async capture(continueRecording: boolean = true): Promise { + // Don't allow if in session mode, use `flush()` instead + if (this.recordingMode === 'session') { + return; + } + + // Allow flush to complete before resuming as a session recording, otherwise + // the checkout from `startRecording` may be included in the payload. + // Prefer to keep the error replay as a separate (and smaller) segment + // than the session replay. + await this.flushImmediate(); + + if (!continueRecording) { + return; + } + + // Stop and re-start recording, but in "session" recording mode + if (this.stopRecording()) { + // Reset all "capture on error" configuration before + // starting a new recording + this.recordingMode = 'session'; + this.startRecording(); + } + } + /** * We want to batch uploads of replay events. Save events only if * `` milliseconds have elapsed since the last event diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index f6d4566d2d7c..414aa357677e 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -442,7 +442,8 @@ export interface ReplayContainer { resume(): void; startRecording(): void; stopRecording(): boolean; - flushImmediate(): void; + capture(continueOnError?: boolean): Promise; + flushImmediate(): Promise; triggerUserActivity(): void; addUpdate(cb: AddUpdateCallback): void; getOptions(): ReplayPluginOptions; From d64d8b2d78f98144cd60f483b54544c06991aac4 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 5 Apr 2023 17:18:52 -0400 Subject: [PATCH 2/8] rename and change options --- .../src/coreHandlers/handleAfterSendEvent.ts | 2 +- packages/replay/src/integration.ts | 23 ++++++------------- packages/replay/src/replay.ts | 9 +++++--- packages/replay/src/types.ts | 6 ++++- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts index 5a31b17737e4..dc94022e5b4a 100644 --- a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -55,7 +55,7 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal ) { setTimeout(() => { // Capture current event buffer as new replay - void replay.capture(); + void replay.sendBufferedReplayOrFlush(); }); } }; diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 6392a1f2a88f..a7a8f532c7a1 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -4,7 +4,7 @@ import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_FLUSH_MAX_DELAY, DEFAULT_FLUSH_MIN_DELAY } from './constants'; import { ReplayContainer } from './replay'; -import type { RecordingOptions, ReplayConfiguration, ReplayPluginOptions } from './types'; +import type { RecordingOptions, ReplayConfiguration, ReplayPluginOptions, SendBufferedReplayOptions } from './types'; import { getPrivacyOptions } from './util/getPrivacyOptions'; import { isBrowser } from './util/isBrowser'; @@ -216,27 +216,18 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, } /** - * Immediately send all pending events. - */ - public flush(): Promise | void { - if (!this._replay || !this._replay.isEnabled()) { - return; - } - - return this._replay.flushImmediate(); - } - - /** - * If not in "session" recording mode, flush event buffer (i.e. creates a new replay). + * If not in "session" recording mode, flush event buffer which will create a new replay. * Unless `continueRecording` is false, the replay will continue to record and * behave as a "session"-based replay. + * + * Otherwise, queue up a flush. */ - public async capture(continueRecording: boolean = true): Promise { - if (!this._replay) { + public flush(options?: SendBufferedReplayOptions): Promise | void { + if (!this._replay || !this._replay.isEnabled()) { return; } - return this._replay.capture(continueRecording); + return this._replay.sendBufferedReplayOrFlush(options); } /** Setup the integration. */ diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 0f80fd99867f..eabd7b959ad1 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -19,6 +19,7 @@ import type { RecordingOptions, ReplayContainer as ReplayContainerInterface, ReplayPluginOptions, + SendBufferedReplayOptions, Session, Timeouts, } from './types'; @@ -216,7 +217,9 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Stops the recording, if it was running. - * Returns true if it was stopped, else false. + * + * Returns true if it was previously stopped, or is now stopped, + * * else false. */ public stopRecording(): boolean { try { @@ -293,10 +296,10 @@ export class ReplayContainer implements ReplayContainerInterface { * Unless `continueRecording` is false, the replay will continue to record and * behave as a "session"-based replay. */ - public async capture(continueRecording: boolean = true): Promise { + public async sendBufferedReplayOrFlush({ continueRecording = true }: SendBufferedReplayOptions = {}): Promise { // Don't allow if in session mode, use `flush()` instead if (this.recordingMode === 'session') { - return; + return this._debouncedFlush() as Promise; } // Allow flush to complete before resuming as a session recording, otherwise diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 414aa357677e..f1d728b7c8c9 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -424,6 +424,10 @@ export interface EventBuffer { export type AddUpdateCallback = () => boolean | void; +export interface SendBufferedReplayOptions { + continueRecording?: boolean; +} + export interface ReplayContainer { eventBuffer: EventBuffer | null; performanceEvents: AllPerformanceEntry[]; @@ -442,7 +446,7 @@ export interface ReplayContainer { resume(): void; startRecording(): void; stopRecording(): boolean; - capture(continueOnError?: boolean): Promise; + sendBufferedReplayOrFlush(options?: SendBufferedReplayOptions): Promise; flushImmediate(): Promise; triggerUserActivity(): void; addUpdate(cb: AddUpdateCallback): void; From 8f8784d93179b29b51e0ffa6192b514b22acb522 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 5 Apr 2023 17:25:59 -0400 Subject: [PATCH 3/8] update comments --- packages/replay/src/replay.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index eabd7b959ad1..f2e301705370 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -219,7 +219,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Stops the recording, if it was running. * * Returns true if it was previously stopped, or is now stopped, - * * else false. + * otherwise false. */ public stopRecording(): boolean { try { @@ -292,12 +292,14 @@ export class ReplayContainer implements ReplayContainerInterface { } /** - * If not in "session" recording mode, flush event buffer (i.e. creates a new replay). + * If not in "session" recording mode, flush event buffer which will create a new replay. * Unless `continueRecording` is false, the replay will continue to record and * behave as a "session"-based replay. + * + * Otherwise, queue up a flush. */ public async sendBufferedReplayOrFlush({ continueRecording = true }: SendBufferedReplayOptions = {}): Promise { - // Don't allow if in session mode, use `flush()` instead + // if in session mode, call debounced flush if (this.recordingMode === 'session') { return this._debouncedFlush() as Promise; } From adcf18d5bade5de60a185417424f4a60d4f89e82 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 13 Apr 2023 15:44:08 -0400 Subject: [PATCH 4/8] flush immediate in session mode as well --- packages/replay/src/replay.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index f2e301705370..e3ed3eacb328 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -299,9 +299,8 @@ export class ReplayContainer implements ReplayContainerInterface { * Otherwise, queue up a flush. */ public async sendBufferedReplayOrFlush({ continueRecording = true }: SendBufferedReplayOptions = {}): Promise { - // if in session mode, call debounced flush if (this.recordingMode === 'session') { - return this._debouncedFlush() as Promise; + return this.flushImmediate(); } // Allow flush to complete before resuming as a session recording, otherwise From 893d0e317a119bd232fa46e1707acfed972a7980 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 17 Apr 2023 14:38:27 -0400 Subject: [PATCH 5/8] update comment --- packages/replay/src/integration.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index a7a8f532c7a1..bb948ce3e217 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -216,11 +216,11 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, } /** - * If not in "session" recording mode, flush event buffer which will create a new replay. + * Immediately send all pending events. In buffer-mode, this should be used + * to capture the initial replay. + * * 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()) { From b4cd07782fb597197a0ab8ac0cc9a4e68110dfc9 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 17 Apr 2023 15:01:04 -0400 Subject: [PATCH 6/8] stopRecording + test --- packages/replay/src/replay.ts | 6 +- .../test/integration/errorSampleRate.test.ts | 94 +++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index e3ed3eacb328..720f3aae43d2 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -309,12 +309,14 @@ export class ReplayContainer implements ReplayContainerInterface { // than the session replay. await this.flushImmediate(); + const hasStoppedRecording = this.stopRecording(); + if (!continueRecording) { return; } - // Stop and re-start recording, but in "session" recording mode - if (this.stopRecording()) { + // Re-start recording, but in "session" recording mode + if (hasStoppedRecording) { // Reset all "capture on error" configuration before // starting a new recording this.recordingMode = 'session'; diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 85f154e523f4..8bdf830b8e1a 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -158,6 +158,100 @@ describe('Integration | errorSampleRate', () => { }); }); + it('manually flushes replay and does not continue to record', async () => { + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + // Does not capture on mouse click + domHandler({ + name: 'click', + }); + jest.runAllTimers(); + await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + + replay.sendBufferedReplayOrFlush({continueRecording: false}) + + await new Promise(process.nextTick); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + + expect(replay).toHaveSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'error', + contexts: { + replay: { + error_sample_rate: 1, + session_sample_rate: 0, + }, + }, + }), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + TEST_EVENT, + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + // Check that click will not get captured + domHandler({ + name: 'click', + }); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + + // This is still the last replay sent since we passed `continueRecording: + // false`. + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'error', + contexts: { + replay: { + error_sample_rate: 1, + session_sample_rate: 0, + }, + }, + }), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + TEST_EVENT, + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + }); + it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_DURATION]ms', async () => { Object.defineProperty(document, 'visibilityState', { configurable: true, From bd981f926bbbd19b9649a64b6b7b50871ce70f5c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 17 Apr 2023 17:11:30 -0400 Subject: [PATCH 7/8] lint --- packages/replay/test/integration/errorSampleRate.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 8bdf830b8e1a..0b5baa8e2512 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -173,7 +173,7 @@ describe('Integration | errorSampleRate', () => { await new Promise(process.nextTick); expect(replay).not.toHaveLastSentReplay(); - replay.sendBufferedReplayOrFlush({continueRecording: false}) + replay.sendBufferedReplayOrFlush({ continueRecording: false }); await new Promise(process.nextTick); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); From 8c1d9c7e432d0d7f7f6afa60e5a6491c251e5f2c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 24 Apr 2023 13:08:23 +0200 Subject: [PATCH 8/8] review change --- packages/replay/src/replay.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 720f3aae43d2..0fc93e4078ee 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -311,17 +311,16 @@ export class ReplayContainer implements ReplayContainerInterface { const hasStoppedRecording = this.stopRecording(); - if (!continueRecording) { + if (!continueRecording || !hasStoppedRecording) { return; } // Re-start recording, but in "session" recording mode - if (hasStoppedRecording) { - // Reset all "capture on error" configuration before - // starting a new recording - this.recordingMode = 'session'; - this.startRecording(); - } + + // Reset all "capture on error" configuration before + // starting a new recording + this.recordingMode = 'session'; + this.startRecording(); } /**