From d253c811ddc70ad8effd9481f1c8db5cee588c3f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 25 Oct 2024 17:35:53 -0400 Subject: [PATCH 01/15] feat(replay): Clear event buffer when full and in buffer mode wip --- .../src/eventBuffer/EventBufferProxy.ts | 3 +++ packages/replay-internal/src/util/addEvent.ts | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index ea81365b66a9..e0c22715c863 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -12,6 +12,7 @@ import { EventBufferCompressionWorker } from './EventBufferCompressionWorker'; * Exported only for testing. */ export class EventBufferProxy implements EventBuffer { + public waitForCheckout: boolean; private _fallback: EventBufferArray; private _compression: EventBufferCompressionWorker; private _used: EventBuffer; @@ -23,6 +24,7 @@ export class EventBufferProxy implements EventBuffer { this._used = this._fallback; this._ensureWorkerIsLoadedPromise = this._ensureWorkerIsLoaded(); + this.waitForCheckout = false; } /** @inheritdoc */ @@ -52,6 +54,7 @@ export class EventBufferProxy implements EventBuffer { /** @inheritdoc */ public clear(): void { + this.waitForCheckout = true; return this._used.clear(); } diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 3a245242e608..77b47d32ecd3 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,12 +54,14 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - if (!replay.eventBuffer) { + if (!replay.eventBuffer || replay.eventBuffer.waitForCheckout) { return null; } + const isBufferMode = replay.recordingMode === 'buffer'; + try { - if (isCheckout && replay.recordingMode === 'buffer') { + if (isCheckout && isBufferMode) { replay.eventBuffer.clear(); } @@ -77,7 +79,17 @@ async function _addEvent( return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { - const reason = error && error instanceof EventBufferSizeExceededError ? 'addEventSizeExceeded' : 'addEvent'; + const isExceeded = error && error instanceof EventBufferSizeExceededError; + const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; + + if (isExceeded && isBufferMode) { + // Clear buffer and wait for next checkout + replay.eventBuffer.clear(); + replay.eventBuffer.waitForCheckout = true; + + return null; + } + replay.handleException(error); await replay.stop({ reason }); From 58592df703eeaa1c6278dc5ff2e458fe37c81bd6 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 28 Oct 2024 22:35:29 -0400 Subject: [PATCH 02/15] wip --- .../src/eventBuffer/EventBufferProxy.ts | 5 ++ packages/replay-internal/src/types/replay.ts | 3 +- packages/replay-internal/src/util/addEvent.ts | 15 +++-- .../test/integration/stop.test.ts | 64 +++++++++++++++---- 4 files changed, 66 insertions(+), 21 deletions(-) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index e0c22715c863..5ff853e08725 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -12,7 +12,12 @@ import { EventBufferCompressionWorker } from './EventBufferCompressionWorker'; * Exported only for testing. */ export class EventBufferProxy implements EventBuffer { + /** + * If the event buffer needs to wait for a checkout event before it + * starts buffering events. + */ public waitForCheckout: boolean; + private _fallback: EventBufferArray; private _compression: EventBufferCompressionWorker; private _used: EventBuffer; diff --git a/packages/replay-internal/src/types/replay.ts b/packages/replay-internal/src/types/replay.ts index 6892c05ee179..4556fdd094f3 100644 --- a/packages/replay-internal/src/types/replay.ts +++ b/packages/replay-internal/src/types/replay.ts @@ -9,6 +9,7 @@ import type { Span, XhrBreadcrumbHint, } from '@sentry/types'; +import type { EventBufferProxy } from '../eventBuffer/EventBufferProxy'; import type { SKIPPED, THROTTLED } from '../util/throttle'; import type { AllPerformanceEntry, AllPerformanceEntryData, ReplayPerformanceEntry } from './performance'; @@ -449,7 +450,7 @@ export interface ReplayClickDetector { } export interface ReplayContainer { - eventBuffer: EventBuffer | null; + eventBuffer: EventBufferProxy | null; clickDetector: ReplayClickDetector | undefined; /** * List of PerformanceEntry from PerformanceObservers. diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 77b47d32ecd3..5aa420bfff15 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,7 +54,9 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - if (!replay.eventBuffer || replay.eventBuffer.waitForCheckout) { + const { eventBuffer } = replay; + + if (!eventBuffer || (eventBuffer.waitForCheckout && !isCheckout)) { return null; } @@ -62,11 +64,12 @@ async function _addEvent( try { if (isCheckout && isBufferMode) { - replay.eventBuffer.clear(); + eventBuffer.clear(); } if (isCheckout) { - replay.eventBuffer.hasCheckout = true; + eventBuffer.hasCheckout = true; + eventBuffer.waitForCheckout = false; } const replayOptions = replay.getOptions(); @@ -77,15 +80,15 @@ async function _addEvent( return; } - return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); + return await eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { const isExceeded = error && error instanceof EventBufferSizeExceededError; const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; if (isExceeded && isBufferMode) { // Clear buffer and wait for next checkout - replay.eventBuffer.clear(); - replay.eventBuffer.waitForCheckout = true; + eventBuffer.clear(); + eventBuffer.waitForCheckout = true; return null; } diff --git a/packages/replay-internal/test/integration/stop.test.ts b/packages/replay-internal/test/integration/stop.test.ts index 6ebca89891b0..228bcc4200ce 100644 --- a/packages/replay-internal/test/integration/stop.test.ts +++ b/packages/replay-internal/test/integration/stop.test.ts @@ -15,7 +15,7 @@ import { addEvent } from '../../src/util/addEvent'; import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; // mock functions need to be imported first import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; -import { getTestEventIncremental } from '../utils/getTestEvent'; +import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent'; import { useFakeTimers } from '../utils/use-fake-timers'; useFakeTimers(); @@ -32,7 +32,7 @@ describe('Integration | stop', () => { let mockAddDomInstrumentationHandler: MockInstance; let mockRunFlush: MockRunFlush; - beforeAll(async () => { + beforeEach(async () => { vi.setSystemTime(new Date(BASE_TIMESTAMP)); mockAddDomInstrumentationHandler = vi.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler'); @@ -41,17 +41,13 @@ describe('Integration | stop', () => { // @ts-expect-error private API mockRunFlush = vi.spyOn(replay, '_runFlush'); - vi.runAllTimers(); - }); - - beforeEach(() => { - vi.setSystemTime(new Date(BASE_TIMESTAMP)); - replay.eventBuffer?.destroy(); + await vi.runAllTimersAsync(); vi.clearAllMocks(); }); afterEach(async () => { - vi.runAllTimers(); + console.log('afterEach'); + await vi.runAllTimersAsync(); await new Promise(process.nextTick); vi.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); @@ -64,6 +60,7 @@ describe('Integration | stop', () => { value: prevLocation, writable: true, }); + vi.clearAllMocks(); }); afterAll(() => { @@ -106,7 +103,7 @@ describe('Integration | stop', () => { vi.advanceTimersByTime(ELAPSED); - const timestamp = +new Date(BASE_TIMESTAMP + ELAPSED + ELAPSED) / 1000; + const timestamp = +new Date(BASE_TIMESTAMP + ELAPSED + ELAPSED + ELAPSED) / 1000; const hiddenBreadcrumb = { type: 5, @@ -123,7 +120,7 @@ describe('Integration | stop', () => { addEvent(replay, TEST_EVENT); WINDOW.dispatchEvent(new Event('blur')); - vi.runAllTimers(); + await vi.runAllTimersAsync(); await new Promise(process.nextTick); expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 0 }, @@ -131,7 +128,7 @@ describe('Integration | stop', () => { // This event happens when we call `replay.start` { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + ELAPSED, + timestamp: BASE_TIMESTAMP + ELAPSED + ELAPSED, type: 2, }, optionsEvent, @@ -142,12 +139,17 @@ describe('Integration | stop', () => { // Session's last activity is last updated when we call `setup()` and *NOT* // when tab is blurred - expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED); + expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED + ELAPSED); }); it('does not buffer new events after being stopped', async function () { + console.log('start test'); + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(mockRunFlush).toHaveBeenCalledTimes(0); const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); - addEvent(replay, TEST_EVENT); + console.log('hi', replay.eventBuffer); + addEvent(replay, TEST_EVENT, true); + console.log(replay.eventBuffer); expect(replay.eventBuffer?.hasEvents).toBe(true); expect(mockRunFlush).toHaveBeenCalledTimes(0); @@ -158,6 +160,7 @@ describe('Integration | stop', () => { expect(replay.eventBuffer).toBe(null); + console.log('before blur'); WINDOW.dispatchEvent(new Event('blur')); await new Promise(process.nextTick); @@ -176,4 +179,37 @@ describe('Integration | stop', () => { expect(mockAddDomInstrumentationHandler).not.toHaveBeenCalled(); }); + + it('does not add replay breadcrumb when stopped due to event buffer limit', async () => { + const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); + + vi.mock('../../src/constants', async requireActual => ({ + ...(await requireActual()), + REPLAY_MAX_EVENT_BUFFER_SIZE: 500, + })); + + await integration.stop(); + integration.startBuffering(); + + await addEvent(replay, TEST_EVENT); + + expect(replay.eventBuffer?.hasEvents).toBe(true); + expect(replay.eventBuffer?.['hasCheckout']).toBe(true); + + // This should should go over max buffer size + await addEvent(replay, TEST_EVENT); + // buffer should be cleared and wait for next checkout + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(replay.eventBuffer?.['hasCheckout']).toBe(false); + + await addEvent(replay, TEST_EVENT); + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(replay.eventBuffer?.['hasCheckout']).toBe(false); + + await addEvent(replay, getTestEventCheckout({ timestamp: Date.now() }), true); + expect(replay.eventBuffer?.hasEvents).toBe(true); + expect(replay.eventBuffer?.['hasCheckout']).toBe(true); + + vi.resetAllMocks(); + }); }); From dd47432a3d2bbdad6833b8e1cd1bd5726f309506 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 29 Oct 2024 12:18:12 -0400 Subject: [PATCH 03/15] cleanup stop.test.ts --- .../test/integration/stop.test.ts | 56 +------------------ 1 file changed, 3 insertions(+), 53 deletions(-) diff --git a/packages/replay-internal/test/integration/stop.test.ts b/packages/replay-internal/test/integration/stop.test.ts index 228bcc4200ce..5c3aa49b77d8 100644 --- a/packages/replay-internal/test/integration/stop.test.ts +++ b/packages/replay-internal/test/integration/stop.test.ts @@ -3,19 +3,18 @@ */ import type { MockInstance, MockedFunction } from 'vitest'; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import { WINDOW } from '../../src/constants'; import type { Replay } from '../../src/integration'; import type { ReplayContainer } from '../../src/replay'; -import { clearSession } from '../../src/session/clearSession'; import { addEvent } from '../../src/util/addEvent'; import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; // mock functions need to be imported first import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; -import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent'; +import { getTestEventIncremental } from '../utils/getTestEvent'; import { useFakeTimers } from '../utils/use-fake-timers'; useFakeTimers(); @@ -46,16 +45,8 @@ describe('Integration | stop', () => { }); afterEach(async () => { - console.log('afterEach'); - await vi.runAllTimersAsync(); - await new Promise(process.nextTick); vi.setSystemTime(new Date(BASE_TIMESTAMP)); - sessionStorage.clear(); - clearSession(replay); - replay['_initializeSessionForSampling'](); - replay.setInitialState(); - mockRecord.takeFullSnapshot.mockClear(); - mockAddDomInstrumentationHandler.mockClear(); + integration && (await integration.stop()); Object.defineProperty(WINDOW, 'location', { value: prevLocation, writable: true, @@ -63,10 +54,6 @@ describe('Integration | stop', () => { vi.clearAllMocks(); }); - afterAll(() => { - integration && integration.stop(); - }); - it('does not upload replay if it was stopped and can resume replays afterwards', async () => { Object.defineProperty(document, 'visibilityState', { configurable: true, @@ -143,13 +130,10 @@ describe('Integration | stop', () => { }); it('does not buffer new events after being stopped', async function () { - console.log('start test'); expect(replay.eventBuffer?.hasEvents).toBe(false); expect(mockRunFlush).toHaveBeenCalledTimes(0); const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); - console.log('hi', replay.eventBuffer); addEvent(replay, TEST_EVENT, true); - console.log(replay.eventBuffer); expect(replay.eventBuffer?.hasEvents).toBe(true); expect(mockRunFlush).toHaveBeenCalledTimes(0); @@ -160,7 +144,6 @@ describe('Integration | stop', () => { expect(replay.eventBuffer).toBe(null); - console.log('before blur'); WINDOW.dispatchEvent(new Event('blur')); await new Promise(process.nextTick); @@ -179,37 +162,4 @@ describe('Integration | stop', () => { expect(mockAddDomInstrumentationHandler).not.toHaveBeenCalled(); }); - - it('does not add replay breadcrumb when stopped due to event buffer limit', async () => { - const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); - - vi.mock('../../src/constants', async requireActual => ({ - ...(await requireActual()), - REPLAY_MAX_EVENT_BUFFER_SIZE: 500, - })); - - await integration.stop(); - integration.startBuffering(); - - await addEvent(replay, TEST_EVENT); - - expect(replay.eventBuffer?.hasEvents).toBe(true); - expect(replay.eventBuffer?.['hasCheckout']).toBe(true); - - // This should should go over max buffer size - await addEvent(replay, TEST_EVENT); - // buffer should be cleared and wait for next checkout - expect(replay.eventBuffer?.hasEvents).toBe(false); - expect(replay.eventBuffer?.['hasCheckout']).toBe(false); - - await addEvent(replay, TEST_EVENT); - expect(replay.eventBuffer?.hasEvents).toBe(false); - expect(replay.eventBuffer?.['hasCheckout']).toBe(false); - - await addEvent(replay, getTestEventCheckout({ timestamp: Date.now() }), true); - expect(replay.eventBuffer?.hasEvents).toBe(true); - expect(replay.eventBuffer?.['hasCheckout']).toBe(true); - - vi.resetAllMocks(); - }); }); From be7ff0317ef899a791bb95196be7a644fe1eaded Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 29 Oct 2024 12:20:11 -0400 Subject: [PATCH 04/15] add eventBuffer integration test --- .../src/eventBuffer/EventBufferProxy.ts | 1 - .../test/integration/eventBuffer.test.ts | 75 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 packages/replay-internal/test/integration/eventBuffer.test.ts diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index 5ff853e08725..c51372c72e1d 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -59,7 +59,6 @@ export class EventBufferProxy implements EventBuffer { /** @inheritdoc */ public clear(): void { - this.waitForCheckout = true; return this._used.clear(); } diff --git a/packages/replay-internal/test/integration/eventBuffer.test.ts b/packages/replay-internal/test/integration/eventBuffer.test.ts new file mode 100644 index 000000000000..bc796516acf8 --- /dev/null +++ b/packages/replay-internal/test/integration/eventBuffer.test.ts @@ -0,0 +1,75 @@ +/** + * @vitest-environment jsdom + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { WINDOW } from '../../src/constants'; +import type { Replay } from '../../src/integration'; +import type { ReplayContainer } from '../../src/replay'; +import { addEvent } from '../../src/util/addEvent'; + +// mock functions need to be imported first +import { BASE_TIMESTAMP, mockSdk } from '../index'; +import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent'; +import { useFakeTimers } from '../utils/use-fake-timers'; + +useFakeTimers(); + +describe('Integration | eventBuffer | Event Buffer Max Size', () => { + let replay: ReplayContainer; + let integration: Replay; + const prevLocation = WINDOW.location; + + beforeEach(async () => { + vi.setSystemTime(new Date(BASE_TIMESTAMP)); + + ({ replay, integration } = await mockSdk()); + + await vi.runAllTimersAsync(); + vi.clearAllMocks(); + }); + + afterEach(async () => { + vi.setSystemTime(new Date(BASE_TIMESTAMP)); + integration && (await integration.stop()); + Object.defineProperty(WINDOW, 'location', { + value: prevLocation, + writable: true, + }); + vi.clearAllMocks(); + }); + + it('does not add replay breadcrumb when stopped due to event buffer limit', async () => { + const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); + + vi.mock('../../src/constants', async requireActual => ({ + ...(await requireActual()), + REPLAY_MAX_EVENT_BUFFER_SIZE: 500, + })); + + await integration.stop(); + integration.startBuffering(); + + await addEvent(replay, TEST_EVENT); + + expect(replay.eventBuffer?.hasEvents).toBe(true); + expect(replay.eventBuffer?.['hasCheckout']).toBe(true); + + // This should should go over max buffer size + await addEvent(replay, TEST_EVENT); + // buffer should be cleared and wait for next checkout + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(replay.eventBuffer?.['hasCheckout']).toBe(false); + + await addEvent(replay, TEST_EVENT); + expect(replay.eventBuffer?.hasEvents).toBe(false); + expect(replay.eventBuffer?.['hasCheckout']).toBe(false); + + await addEvent(replay, getTestEventCheckout({ timestamp: Date.now() }), true); + expect(replay.eventBuffer?.hasEvents).toBe(true); + expect(replay.eventBuffer?.['hasCheckout']).toBe(true); + + vi.resetAllMocks(); + }); +}); From e07f4f3d9e036a1d98b78052fee196f1b930d946 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 29 Oct 2024 13:24:38 -0400 Subject: [PATCH 05/15] adjust types for eventbuffer -> proxy --- packages/replay-internal/src/eventBuffer/index.ts | 3 +-- packages/replay-internal/src/replay.ts | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/replay-internal/src/eventBuffer/index.ts b/packages/replay-internal/src/eventBuffer/index.ts index bc000da5db7e..5ee4d609ff20 100644 --- a/packages/replay-internal/src/eventBuffer/index.ts +++ b/packages/replay-internal/src/eventBuffer/index.ts @@ -1,7 +1,6 @@ import { getWorkerURL } from '@sentry-internal/replay-worker'; import { DEBUG_BUILD } from '../debug-build'; -import type { EventBuffer } from '../types'; import { logger } from '../util/logger'; import { EventBufferArray } from './EventBufferArray'; import { EventBufferProxy } from './EventBufferProxy'; @@ -20,7 +19,7 @@ declare const __SENTRY_EXCLUDE_REPLAY_WORKER__: boolean; export function createEventBuffer({ useCompression, workerUrl: customWorkerUrl, -}: CreateEventBufferParams): EventBuffer { +}: CreateEventBufferParams): EventBufferProxy { if ( useCompression && // eslint-disable-next-line no-restricted-globals diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index d87a73a7e51d..02c795871257 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -27,7 +27,6 @@ import type { AddUpdateCallback, AllPerformanceEntry, AllPerformanceEntryData, - EventBuffer, InternalEventContext, PopEventContext, RecordingEvent, @@ -58,12 +57,13 @@ import { sendReplay } from './util/sendReplay'; import { RateLimitError } from './util/sendReplayRequest'; import type { SKIPPED } from './util/throttle'; import { THROTTLED, throttle } from './util/throttle'; +import type { EventBufferProxy } from './eventBuffer/EventBufferProxy'; /** * The main replay container class, which holds all the state and methods for recording and sending replays. */ export class ReplayContainer implements ReplayContainerInterface { - public eventBuffer: EventBuffer | null; + public eventBuffer: EventBufferProxy | null; public performanceEntries: AllPerformanceEntry[]; From 5b0a37026891108aff307515de56514820f6525c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 29 Oct 2024 13:35:32 -0400 Subject: [PATCH 06/15] change EventBufferProxy types back --- .../src/eventBuffer/EventBufferArray.ts | 4 ++++ .../src/eventBuffer/EventBufferCompressionWorker.ts | 4 ++++ .../src/eventBuffer/EventBufferProxy.ts | 12 +++++------- packages/replay-internal/src/eventBuffer/index.ts | 3 ++- packages/replay-internal/src/replay.ts | 4 ++-- packages/replay-internal/src/types/replay.ts | 9 +++++++-- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferArray.ts b/packages/replay-internal/src/eventBuffer/EventBufferArray.ts index 2eb760409d9f..9b6f5f602e63 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferArray.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferArray.ts @@ -14,12 +14,16 @@ export class EventBufferArray implements EventBuffer { /** @inheritdoc */ public hasCheckout: boolean; + /** @inheritdoc */ + public waitForCheckout: boolean; + private _totalSize: number; public constructor() { this.events = []; this._totalSize = 0; this.hasCheckout = false; + this.waitForCheckout = false; } /** @inheritdoc */ diff --git a/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts index 8ca7f3caccca..13fab7e6717b 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts @@ -16,6 +16,9 @@ export class EventBufferCompressionWorker implements EventBuffer { /** @inheritdoc */ public hasCheckout: boolean; + /** @inheritdoc */ + public waitForCheckout: boolean; + private _worker: WorkerHandler; private _earliestTimestamp: number | null; private _totalSize; @@ -25,6 +28,7 @@ export class EventBufferCompressionWorker implements EventBuffer { this._earliestTimestamp = null; this._totalSize = 0; this.hasCheckout = false; + this.waitForCheckout = false; } /** @inheritdoc */ diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index c51372c72e1d..5ca40c2dd8c9 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -12,12 +12,6 @@ import { EventBufferCompressionWorker } from './EventBufferCompressionWorker'; * Exported only for testing. */ export class EventBufferProxy implements EventBuffer { - /** - * If the event buffer needs to wait for a checkout event before it - * starts buffering events. - */ - public waitForCheckout: boolean; - private _fallback: EventBufferArray; private _compression: EventBufferCompressionWorker; private _used: EventBuffer; @@ -29,7 +23,11 @@ export class EventBufferProxy implements EventBuffer { this._used = this._fallback; this._ensureWorkerIsLoadedPromise = this._ensureWorkerIsLoaded(); - this.waitForCheckout = false; + } + + /** @inheritdoc */ + public get waitForCheckout(): boolean { + return this._used.waitForCheckout; } /** @inheritdoc */ diff --git a/packages/replay-internal/src/eventBuffer/index.ts b/packages/replay-internal/src/eventBuffer/index.ts index 5ee4d609ff20..00d493dd95e1 100644 --- a/packages/replay-internal/src/eventBuffer/index.ts +++ b/packages/replay-internal/src/eventBuffer/index.ts @@ -1,5 +1,6 @@ import { getWorkerURL } from '@sentry-internal/replay-worker'; +import type { EventBuffer } from '../types'; import { DEBUG_BUILD } from '../debug-build'; import { logger } from '../util/logger'; import { EventBufferArray } from './EventBufferArray'; @@ -19,7 +20,7 @@ declare const __SENTRY_EXCLUDE_REPLAY_WORKER__: boolean; export function createEventBuffer({ useCompression, workerUrl: customWorkerUrl, -}: CreateEventBufferParams): EventBufferProxy { +}: CreateEventBufferParams): EventBuffer { if ( useCompression && // eslint-disable-next-line no-restricted-globals diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index 02c795871257..d87a73a7e51d 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -27,6 +27,7 @@ import type { AddUpdateCallback, AllPerformanceEntry, AllPerformanceEntryData, + EventBuffer, InternalEventContext, PopEventContext, RecordingEvent, @@ -57,13 +58,12 @@ import { sendReplay } from './util/sendReplay'; import { RateLimitError } from './util/sendReplayRequest'; import type { SKIPPED } from './util/throttle'; import { THROTTLED, throttle } from './util/throttle'; -import type { EventBufferProxy } from './eventBuffer/EventBufferProxy'; /** * The main replay container class, which holds all the state and methods for recording and sending replays. */ export class ReplayContainer implements ReplayContainerInterface { - public eventBuffer: EventBufferProxy | null; + public eventBuffer: EventBuffer | null; public performanceEntries: AllPerformanceEntry[]; diff --git a/packages/replay-internal/src/types/replay.ts b/packages/replay-internal/src/types/replay.ts index 4556fdd094f3..4f78a438ecc3 100644 --- a/packages/replay-internal/src/types/replay.ts +++ b/packages/replay-internal/src/types/replay.ts @@ -9,7 +9,6 @@ import type { Span, XhrBreadcrumbHint, } from '@sentry/types'; -import type { EventBufferProxy } from '../eventBuffer/EventBufferProxy'; import type { SKIPPED, THROTTLED } from '../util/throttle'; import type { AllPerformanceEntry, AllPerformanceEntryData, ReplayPerformanceEntry } from './performance'; @@ -401,6 +400,12 @@ export interface EventBuffer { */ hasCheckout: boolean; + /** + * If the event buffer needs to wait for a checkout event before it + * starts buffering events. + */ + waitForCheckout: boolean; + /** * Destroy the event buffer. */ @@ -450,7 +455,7 @@ export interface ReplayClickDetector { } export interface ReplayContainer { - eventBuffer: EventBufferProxy | null; + eventBuffer: EventBuffer | null; clickDetector: ReplayClickDetector | undefined; /** * List of PerformanceEntry from PerformanceObservers. From 9f381c6c03d407ad25cde27f2ed38a52b83048d3 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 29 Oct 2024 13:44:17 -0400 Subject: [PATCH 07/15] formatting --- packages/replay-internal/src/eventBuffer/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay-internal/src/eventBuffer/index.ts b/packages/replay-internal/src/eventBuffer/index.ts index 00d493dd95e1..bc000da5db7e 100644 --- a/packages/replay-internal/src/eventBuffer/index.ts +++ b/packages/replay-internal/src/eventBuffer/index.ts @@ -1,7 +1,7 @@ import { getWorkerURL } from '@sentry-internal/replay-worker'; -import type { EventBuffer } from '../types'; import { DEBUG_BUILD } from '../debug-build'; +import type { EventBuffer } from '../types'; import { logger } from '../util/logger'; import { EventBufferArray } from './EventBufferArray'; import { EventBufferProxy } from './EventBufferProxy'; From d7cd934c268bc3108ca74d91111f4039bee5fadb Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 12:38:33 -0400 Subject: [PATCH 08/15] transfer `waitForCheckout` to compression buffer --- packages/replay-internal/src/eventBuffer/EventBufferProxy.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index 5ca40c2dd8c9..89737952ac8e 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -104,7 +104,7 @@ export class EventBufferProxy implements EventBuffer { /** Switch the used buffer to the compression worker. */ private async _switchToCompressionWorker(): Promise { - const { events, hasCheckout } = this._fallback; + const { events, hasCheckout, waitForCheckout } = this._fallback; const addEventPromises: Promise[] = []; for (const event of events) { @@ -112,6 +112,7 @@ export class EventBufferProxy implements EventBuffer { } this._compression.hasCheckout = hasCheckout; + this._compression.waitForCheckout = waitForCheckout; // We switch over to the new buffer immediately - any further events will be added // after the previously buffered ones From 9f898c463fa2fa2aac773c3e2731680048cd3c10 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 12:38:56 -0400 Subject: [PATCH 09/15] revert some changes --- packages/replay-internal/src/util/addEvent.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 5aa420bfff15..81de9c4515a6 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,9 +54,7 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - const { eventBuffer } = replay; - - if (!eventBuffer || (eventBuffer.waitForCheckout && !isCheckout)) { + if (!replay.eventBuffer || (replay.eventBuffer.waitForCheckout && !isCheckout)) { return null; } @@ -64,12 +62,12 @@ async function _addEvent( try { if (isCheckout && isBufferMode) { - eventBuffer.clear(); + replay.eventBuffer.clear(); } if (isCheckout) { - eventBuffer.hasCheckout = true; - eventBuffer.waitForCheckout = false; + replay.eventBuffer.hasCheckout = true; + replay.eventBuffer.waitForCheckout = false; } const replayOptions = replay.getOptions(); @@ -80,15 +78,15 @@ async function _addEvent( return; } - return await eventBuffer.addEvent(eventAfterPossibleCallback); + return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { const isExceeded = error && error instanceof EventBufferSizeExceededError; const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; if (isExceeded && isBufferMode) { // Clear buffer and wait for next checkout - eventBuffer.clear(); - eventBuffer.waitForCheckout = true; + replay.eventBuffer.clear(); + replay.eventBuffer.waitForCheckout = true; return null; } From 48b9cb74f2fc43f0edf8562f941418d9b919a668 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 13:41:02 -0400 Subject: [PATCH 10/15] undo for tests --- packages/replay-internal/src/util/addEvent.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 81de9c4515a6..0e7dd5413682 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,7 +54,8 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - if (!replay.eventBuffer || (replay.eventBuffer.waitForCheckout && !isCheckout)) { + if (!replay.eventBuffer) { + // || (replay.eventBuffer.waitForCheckout && !isCheckout)) { return null; } @@ -83,13 +84,13 @@ async function _addEvent( const isExceeded = error && error instanceof EventBufferSizeExceededError; const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; - if (isExceeded && isBufferMode) { - // Clear buffer and wait for next checkout - replay.eventBuffer.clear(); - replay.eventBuffer.waitForCheckout = true; - - return null; - } + // if (isExceeded && isBufferMode) { + // // Clear buffer and wait for next checkout + // replay.eventBuffer.clear(); + // replay.eventBuffer.waitForCheckout = true; + // + // return null; + // } replay.handleException(error); From 893520f221c791b7b8bacca8ac8be3f8a9e67f87 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 14:01:33 -0400 Subject: [PATCH 11/15] Revert "undo for tests" This reverts commit b9921104e773435b0a9333b53cc04a790fbcb1c0. --- packages/replay-internal/src/util/addEvent.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 0e7dd5413682..81de9c4515a6 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,8 +54,7 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - if (!replay.eventBuffer) { - // || (replay.eventBuffer.waitForCheckout && !isCheckout)) { + if (!replay.eventBuffer || (replay.eventBuffer.waitForCheckout && !isCheckout)) { return null; } @@ -84,13 +83,13 @@ async function _addEvent( const isExceeded = error && error instanceof EventBufferSizeExceededError; const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; - // if (isExceeded && isBufferMode) { - // // Clear buffer and wait for next checkout - // replay.eventBuffer.clear(); - // replay.eventBuffer.waitForCheckout = true; - // - // return null; - // } + if (isExceeded && isBufferMode) { + // Clear buffer and wait for next checkout + replay.eventBuffer.clear(); + replay.eventBuffer.waitForCheckout = true; + + return null; + } replay.handleException(error); From 3620558f06e53aad1f7bb1ba771fc87056b797e5 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 16:07:39 -0400 Subject: [PATCH 12/15] forgot to add a setter --- packages/replay-internal/src/eventBuffer/EventBufferProxy.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index 89737952ac8e..b0de36e274a6 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -30,6 +30,11 @@ export class EventBufferProxy implements EventBuffer { return this._used.waitForCheckout; } + /** @inheritdoc */ + public set waitForCheckout(value: boolean) { + this._used.waitForCheckout = value; + } + /** @inheritdoc */ public get type(): EventBufferType { return this._used.type; From c4d883e4d0ecf71da4d502ca86d58accfd767ee9 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 16:18:21 -0400 Subject: [PATCH 13/15] Revert "revert some changes" This reverts commit e5be6d15650487c86eaab1afc858e87c363cb321. --- packages/replay-internal/src/util/addEvent.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index 81de9c4515a6..5aa420bfff15 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -54,7 +54,9 @@ async function _addEvent( event: RecordingEvent, isCheckout?: boolean, ): Promise { - if (!replay.eventBuffer || (replay.eventBuffer.waitForCheckout && !isCheckout)) { + const { eventBuffer } = replay; + + if (!eventBuffer || (eventBuffer.waitForCheckout && !isCheckout)) { return null; } @@ -62,12 +64,12 @@ async function _addEvent( try { if (isCheckout && isBufferMode) { - replay.eventBuffer.clear(); + eventBuffer.clear(); } if (isCheckout) { - replay.eventBuffer.hasCheckout = true; - replay.eventBuffer.waitForCheckout = false; + eventBuffer.hasCheckout = true; + eventBuffer.waitForCheckout = false; } const replayOptions = replay.getOptions(); @@ -78,15 +80,15 @@ async function _addEvent( return; } - return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); + return await eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { const isExceeded = error && error instanceof EventBufferSizeExceededError; const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent'; if (isExceeded && isBufferMode) { // Clear buffer and wait for next checkout - replay.eventBuffer.clear(); - replay.eventBuffer.waitForCheckout = true; + eventBuffer.clear(); + eventBuffer.waitForCheckout = true; return null; } From 55c2447610991e078a191b0a6cdd0a25c4ff3e4d Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 31 Oct 2024 16:18:59 -0400 Subject: [PATCH 14/15] lint --- .../src/eventBuffer/EventBufferProxy.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index b0de36e274a6..f4d5cca963e9 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -30,11 +30,6 @@ export class EventBufferProxy implements EventBuffer { return this._used.waitForCheckout; } - /** @inheritdoc */ - public set waitForCheckout(value: boolean) { - this._used.waitForCheckout = value; - } - /** @inheritdoc */ public get type(): EventBufferType { return this._used.type; @@ -54,6 +49,11 @@ export class EventBufferProxy implements EventBuffer { this._used.hasCheckout = value; } + /** @inheritdoc */ + public set waitForCheckout(value: boolean) { + this._used.waitForCheckout = value; + } + /** @inheritDoc */ public destroy(): void { this._fallback.destroy(); From bae04eb2adbda8f813d35cec318e7ea757b694ed Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 1 Nov 2024 11:36:33 -0400 Subject: [PATCH 15/15] conflicting eslint rules --- packages/replay-internal/src/eventBuffer/EventBufferProxy.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index f4d5cca963e9..c982567e49c1 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -50,6 +50,7 @@ export class EventBufferProxy implements EventBuffer { } /** @inheritdoc */ + // eslint-disable-next-line @typescript-eslint/adjacent-overload-signatures public set waitForCheckout(value: boolean) { this._used.waitForCheckout = value; }