From ba73d9205992c5d221cf7baaef75bd38abfde12e Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 9 Jan 2023 14:49:37 -0500 Subject: [PATCH 1/3] feat(replay): Add non-async flush for page unloads Add a method of using a non-async flush so that we are able to send a segment when the user unloads the page. --- packages/replay/src/eventBuffer.ts | 33 ++++++-- packages/replay/src/integration.ts | 2 +- packages/replay/src/replay.ts | 77 +++++++++++++------ packages/replay/src/types.ts | 18 +++++ .../replay/test/integration/flush.test.ts | 47 +++++------ packages/replay/test/integration/stop.test.ts | 7 +- 6 files changed, 131 insertions(+), 53 deletions(-) diff --git a/packages/replay/src/eventBuffer.ts b/packages/replay/src/eventBuffer.ts index d80cf22879ad..082a63477a71 100644 --- a/packages/replay/src/eventBuffer.ts +++ b/packages/replay/src/eventBuffer.ts @@ -73,14 +73,23 @@ class EventBufferArray implements EventBuffer { public finish(): Promise { return new Promise(resolve => { - // Make a copy of the events array reference and immediately clear the - // events member so that we do not lose new events while uploading - // attachment. - const eventsRet = this._events; - this._events = []; - resolve(JSON.stringify(eventsRet)); + resolve( + this._finish()); }); } + + public finishImmediate(): string { + return this._finish(); + } + + private _finish(): string { + // Make a copy of the events array reference and immediately clear the + // events member so that we do not lose new events while uploading + // attachment. + const events = this._events; + this._events = []; + return JSON.stringify(events); + } } /** @@ -158,6 +167,18 @@ export class EventBufferCompressionWorker implements EventBuffer { return this._finishRequest(this._getAndIncrementId()); } + /** + * Finish the event buffer and return the pending events. + */ + public finishImmediate(): string { + const events = this._pendingEvents; + + // Ensure worker is still in a good state and disregard the result + void this._finishRequest(this._getAndIncrementId()); + + return JSON.stringify(events); + } + /** * Post message to worker and wait for response before resolving promise. */ diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 274ec6147e34..2be5cfdd40a5 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -170,7 +170,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, return; } - this._replay.start(); + void this._replay.start(); } /** diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index ffba33c5631f..03906aae0d88 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { addGlobalEventProcessor, captureException, getCurrentHub } from '@sentry/core'; -import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types'; +import type { Breadcrumb, ReplayRecordingMode, ReplayRecordingData } from '@sentry/types'; import type { RateLimits } from '@sentry/utils'; import { addInstrumentationHandler, disabledUntil, logger } from '@sentry/utils'; import { EventType, record } from 'rrweb'; @@ -28,6 +28,7 @@ import type { ReplayContainer as ReplayContainerInterface, ReplayPluginOptions, Session, + FlushOptions, } from './types'; import { addEvent } from './util/addEvent'; import { addMemoryEntry } from './util/addMemoryEntry'; @@ -151,7 +152,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Creates or loads a session, attaches listeners to varying events (DOM, * _performanceObserver, Recording, Sentry SDK, etc) */ - public start(): void { + public async start(): Promise { this._setInitialState(); this._loadSession({ expiry: SESSION_IDLE_DURATION }); @@ -324,7 +325,6 @@ export class ReplayContainer implements ReplayContainerInterface { } /** - * * Always flush via `_debouncedFlush` so that we do not have flushes triggered * from calling both `flush` and `_debouncedFlush`. Otherwise, there could be * cases of mulitple flushes happening closely together. @@ -335,7 +335,7 @@ export class ReplayContainer implements ReplayContainerInterface { return this._debouncedFlush.flush() as Promise; } - /** Get the current sesion (=replay) ID */ + /** Get the current session (=replay) ID */ public getSessionId(): string | undefined { return this.session && this.session.id; } @@ -625,7 +625,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Send replay when the page/tab becomes hidden. There is no reason to send // replay if it becomes visible, since no actions we care about were done // while it was hidden - this._conditionalFlush(); + this._conditionalFlush({finishImmediate: true}); } /** @@ -747,11 +747,20 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Only flush if `this.recordingMode === 'session'` */ - private _conditionalFlush(): void { + private _conditionalFlush(options: FlushOptions = {}): void { if (this.recordingMode === 'error') { return; } + /** + * Page is likely to unload so need to bypass debounce completely and + * synchronously retrieve pending events from buffer and send request asap. + */ + if (options.finishImmediate) { + void this._runFlush(options); + return; + } + void this.flushImmediate(); } @@ -795,30 +804,26 @@ export class ReplayContainer implements ReplayContainerInterface { * * Should never be called directly, only by `flush` */ - private async _runFlush(): Promise { + private async _runFlush(options: FlushOptions = {}): Promise { if (!this.session || !this.eventBuffer) { __DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.'); return; } - await this._addPerformanceEntries(); + try { + this._debouncedFlush.cancel(); - // Check eventBuffer again, as it could have been stopped in the meanwhile - if (!this.eventBuffer || !this.eventBuffer.pendingLength) { - return; - } + const promises: Promise[] = []; - // Only attach memory event if eventBuffer is not empty - await addMemoryEntry(this); + promises.push(this._addPerformanceEntries()); - // Check eventBuffer again, as it could have been stopped in the meanwhile - if (!this.eventBuffer) { - return; - } + // Do not continue if there are no pending events in buffer + if (!this.eventBuffer?.pendingLength) { + return; + } - try { - // Note this empties the event buffer regardless of outcome of sending replay - const recordingData = await this.eventBuffer.finish(); + // Only attach memory entry if eventBuffer is not empty + promises.push(addMemoryEntry(this)); // NOTE: Copy values from instance members, as it's possible they could // change before the flush finishes. @@ -826,9 +831,33 @@ export class ReplayContainer implements ReplayContainerInterface { const eventContext = this._popEventContext(); // Always increment segmentId regardless of outcome of sending replay const segmentId = this.session.segmentId++; + + // Save session (new segment id) after we save flush data assuming either + // 1) request succeeds or 2) it fails or never happens, in which case we + // need to retry this segment. this._maybeSaveSession(); - await sendReplay({ + let recordingData: ReplayRecordingData; + + if (options.finishImmediate && this.eventBuffer.pendingLength) { + recordingData = this.eventBuffer.finishImmediate(); + } else { + // NOTE: Be mindful that nothing after this point (the first `await`) + // will run after when the page is unloaded. + await Promise.all(promises); + + // This can be empty due to blur events calling `runFlush` directly. In + // the case where we have a snapshot checkout and a blur event + // happening near the same time, the blur event can end up emptying the + // buffer even if snapshot happens first. + if (!this.eventBuffer.pendingLength) { + return; + } + // This empties the event buffer regardless of outcome of sending replay + recordingData = await this.eventBuffer.finish(); + } + + const sendReplayPromise = sendReplay({ replayId, recordingData, segmentId, @@ -838,6 +867,10 @@ export class ReplayContainer implements ReplayContainerInterface { options: this.getOptions(), timestamp: new Date().getTime(), }); + + await sendReplayPromise; + + return; } catch (err) { this._handleException(err); diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 356a3c1179fc..1d5e91d4c03d 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -7,6 +7,15 @@ export type RecordingOptions = recordOptions; export type AllPerformanceEntry = PerformancePaintTiming | PerformanceResourceTiming | PerformanceNavigationTiming; +export interface FlushOptions { + /** + * Attempt to finish the flush immediately without any asynchronous operations + * (e.g. worker calls). This is not directly related to `flushImmediate` which + * skips the debounced flush. + */ + finishImmediate?: boolean; +} + export interface SendReplayData { recordingData: ReplayRecordingData; replayId: string; @@ -18,6 +27,10 @@ export interface SendReplayData { options: ReplayPluginOptions; } +export type PendingReplayData = Omit & { + recordingData: RecordingEvent[]; +}; + export type InstrumentationTypeBreadcrumb = 'dom' | 'scope'; /** @@ -237,6 +250,11 @@ export interface EventBuffer { * Clears and returns the contents of the buffer. */ finish(): Promise; + + /** +* Clears and synchronously returns the pending contents of the buffer. This means no compression. +*/ + finishImmediate(): string; } export type AddUpdateCallback = () => boolean | void; diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index c0c143b255e4..9d665e0ab95a 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -107,30 +107,24 @@ describe('Integration | flush', () => { replay && replay.stop(); }); - it('flushes twice after multiple flush() calls)', async () => { - // blur events cause an immediate flush (as well as a flush due to adding a - // breadcrumb) -- this means that the first blur event will be flushed and - // the following blur events will all call a debounced flush function, which - // should end up queueing a second flush - + it('flushes after each blur event', async () => { + // blur events cause an immediate flush that bypass the debounced flush + // function and skip any async workers + expect(mockRunFlush).toHaveBeenCalledTimes(0); WINDOW.dispatchEvent(new Event('blur')); + expect(mockRunFlush).toHaveBeenCalledTimes(1); WINDOW.dispatchEvent(new Event('blur')); + expect(mockRunFlush).toHaveBeenCalledTimes(2); WINDOW.dispatchEvent(new Event('blur')); + expect(mockRunFlush).toHaveBeenCalledTimes(3); WINDOW.dispatchEvent(new Event('blur')); + expect(mockRunFlush).toHaveBeenCalledTimes(4); - expect(mockFlush).toHaveBeenCalledTimes(4); + expect(mockFlush).toHaveBeenCalledTimes(0); jest.runAllTimers(); await new Promise(process.nextTick); - expect(mockRunFlush).toHaveBeenCalledTimes(1); - - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(mockRunFlush).toHaveBeenCalledTimes(2); - - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(mockRunFlush).toHaveBeenCalledTimes(2); + expect(mockRunFlush).toHaveBeenCalledTimes(4); }); it('long first flush enqueues following events', async () => { @@ -141,8 +135,11 @@ describe('Integration | flush', () => { expect(mockAddPerformanceEntries).not.toHaveBeenCalled(); - // flush #1 @ t=0s - due to blur - WINDOW.dispatchEvent(new Event('blur')); + // flush #1 @ t=0s - (blur bypasses debounce, so manually call `flushImmedate`) + domHandler({ + name: 'click', + }); + replay.flushImmediate(); expect(mockFlush).toHaveBeenCalledTimes(1); expect(mockRunFlush).toHaveBeenCalledTimes(1); @@ -155,8 +152,11 @@ describe('Integration | flush', () => { expect(mockFlush).toHaveBeenCalledTimes(2); await advanceTimers(1000); - // flush #3 @ t=6s - due to blur - WINDOW.dispatchEvent(new Event('blur')); + domHandler({ + name: 'click', + }); + // flush #3 @ t=6s + replay.flushImmediate(); expect(mockFlush).toHaveBeenCalledTimes(3); // NOTE: Blur also adds a breadcrumb which calls `addUpdate`, meaning it will @@ -164,8 +164,11 @@ describe('Integration | flush', () => { await advanceTimers(8000); expect(mockFlush).toHaveBeenCalledTimes(3); - // flush #4 @ t=14s - due to blur - WINDOW.dispatchEvent(new Event('blur')); + // flush #4 @ t=14s + domHandler({ + name: 'click', + }); + replay.flushImmediate(); expect(mockFlush).toHaveBeenCalledTimes(4); expect(mockRunFlush).toHaveBeenCalledTimes(1); diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index 55f8dafd9289..0752089db984 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -106,6 +106,9 @@ describe('Integration | stop', () => { }; addEvent(replay, TEST_EVENT); + // This is an interesting test case because `start()` causes a checkout and a `flushImmediate`, and + // blur causes a direct `runFlush` call to ensure if window unloads, it is able to send out an uncompressed segment. + // This means that the non-async blur call can empty out the buffer before `flushImmediate` finishes. WINDOW.dispatchEvent(new Event('blur')); jest.runAllTimers(); await new Promise(process.nextTick); @@ -127,7 +130,7 @@ describe('Integration | stop', () => { }); it('does not buffer events when stopped', async function () { - WINDOW.dispatchEvent(new Event('blur')); + WINDOW.dispatchEvent(new Event('focus')); expect(replay.eventBuffer?.pendingLength).toBe(1); // stop replays @@ -135,7 +138,7 @@ describe('Integration | stop', () => { expect(replay.eventBuffer?.pendingLength).toBe(undefined); - WINDOW.dispatchEvent(new Event('blur')); + WINDOW.dispatchEvent(new Event('focus')); await new Promise(process.nextTick); expect(replay.eventBuffer?.pendingLength).toBe(undefined); From 1601a2442a53e14355129d17c60c292a56af41e6 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 18 Jan 2023 12:57:57 -0500 Subject: [PATCH 2/3] undo some unnecessary changes and lint --- packages/replay/src/eventBuffer.ts | 3 +-- packages/replay/src/integration.ts | 2 +- packages/replay/src/replay.ts | 16 ++++++---------- packages/replay/src/types.ts | 6 +++--- 4 files changed, 11 insertions(+), 16 deletions(-) diff --git a/packages/replay/src/eventBuffer.ts b/packages/replay/src/eventBuffer.ts index 082a63477a71..99e93f90f436 100644 --- a/packages/replay/src/eventBuffer.ts +++ b/packages/replay/src/eventBuffer.ts @@ -73,8 +73,7 @@ class EventBufferArray implements EventBuffer { public finish(): Promise { return new Promise(resolve => { - resolve( - this._finish()); + resolve(this._finish()); }); } diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 2be5cfdd40a5..274ec6147e34 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -170,7 +170,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, return; } - void this._replay.start(); + this._replay.start(); } /** diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 03906aae0d88..b88af6ef860b 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { addGlobalEventProcessor, captureException, getCurrentHub } from '@sentry/core'; -import type { Breadcrumb, ReplayRecordingMode, ReplayRecordingData } from '@sentry/types'; +import type { Breadcrumb, ReplayRecordingData, ReplayRecordingMode } from '@sentry/types'; import type { RateLimits } from '@sentry/utils'; import { addInstrumentationHandler, disabledUntil, logger } from '@sentry/utils'; import { EventType, record } from 'rrweb'; @@ -20,6 +20,7 @@ import type { AddUpdateCallback, AllPerformanceEntry, EventBuffer, + FlushOptions, InstrumentationTypeBreadcrumb, InternalEventContext, PopEventContext, @@ -28,7 +29,6 @@ import type { ReplayContainer as ReplayContainerInterface, ReplayPluginOptions, Session, - FlushOptions, } from './types'; import { addEvent } from './util/addEvent'; import { addMemoryEntry } from './util/addMemoryEntry'; @@ -152,7 +152,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Creates or loads a session, attaches listeners to varying events (DOM, * _performanceObserver, Recording, Sentry SDK, etc) */ - public async start(): Promise { + public start(): void { this._setInitialState(); this._loadSession({ expiry: SESSION_IDLE_DURATION }); @@ -625,7 +625,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Send replay when the page/tab becomes hidden. There is no reason to send // replay if it becomes visible, since no actions we care about were done // while it was hidden - this._conditionalFlush({finishImmediate: true}); + this._conditionalFlush({ finishImmediate: true }); } /** @@ -818,7 +818,7 @@ export class ReplayContainer implements ReplayContainerInterface { promises.push(this._addPerformanceEntries()); // Do not continue if there are no pending events in buffer - if (!this.eventBuffer?.pendingLength) { + if (!this.eventBuffer || !this.eventBuffer.pendingLength) { return; } @@ -857,7 +857,7 @@ export class ReplayContainer implements ReplayContainerInterface { recordingData = await this.eventBuffer.finish(); } - const sendReplayPromise = sendReplay({ + await sendReplay({ replayId, recordingData, segmentId, @@ -867,10 +867,6 @@ export class ReplayContainer implements ReplayContainerInterface { options: this.getOptions(), timestamp: new Date().getTime(), }); - - await sendReplayPromise; - - return; } catch (err) { this._handleException(err); diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 1d5e91d4c03d..b4f427dc958b 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -27,7 +27,7 @@ export interface SendReplayData { options: ReplayPluginOptions; } -export type PendingReplayData = Omit & { +export type PendingReplayData = Omit & { recordingData: RecordingEvent[]; }; @@ -252,8 +252,8 @@ export interface EventBuffer { finish(): Promise; /** -* Clears and synchronously returns the pending contents of the buffer. This means no compression. -*/ + * Clears and synchronously returns the pending contents of the buffer. This means no compression. + */ finishImmediate(): string; } From 602299714808cf1815f9980e84e6e3837b46d2a8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Jan 2023 16:28:20 +0100 Subject: [PATCH 3/3] ref(replay): Try to make flush fully sync (#6859) --- packages/replay/src/eventBuffer.ts | 14 +- packages/replay/src/replay.ts | 170 ++++++++++++------ packages/replay/src/types.ts | 4 +- .../replay/test/integration/flush.test.ts | 17 +- 4 files changed, 133 insertions(+), 72 deletions(-) diff --git a/packages/replay/src/eventBuffer.ts b/packages/replay/src/eventBuffer.ts index 99e93f90f436..d36094958c31 100644 --- a/packages/replay/src/eventBuffer.ts +++ b/packages/replay/src/eventBuffer.ts @@ -71,17 +71,11 @@ class EventBufferArray implements EventBuffer { return; } - public finish(): Promise { - return new Promise(resolve => { - resolve(this._finish()); - }); - } - - public finishImmediate(): string { - return this._finish(); + public async finish(): Promise { + return this.finishSync(); } - private _finish(): string { + public finishSync(): string { // Make a copy of the events array reference and immediately clear the // events member so that we do not lose new events while uploading // attachment. @@ -169,7 +163,7 @@ export class EventBufferCompressionWorker implements EventBuffer { /** * Finish the event buffer and return the pending events. */ - public finishImmediate(): string { + public finishSync(): string { const events = this._pendingEvents; // Ensure worker is still in a good state and disregard the result diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index b88af6ef860b..daee6d6c1615 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { addGlobalEventProcessor, captureException, getCurrentHub } from '@sentry/core'; -import type { Breadcrumb, ReplayRecordingData, ReplayRecordingMode } from '@sentry/types'; +import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types'; import type { RateLimits } from '@sentry/utils'; import { addInstrumentationHandler, disabledUntil, logger } from '@sentry/utils'; import { EventType, record } from 'rrweb'; @@ -625,7 +625,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Send replay when the page/tab becomes hidden. There is no reason to send // replay if it becomes visible, since no actions we care about were done // while it was hidden - this._conditionalFlush({ finishImmediate: true }); + this._conditionalFlush({ sync: true }); } /** @@ -756,8 +756,8 @@ export class ReplayContainer implements ReplayContainerInterface { * Page is likely to unload so need to bypass debounce completely and * synchronously retrieve pending events from buffer and send request asap. */ - if (options.finishImmediate) { - void this._runFlush(options); + if (options.sync) { + this._flushSync(); return; } @@ -804,81 +804,143 @@ export class ReplayContainer implements ReplayContainerInterface { * * Should never be called directly, only by `flush` */ - private async _runFlush(options: FlushOptions = {}): Promise { - if (!this.session || !this.eventBuffer) { - __DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.'); - return; - } - + private async _runFlush(): Promise { try { - this._debouncedFlush.cancel(); + const flushData = this._prepareFlush(); + + if (!flushData) { + return; + } - const promises: Promise[] = []; + const { promises, replayId, segmentId, eventContext, eventBuffer, session } = flushData; - promises.push(this._addPerformanceEntries()); + // NOTE: Be mindful that nothing after this point (the first `await`) + // will run after when the page is unloaded. + await Promise.all(promises); - // Do not continue if there are no pending events in buffer - if (!this.eventBuffer || !this.eventBuffer.pendingLength) { + // This can be empty due to blur events calling `runFlush` directly. In + // the case where we have a snapshot checkout and a blur event + // happening near the same time, the blur event can end up emptying the + // buffer even if snapshot happens first. + if (!eventBuffer.pendingLength) { return; } - // Only attach memory entry if eventBuffer is not empty - promises.push(addMemoryEntry(this)); + // This empties the event buffer regardless of outcome of sending replay + const recordingData = await eventBuffer.finish(); - // NOTE: Copy values from instance members, as it's possible they could - // change before the flush finishes. - const replayId = this.session.id; - const eventContext = this._popEventContext(); - // Always increment segmentId regardless of outcome of sending replay - const segmentId = this.session.segmentId++; - - // Save session (new segment id) after we save flush data assuming either - // 1) request succeeds or 2) it fails or never happens, in which case we - // need to retry this segment. - this._maybeSaveSession(); + await sendReplay({ + replayId, + recordingData, + segmentId, + includeReplayStartTimestamp: segmentId === 0, + eventContext, + session, + options: this.getOptions(), + timestamp: new Date().getTime(), + }); + } catch (err) { + this._handleSendError(err); + } + } - let recordingData: ReplayRecordingData; + /** + * Flush event buffer synchonously. + * This is necessary e.g. when running flush on page unload or similar. + */ + private _flushSync(): void { + try { + const flushData = this._prepareFlush(); - if (options.finishImmediate && this.eventBuffer.pendingLength) { - recordingData = this.eventBuffer.finishImmediate(); - } else { - // NOTE: Be mindful that nothing after this point (the first `await`) - // will run after when the page is unloaded. - await Promise.all(promises); - - // This can be empty due to blur events calling `runFlush` directly. In - // the case where we have a snapshot checkout and a blur event - // happening near the same time, the blur event can end up emptying the - // buffer even if snapshot happens first. - if (!this.eventBuffer.pendingLength) { - return; - } - // This empties the event buffer regardless of outcome of sending replay - recordingData = await this.eventBuffer.finish(); + if (!flushData) { + return; } - await sendReplay({ + const { replayId, segmentId, eventContext, eventBuffer, session } = flushData; + + const recordingData = eventBuffer.finishSync(); + + sendReplay({ replayId, recordingData, segmentId, includeReplayStartTimestamp: segmentId === 0, eventContext, - session: this.session, + session, options: this.getOptions(), timestamp: new Date().getTime(), + }).catch(err => { + this._handleSendError(err); }); } catch (err) { - this._handleException(err); + this._handleSendError(err); + } + } - if (err instanceof RateLimitError) { - this._handleRateLimit(err.rateLimits); - return; + /** Prepare flush data */ + private _prepareFlush(): + | { + replayId: string; + eventContext: PopEventContext; + segmentId: number; + promises: Promise[]; + eventBuffer: EventBuffer; + session: Session; } + | undefined { + if (!this.session || !this.eventBuffer) { + __DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.'); + return; + } - // This means we retried 3 times, and all of them failed - // In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments - this.stop(); + this._debouncedFlush.cancel(); + + const promises: Promise[] = []; + + promises.push(this._addPerformanceEntries()); + + // Do not continue if there are no pending events in buffer + if (!this.eventBuffer || !this.eventBuffer.pendingLength) { + return; } + + // Only attach memory entry if eventBuffer is not empty + promises.push(addMemoryEntry(this)); + + // NOTE: Copy values from instance members, as it's possible they could + // change before the flush finishes. + const replayId = this.session.id; + const eventContext = this._popEventContext(); + // Always increment segmentId regardless of outcome of sending replay + const segmentId = this.session.segmentId++; + + // Save session (new segment id) after we save flush data assuming either + // 1) request succeeds or 2) it fails or never happens, in which case we + // need to retry this segment. + this._maybeSaveSession(); + + return { + replayId, + eventContext, + segmentId, + promises, + eventBuffer: this.eventBuffer, + session: this.session, + }; + } + + /** Handle an error when sending a replay. */ + private _handleSendError(error: unknown): void { + this._handleException(error); + + if (error instanceof RateLimitError) { + this._handleRateLimit(error.rateLimits); + return; + } + + // This means we retried 3 times, and all of them failed + // In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments + this.stop(); } /** diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index b4f427dc958b..dd23c4e91cd0 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -13,7 +13,7 @@ export interface FlushOptions { * (e.g. worker calls). This is not directly related to `flushImmediate` which * skips the debounced flush. */ - finishImmediate?: boolean; + sync?: boolean; } export interface SendReplayData { @@ -254,7 +254,7 @@ export interface EventBuffer { /** * Clears and synchronously returns the pending contents of the buffer. This means no compression. */ - finishImmediate(): string; + finishSync(): string; } export type AddUpdateCallback = () => boolean | void; diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index 9d665e0ab95a..7ba49eccb19c 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -108,23 +108,28 @@ describe('Integration | flush', () => { }); it('flushes after each blur event', async () => { + // @ts-ignore privaye API + const mockFlushSync = jest.spyOn(replay, '_flushSync'); + // blur events cause an immediate flush that bypass the debounced flush // function and skip any async workers - expect(mockRunFlush).toHaveBeenCalledTimes(0); + expect(mockFlushSync).toHaveBeenCalledTimes(0); WINDOW.dispatchEvent(new Event('blur')); - expect(mockRunFlush).toHaveBeenCalledTimes(1); + expect(mockFlushSync).toHaveBeenCalledTimes(1); WINDOW.dispatchEvent(new Event('blur')); - expect(mockRunFlush).toHaveBeenCalledTimes(2); + expect(mockFlushSync).toHaveBeenCalledTimes(2); WINDOW.dispatchEvent(new Event('blur')); - expect(mockRunFlush).toHaveBeenCalledTimes(3); + expect(mockFlushSync).toHaveBeenCalledTimes(3); WINDOW.dispatchEvent(new Event('blur')); - expect(mockRunFlush).toHaveBeenCalledTimes(4); + expect(mockFlushSync).toHaveBeenCalledTimes(4); + expect(mockRunFlush).toHaveBeenCalledTimes(0); expect(mockFlush).toHaveBeenCalledTimes(0); jest.runAllTimers(); await new Promise(process.nextTick); - expect(mockRunFlush).toHaveBeenCalledTimes(4); + expect(mockFlushSync).toHaveBeenCalledTimes(4); + expect(mockRunFlush).toHaveBeenCalledTimes(0); }); it('long first flush enqueues following events', async () => {