From cbbc97e247e1a8828f0bda9980b7403cf1a12edc Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 12 Jan 2023 10:46:38 +0100 Subject: [PATCH] ref(replay): Mark all methods & properties as public/private --- packages/replay/.eslintrc.js | 2 - packages/replay/src/integration.ts | 42 +-- packages/replay/src/replay.ts | 306 +++++++++--------- .../test/integration/autoSaveSession.test.ts | 4 +- .../replay/test/integration/events.test.ts | 12 +- .../replay/test/integration/flush.test.ts | 62 ++-- .../replay/test/integration/sampling.test.ts | 6 +- .../test/integration/sendReplayEvent.test.ts | 32 +- .../replay/test/integration/session.test.ts | 2 +- packages/replay/test/integration/stop.test.ts | 2 +- .../test/unit/util/getReplayEvent.test.ts | 62 ---- 11 files changed, 238 insertions(+), 294 deletions(-) delete mode 100644 packages/replay/test/unit/util/getReplayEvent.test.ts diff --git a/packages/replay/.eslintrc.js b/packages/replay/.eslintrc.js index cd80e893576c..85d8dbf13a1e 100644 --- a/packages/replay/.eslintrc.js +++ b/packages/replay/.eslintrc.js @@ -23,8 +23,6 @@ module.exports = { { files: ['*.ts', '*.tsx', '*.d.ts'], rules: { - // TODO (high-prio): Re-enable this after migration - '@typescript-eslint/explicit-member-accessibility': 'off', // Since we target only es6 here, we can leave this off '@sentry-internal/sdk/no-async-await': 'off', }, diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index a7cadab5ccfd..274ec6147e34 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -34,13 +34,13 @@ export class Replay implements Integration { /** * Options to pass to `rrweb.record()` */ - readonly recordingOptions: RecordingOptions; + private readonly _recordingOptions: RecordingOptions; - readonly options: ReplayPluginOptions; + private readonly _options: ReplayPluginOptions; private _replay?: ReplayContainer; - constructor({ + public constructor({ flushMinDelay = DEFAULT_FLUSH_MIN_DELAY, flushMaxDelay = DEFAULT_FLUSH_MAX_DELAY, initialFlushDelay = INITIAL_FLUSH_DELAY, @@ -57,19 +57,19 @@ export class Replay implements Integration { ignoreClass = 'sentry-ignore', maskTextClass = 'sentry-mask', blockSelector = '[data-sentry-block]', - ...recordingOptions + ..._recordingOptions }: ReplayConfiguration = {}) { - this.recordingOptions = { + this._recordingOptions = { maskAllInputs, blockClass, ignoreClass, maskTextClass, maskTextSelector, blockSelector, - ...recordingOptions, + ..._recordingOptions, }; - this.options = { + this._options = { flushMinDelay, flushMaxDelay, stickySession, @@ -91,7 +91,7 @@ Instead, configure \`replaysSessionSampleRate\` directly in the SDK init options Sentry.init({ replaysSessionSampleRate: ${sessionSampleRate} })`, ); - this.options.sessionSampleRate = sessionSampleRate; + this._options.sessionSampleRate = sessionSampleRate; } if (typeof errorSampleRate === 'number') { @@ -103,22 +103,22 @@ Instead, configure \`replaysOnErrorSampleRate\` directly in the SDK init options Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, ); - this.options.errorSampleRate = errorSampleRate; + this._options.errorSampleRate = errorSampleRate; } - if (this.options.maskAllText) { + if (this._options.maskAllText) { // `maskAllText` is a more user friendly option to configure // `maskTextSelector`. This means that all nodes will have their text // content masked. - this.recordingOptions.maskTextSelector = MASK_ALL_TEXT_SELECTOR; + this._recordingOptions.maskTextSelector = MASK_ALL_TEXT_SELECTOR; } - if (this.options.blockAllMedia) { + if (this._options.blockAllMedia) { // `blockAllMedia` is a more user friendly option to configure blocking // embedded media elements - this.recordingOptions.blockSelector = !this.recordingOptions.blockSelector + this._recordingOptions.blockSelector = !this._recordingOptions.blockSelector ? MEDIA_SELECTORS - : `${this.recordingOptions.blockSelector},${MEDIA_SELECTORS}`; + : `${this._recordingOptions.blockSelector},${MEDIA_SELECTORS}`; } if (this._isInitialized && isBrowser()) { @@ -148,7 +148,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, * global event processors to finish. This is no longer needed, but keeping it * here to avoid any future issues. */ - setupOnce(): void { + public setupOnce(): void { if (!isBrowser()) { return; } @@ -165,7 +165,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, * Creates or loads a session, attaches listeners to varying events (DOM, * PerformanceObserver, Recording, Sentry SDK, etc) */ - start(): void { + public start(): void { if (!this._replay) { return; } @@ -177,7 +177,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, * Currently, this needs to be manually called (e.g. for tests). Sentry SDK * does not support a teardown */ - stop(): void { + public stop(): void { if (!this._replay) { return; } @@ -191,8 +191,8 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, this._loadReplayOptionsFromClient(); this._replay = new ReplayContainer({ - options: this.options, - recordingOptions: this.recordingOptions, + options: this._options, + recordingOptions: this._recordingOptions, }); } @@ -202,11 +202,11 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, const opt = client && (client.getOptions() as BrowserClientReplayOptions | undefined); if (opt && typeof opt.replaysSessionSampleRate === 'number') { - this.options.sessionSampleRate = opt.replaysSessionSampleRate; + this._options.sessionSampleRate = opt.replaysSessionSampleRate; } if (opt && typeof opt.replaysOnErrorSampleRate === 'number') { - this.options.errorSampleRate = opt.replaysOnErrorSampleRate; + this._options.errorSampleRate = opt.replaysOnErrorSampleRate; } } } diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 8bb123a36599..e82908defa58 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -128,11 +128,17 @@ export class ReplayContainer implements ReplayContainerInterface { initialUrl: '', }; - constructor({ options, recordingOptions }: { options: ReplayPluginOptions; recordingOptions: RecordingOptions }) { + public constructor({ + options, + recordingOptions, + }: { + options: ReplayPluginOptions; + recordingOptions: RecordingOptions; + }) { this._recordingOptions = recordingOptions; this._options = options; - this._debouncedFlush = debounce(() => this.flush(), this._options.flushMinDelay, { + this._debouncedFlush = debounce(() => this._flush(), this._options.flushMinDelay, { maxWait: this._options.flushMaxDelay, }); } @@ -163,14 +169,14 @@ export class ReplayContainer implements ReplayContainerInterface { * Creates or loads a session, attaches listeners to varying events (DOM, * _performanceObserver, Recording, Sentry SDK, etc) */ - start(): void { - this.setInitialState(); + public start(): void { + this._setInitialState(); - this.loadSession({ expiry: SESSION_IDLE_DURATION }); + this._loadSession({ expiry: SESSION_IDLE_DURATION }); // If there is no session, then something bad has happened - can't continue if (!this.session) { - this.handleException(new Error('No session found')); + this._handleException(new Error('No session found')); return; } @@ -187,13 +193,13 @@ export class ReplayContainer implements ReplayContainerInterface { // setup() is generally called on page load or manually - in both cases we // should treat it as an activity - this.updateSessionActivity(); + this._updateSessionActivity(); this.eventBuffer = createEventBuffer({ useCompression: Boolean(this._options.useCompression), }); - this.addListeners(); + this._addListeners(); // Need to set as enabled before we start recording, as `record()` can trigger a flush with a new checkout this._isEnabled = true; @@ -206,7 +212,7 @@ export class ReplayContainer implements ReplayContainerInterface { * * Note that this will cause a new DOM checkout */ - startRecording(): void { + public startRecording(): void { try { this._stopRecording = record({ ...this._recordingOptions, @@ -214,10 +220,10 @@ export class ReplayContainer implements ReplayContainerInterface { // Without this, it would record forever, until an error happens, which we don't want // instead, we'll always keep the last 60 seconds of replay before an error happened ...(this.recordingMode === 'error' && { checkoutEveryNms: 60000 }), - emit: this.handleRecordingEmit, + emit: this._handleRecordingEmit, }); } catch (err) { - this.handleException(err); + this._handleException(err); } } @@ -238,16 +244,16 @@ export class ReplayContainer implements ReplayContainerInterface { * Currently, this needs to be manually called (e.g. for tests). Sentry SDK * does not support a teardown */ - stop(): void { + public stop(): void { try { __DEBUG_BUILD__ && logger.log('[Replay] Stopping Replays'); this._isEnabled = false; - this.removeListeners(); + this._removeListeners(); this._stopRecording?.(); this.eventBuffer?.destroy(); this.eventBuffer = null; } catch (err) { - this.handleException(err); + this._handleException(err); } } @@ -256,7 +262,7 @@ export class ReplayContainer implements ReplayContainerInterface { * This differs from stop as this only stops DOM recording, it is * not as thorough of a shutdown as `stop()`. */ - pause(): void { + public pause(): void { this._isPaused = true; try { if (this._stopRecording) { @@ -264,7 +270,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._stopRecording = undefined; } } catch (err) { - this.handleException(err); + this._handleException(err); } } @@ -274,13 +280,80 @@ export class ReplayContainer implements ReplayContainerInterface { * Note that calling `startRecording()` here will cause a * new DOM checkout.` */ - resume(): void { + public resume(): void { this._isPaused = false; this.startRecording(); } + /** + * We want to batch uploads of replay events. Save events only if + * `` milliseconds have elapsed since the last event + * *OR* if `` milliseconds have elapsed. + * + * Accepts a callback to perform side-effects and returns true to stop batch + * processing and hand back control to caller. + */ + public addUpdate(cb: AddUpdateCallback): void { + // We need to always run `cb` (e.g. in the case of `this.recordingMode == 'error'`) + const cbResult = cb?.(); + + // If this option is turned on then we will only want to call `flush` + // explicitly + if (this.recordingMode === 'error') { + return; + } + + // If callback is true, we do not want to continue with flushing -- the + // caller will need to handle it. + if (cbResult === true) { + return; + } + + // addUpdate is called quite frequently - use _debouncedFlush so that it + // respects the flush delays and does not flush immediately + this._debouncedFlush(); + } + + /** + * Updates the user activity timestamp and resumes recording. This should be + * called in an event handler for a user action that we consider as the user + * being "active" (e.g. a mouse click). + */ + public triggerUserActivity(): void { + this._updateUserActivity(); + + // This case means that recording was once stopped due to inactivity. + // Ensure that recording is resumed. + if (!this._stopRecording) { + // Create a new session, otherwise when the user action is flushed, it + // will get rejected due to an expired session. + this._loadSession({ expiry: SESSION_IDLE_DURATION }); + + // Note: This will cause a new DOM checkout + this.resume(); + return; + } + + // Otherwise... recording was never suspended, continue as normalish + this._checkAndHandleExpiredSession(); + + this._updateSessionActivity(); + } + + /** + * + * 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. + */ + public flushImmediate(): Promise { + this._debouncedFlush(); + // `.flush` is provided by the debounced function, analogously to lodash.debounce + return this._debouncedFlush.flush() as Promise; + } + /** A wrapper to conditionally capture exceptions. */ - handleException(error: unknown): void { + private _handleException(error: unknown): void { __DEBUG_BUILD__ && logger.error('[Replay]', error); if (__DEBUG_BUILD__ && this._options._experiments && this._options._experiments.captureExceptions) { @@ -292,7 +365,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Loads a session from storage, or creates a new one if it does not exist or * is expired. */ - loadSession({ expiry }: { expiry: number }): void { + private _loadSession({ expiry }: { expiry: number }): void { const { type, session } = getSession({ expiry, stickySession: Boolean(this._options.stickySession), @@ -304,7 +377,7 @@ export class ReplayContainer implements ReplayContainerInterface { // If session was newly created (i.e. was not loaded from storage), then // enable flag to create the root replay if (type === 'new') { - this.setInitialState(); + this._setInitialState(); } if (session.id !== this.session?.id) { @@ -319,14 +392,14 @@ export class ReplayContainer implements ReplayContainerInterface { * replay. This is required because otherwise they would be captured at the * first flush. */ - setInitialState(): void { + private _setInitialState(): void { const urlPath = `${WINDOW.location.pathname}${WINDOW.location.hash}${WINDOW.location.search}`; const url = `${WINDOW.location.origin}${urlPath}`; this.performanceEvents = []; // Reset _context as well - this.clearContext(); + this._clearContext(); this._context.initialUrl = url; this._context.initialTimestamp = new Date().getTime(); @@ -336,11 +409,11 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Adds listeners to record events for the replay */ - addListeners(): void { + private _addListeners(): void { try { - WINDOW.document.addEventListener('visibilitychange', this.handleVisibilityChange); - WINDOW.addEventListener('blur', this.handleWindowBlur); - WINDOW.addEventListener('focus', this.handleWindowFocus); + WINDOW.document.addEventListener('visibilitychange', this._handleVisibilityChange); + WINDOW.addEventListener('blur', this._handleWindowBlur); + WINDOW.addEventListener('focus', this._handleWindowFocus); // We need to filter out dropped events captured by `addGlobalEventProcessor(this.handleGlobalEvent)` below overwriteRecordDroppedEvent(this._context.errorIds); @@ -349,8 +422,8 @@ export class ReplayContainer implements ReplayContainerInterface { if (!this._hasInitializedCoreListeners) { // Listeners from core SDK // const scope = getCurrentHub().getScope(); - scope?.addScopeListener(this.handleCoreBreadcrumbListener('scope')); - addInstrumentationHandler('dom', this.handleCoreBreadcrumbListener('dom')); + scope?.addScopeListener(this._handleCoreBreadcrumbListener('scope')); + addInstrumentationHandler('dom', this._handleCoreBreadcrumbListener('dom')); addInstrumentationHandler('fetch', handleFetchSpanListener(this)); addInstrumentationHandler('xhr', handleXhrSpanListener(this)); addInstrumentationHandler('history', handleHistorySpanListener(this)); @@ -362,7 +435,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._hasInitializedCoreListeners = true; } } catch (err) { - this.handleException(err); + this._handleException(err); } // _performanceObserver // @@ -374,14 +447,14 @@ export class ReplayContainer implements ReplayContainerInterface { } /** - * Cleans up listeners that were created in `addListeners` + * Cleans up listeners that were created in `_addListeners` */ - removeListeners(): void { + private _removeListeners(): void { try { - WINDOW.document.removeEventListener('visibilitychange', this.handleVisibilityChange); + WINDOW.document.removeEventListener('visibilitychange', this._handleVisibilityChange); - WINDOW.removeEventListener('blur', this.handleWindowBlur); - WINDOW.removeEventListener('focus', this.handleWindowFocus); + WINDOW.removeEventListener('blur', this._handleWindowBlur); + WINDOW.removeEventListener('focus', this._handleWindowFocus); restoreRecordDroppedEvent(); @@ -390,50 +463,21 @@ export class ReplayContainer implements ReplayContainerInterface { this._performanceObserver = null; } } catch (err) { - this.handleException(err); + this._handleException(err); } } - /** - * We want to batch uploads of replay events. Save events only if - * `` milliseconds have elapsed since the last event - * *OR* if `` milliseconds have elapsed. - * - * Accepts a callback to perform side-effects and returns true to stop batch - * processing and hand back control to caller. - */ - addUpdate(cb: AddUpdateCallback): void { - // We need to always run `cb` (e.g. in the case of `this.recordingMode == 'error'`) - const cbResult = cb?.(); - - // If this option is turned on then we will only want to call `flush` - // explicitly - if (this.recordingMode === 'error') { - return; - } - - // If callback is true, we do not want to continue with flushing -- the - // caller will need to handle it. - if (cbResult === true) { - return; - } - - // addUpdate is called quite frequently - use _debouncedFlush so that it - // respects the flush delays and does not flush immediately - this._debouncedFlush(); - } - /** * Handler for recording events. * * Adds to event buffer, and has varying flushing behaviors if the event was a checkout. */ - handleRecordingEmit: (event: RecordingEvent, isCheckout?: boolean) => void = ( + private _handleRecordingEmit: (event: RecordingEvent, isCheckout?: boolean) => void = ( event: RecordingEvent, isCheckout?: boolean, ) => { // If this is false, it means session is expired, create and a new session and wait for checkout - if (!this.checkAndHandleExpiredSession()) { + if (!this._checkAndHandleExpiredSession()) { __DEBUG_BUILD__ && logger.error('[Replay] Received replay event after session expired.'); return; @@ -446,7 +490,7 @@ export class ReplayContainer implements ReplayContainerInterface { // checkout. This needs to happen before `addEvent()` which updates state // dependent on this reset. if (this.recordingMode === 'error' && event.type === 2) { - this.setInitialState(); + this._setInitialState(); } // We need to clear existing events on a checkout, otherwise they are @@ -494,38 +538,38 @@ export class ReplayContainer implements ReplayContainerInterface { * be hidden. Likewise, moving a different window to cover the contents of the * page will also trigger a change to a hidden state. */ - handleVisibilityChange: () => void = () => { + private _handleVisibilityChange: () => void = () => { if (WINDOW.document.visibilityState === 'visible') { - this.doChangeToForegroundTasks(); + this._doChangeToForegroundTasks(); } else { - this.doChangeToBackgroundTasks(); + this._doChangeToBackgroundTasks(); } }; /** * Handle when page is blurred */ - handleWindowBlur: () => void = () => { + private _handleWindowBlur: () => void = () => { const breadcrumb = createBreadcrumb({ category: 'ui.blur', }); // Do not count blur as a user action -- it's part of the process of them // leaving the page - this.doChangeToBackgroundTasks(breadcrumb); + this._doChangeToBackgroundTasks(breadcrumb); }; /** * Handle when page is focused */ - handleWindowFocus: () => void = () => { + private _handleWindowFocus: () => void = () => { const breadcrumb = createBreadcrumb({ category: 'ui.focus', }); // Do not count focus as a user action -- instead wait until they focus and // interactive with page - this.doChangeToForegroundTasks(breadcrumb); + this._doChangeToForegroundTasks(breadcrumb); }; /** @@ -533,7 +577,7 @@ export class ReplayContainer implements ReplayContainerInterface { * * These events will create breadcrumb-like objects in the recording. */ - handleCoreBreadcrumbListener: (type: InstrumentationTypeBreadcrumb) => (handlerData: unknown) => void = + private _handleCoreBreadcrumbListener: (type: InstrumentationTypeBreadcrumb) => (handlerData: unknown) => void = (type: InstrumentationTypeBreadcrumb) => (handlerData: unknown): void => { if (!this._isEnabled) { @@ -553,7 +597,7 @@ export class ReplayContainer implements ReplayContainerInterface { if (result.category === 'ui.click') { this.triggerUserActivity(); } else { - this.checkAndHandleExpiredSession(); + this._checkAndHandleExpiredSession(); } this.addUpdate(() => { @@ -576,7 +620,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Tasks to run when we consider a page to be hidden (via blurring and/or visibility) */ - doChangeToBackgroundTasks(breadcrumb?: Breadcrumb): void { + private _doChangeToBackgroundTasks(breadcrumb?: Breadcrumb): void { if (!this.session) { return; } @@ -584,24 +628,24 @@ export class ReplayContainer implements ReplayContainerInterface { const expired = isSessionExpired(this.session, VISIBILITY_CHANGE_TIMEOUT); if (breadcrumb && !expired) { - this.createCustomBreadcrumb(breadcrumb); + this._createCustomBreadcrumb(breadcrumb); } // 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(); } /** * Tasks to run when we consider a page to be visible (via focus and/or visibility) */ - doChangeToForegroundTasks(breadcrumb?: Breadcrumb): void { + private _doChangeToForegroundTasks(breadcrumb?: Breadcrumb): void { if (!this.session) { return; } - const isSessionActive = this.checkAndHandleExpiredSession({ + const isSessionActive = this._checkAndHandleExpiredSession({ expiry: VISIBILITY_CHANGE_TIMEOUT, }); @@ -614,7 +658,7 @@ export class ReplayContainer implements ReplayContainerInterface { } if (breadcrumb) { - this.createCustomBreadcrumb(breadcrumb); + this._createCustomBreadcrumb(breadcrumb); } } @@ -622,7 +666,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Trigger rrweb to take a full snapshot which will cause this plugin to * create a new Replay event. */ - triggerFullSnapshot(): void { + private _triggerFullSnapshot(): void { __DEBUG_BUILD__ && logger.log('[Replay] Taking full rrweb snapshot'); record.takeFullSnapshot(true); } @@ -630,50 +674,24 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Update user activity (across session lifespans) */ - updateUserActivity(_lastActivity: number = new Date().getTime()): void { + private _updateUserActivity(_lastActivity: number = new Date().getTime()): void { this._lastActivity = _lastActivity; } /** * Updates the session's last activity timestamp */ - updateSessionActivity(_lastActivity: number = new Date().getTime()): void { + private _updateSessionActivity(_lastActivity: number = new Date().getTime()): void { if (this.session) { this.session.lastActivity = _lastActivity; this._maybeSaveSession(); } } - /** - * Updates the user activity timestamp and resumes recording. This should be - * called in an event handler for a user action that we consider as the user - * being "active" (e.g. a mouse click). - */ - triggerUserActivity(): void { - this.updateUserActivity(); - - // This case means that recording was once stopped due to inactivity. - // Ensure that recording is resumed. - if (!this._stopRecording) { - // Create a new session, otherwise when the user action is flushed, it - // will get rejected due to an expired session. - this.loadSession({ expiry: SESSION_IDLE_DURATION }); - - // Note: This will cause a new DOM checkout - this.resume(); - return; - } - - // Otherwise... recording was never suspended, continue as normalish - this.checkAndHandleExpiredSession(); - - this.updateSessionActivity(); - } - /** * Helper to create (and buffer) a replay breadcrumb from a core SDK breadcrumb */ - createCustomBreadcrumb(breadcrumb: Breadcrumb): void { + private _createCustomBreadcrumb(breadcrumb: Breadcrumb): void { this.addUpdate(() => { void addEvent(this, { type: EventType.Custom, @@ -690,7 +708,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Observed performance events are added to `this.performanceEvents`. These * are included in the replay event before it is finished and sent to Sentry. */ - addPerformanceEntries(): Promise> { + private _addPerformanceEntries(): Promise> { // Copy and reset entries before processing const entries = [...this.performanceEvents]; this.performanceEvents = []; @@ -705,7 +723,7 @@ export class ReplayContainer implements ReplayContainerInterface { * * Returns true if session is not expired, false otherwise. */ - checkAndHandleExpiredSession({ expiry = SESSION_IDLE_DURATION }: { expiry?: number } = {}): boolean | void { + private _checkAndHandleExpiredSession({ expiry = SESSION_IDLE_DURATION }: { expiry?: number } = {}): boolean | void { const oldSessionId = this.session?.id; // Prevent starting a new session if the last user activity is older than @@ -720,7 +738,7 @@ export class ReplayContainer implements ReplayContainerInterface { // --- There is recent user activity --- // // This will create a new session if expired, based on expiry length - this.loadSession({ expiry }); + this._loadSession({ expiry }); // Session was expired if session ids do not match const expired = oldSessionId !== this.session?.id; @@ -730,7 +748,7 @@ export class ReplayContainer implements ReplayContainerInterface { } // Session is expired, trigger a full snapshot (which will create a new session) - this.triggerFullSnapshot(); + this._triggerFullSnapshot(); return false; } @@ -738,7 +756,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Only flush if `this.recordingMode === 'session'` */ - conditionalFlush(): void { + private _conditionalFlush(): void { if (this.recordingMode === 'error') { return; } @@ -749,7 +767,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Clear _context */ - clearContext(): void { + private _clearContext(): void { // XXX: `initialTimestamp` and `initialUrl` do not get cleared this._context.errorIds.clear(); this._context.traceIds.clear(); @@ -760,7 +778,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Return and clear _context */ - popEventContext(): PopEventContext { + private _popEventContext(): PopEventContext { if (this._context.earliestEvent && this._context.earliestEvent < this._context.initialTimestamp) { this._context.initialTimestamp = this._context.earliestEvent; } @@ -773,7 +791,7 @@ export class ReplayContainer implements ReplayContainerInterface { urls: this._context.urls, }; - this.clearContext(); + this._clearContext(); return _context; } @@ -786,13 +804,13 @@ export class ReplayContainer implements ReplayContainerInterface { * * Should never be called directly, only by `flush` */ - async runFlush(): Promise { + private async _runFlush(): Promise { if (!this.session) { __DEBUG_BUILD__ && logger.error('[Replay] No session found to flush.'); return; } - await this.addPerformanceEntries(); + await this._addPerformanceEntries(); if (!this.eventBuffer?.pendingLength) { return; @@ -808,12 +826,12 @@ export class ReplayContainer implements ReplayContainerInterface { // 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(); + const eventContext = this._popEventContext(); // Always increment segmentId regardless of outcome of sending replay const segmentId = this.session.segmentId++; this._maybeSaveSession(); - await this.sendReplay({ + await this._sendReplay({ replayId, events: recordingData, segmentId, @@ -821,7 +839,7 @@ export class ReplayContainer implements ReplayContainerInterface { eventContext, }); } catch (err) { - this.handleException(err); + this._handleException(err); } } @@ -829,14 +847,14 @@ export class ReplayContainer implements ReplayContainerInterface { * Flush recording data to Sentry. Creates a lock so that only a single flush * can be active at a time. Do not call this directly. */ - flush: () => Promise = async () => { + private _flush: () => Promise = async () => { if (!this._isEnabled) { // This is just a precaution, there should be no listeners that would // cause a flush. return; } - if (!this.checkAndHandleExpiredSession()) { + if (!this._checkAndHandleExpiredSession()) { __DEBUG_BUILD__ && logger.error('[Replay] Attempting to finish replay event after session expired.'); return; } @@ -849,16 +867,16 @@ export class ReplayContainer implements ReplayContainerInterface { // A flush is about to happen, cancel any queued flushes this._debouncedFlush?.cancel(); - // this._flushLock acts as a lock so that future calls to `flush()` + // this._flushLock acts as a lock so that future calls to `_flush()` // will be blocked until this promise resolves if (!this._flushLock) { - this._flushLock = this.runFlush(); + this._flushLock = this._runFlush(); await this._flushLock; this._flushLock = null; return; } - // Wait for previous flush to finish, then call the debounced `flush()`. + // Wait for previous flush to finish, then call the debounced `_flush()`. // It's possible there are other flush requests queued and waiting for it // to resolve. We want to reduce all outstanding requests (as well as any // new flush requests that occur within a second of the locked flush @@ -873,22 +891,10 @@ 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. - */ - flushImmediate(): Promise { - this._debouncedFlush(); - // `.flush` is provided by the debounced function, analogously to lodash.debounce - return this._debouncedFlush.flush() as Promise; - } - /** * Send replay attachment using `fetch()` */ - async sendReplayRequest({ + private async _sendReplayRequest({ events, replayId, segmentId: segment_id, @@ -991,7 +997,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Reset the counter of retries for sending replays. */ - resetRetries(): void { + private _resetRetries(): void { this._retryCount = 0; this._retryInterval = BASE_RETRY_INTERVAL; } @@ -999,34 +1005,34 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Finalize and send the current replay event to Sentry */ - async sendReplay({ + private async _sendReplay({ replayId, events, segmentId, includeReplayStartTimestamp, eventContext, }: SendReplay): Promise { - // short circuit if there's no events to upload (this shouldn't happen as runFlush makes this check) + // short circuit if there's no events to upload (this shouldn't happen as _runFlush makes this check) if (!events.length) { return; } try { - await this.sendReplayRequest({ + await this._sendReplayRequest({ events, replayId, segmentId, includeReplayStartTimestamp, eventContext, }); - this.resetRetries(); + this._resetRetries(); return true; } catch (err) { // Capture error for every failed replay setContext('Replays', { _retryCount: this._retryCount, }); - this.handleException(err); + this._handleException(err); // If an error happened here, it's likely that uploading the attachment // failed, we'll can retry with the same events payload @@ -1041,7 +1047,7 @@ export class ReplayContainer implements ReplayContainerInterface { return await new Promise((resolve, reject) => { setTimeout(async () => { try { - await this.sendReplay({ + await this._sendReplay({ replayId, events, segmentId, diff --git a/packages/replay/test/integration/autoSaveSession.test.ts b/packages/replay/test/integration/autoSaveSession.test.ts index 5cdcf3a6525b..fb047e07bcc8 100644 --- a/packages/replay/test/integration/autoSaveSession.test.ts +++ b/packages/replay/test/integration/autoSaveSession.test.ts @@ -35,7 +35,7 @@ describe('Integration | autoSaveSession', () => { // Initially called up to three times: once for start, then once for replay.updateSessionActivity & once for segmentId increase expect(saveSessionSpy).toHaveBeenCalledTimes(addSummand * 3); - replay.updateSessionActivity(); + replay['_updateSessionActivity'](); expect(saveSessionSpy).toHaveBeenCalledTimes(addSummand * 4); @@ -50,7 +50,7 @@ describe('Integration | autoSaveSession', () => { addEvent(replay, event); - await replay.runFlush(); + await replay['_runFlush'](); expect(saveSessionSpy).toHaveBeenCalledTimes(addSummand * 5); }); diff --git a/packages/replay/test/integration/events.test.ts b/packages/replay/test/integration/events.test.ts index ed74b6978fe6..9bfe34484ce6 100644 --- a/packages/replay/test/integration/events.test.ts +++ b/packages/replay/test/integration/events.test.ts @@ -23,7 +23,7 @@ describe('Integration | events', () => { let mockTransportSend: jest.SpyInstance; const prevLocation = WINDOW.location; - type MockSendReplayRequest = jest.MockedFunction; + type MockSendReplayRequest = jest.MockedFunction; let mockSendReplayRequest: MockSendReplayRequest; beforeAll(async () => { @@ -40,16 +40,14 @@ describe('Integration | events', () => { mockTransportSend = jest.spyOn(getCurrentHub().getClient()!.getTransport()!, 'send'); - jest.spyOn(replay, 'flush'); - jest.spyOn(replay, 'runFlush'); - jest.spyOn(replay, 'sendReplayRequest'); + // @ts-ignore private API + mockSendReplayRequest = jest.spyOn(replay, '_sendReplayRequest'); // Create a new session and clear mocks because a segment (from initial // checkout) will have already been uploaded by the time the tests run clearSession(replay); - replay.loadSession({ expiry: 0 }); + replay['_loadSession']({ expiry: 0 }); mockTransportSend.mockClear(); - mockSendReplayRequest = replay.sendReplayRequest as MockSendReplayRequest; mockSendReplayRequest.mockClear(); }); @@ -103,7 +101,7 @@ describe('Integration | events', () => { it('has correct timestamps when there are events earlier than initial timestamp', async function () { clearSession(replay); - replay.loadSession({ expiry: 0 }); + replay['_loadSession']({ expiry: 0 }); mockTransportSend.mockClear(); Object.defineProperty(document, 'visibilityState', { configurable: true, diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index 7e9277b9d47a..f2760ef43371 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -2,6 +2,7 @@ import * as SentryUtils from '@sentry/utils'; import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; +import type { EventBuffer } from '../../src/types'; import * as AddMemoryEntry from '../../src/util/addMemoryEntry'; import { createPerformanceEntries } from '../../src/util/createPerformanceEntries'; import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; @@ -16,12 +17,12 @@ async function advanceTimers(time: number) { await new Promise(process.nextTick); } -type MockSendReplay = jest.MockedFunction; -type MockAddPerformanceEntries = jest.MockedFunction; +type MockSendReplay = jest.MockedFunction; +type MockAddPerformanceEntries = jest.MockedFunction; type MockAddMemoryEntry = jest.SpyInstance; -type MockEventBufferFinish = jest.MockedFunction['finish']>; -type MockFlush = jest.MockedFunction; -type MockRunFlush = jest.MockedFunction; +type MockEventBufferFinish = jest.MockedFunction; +type MockFlush = jest.MockedFunction; +type MockRunFlush = jest.MockedFunction; const prevLocation = WINDOW.location; @@ -46,22 +47,23 @@ describe('Integration | flush', () => { }); ({ replay } = await mockSdk()); - jest.spyOn(replay, 'sendReplay'); - mockSendReplay = replay.sendReplay as MockSendReplay; + + // @ts-ignore private API + mockSendReplay = jest.spyOn(replay, '_sendReplay'); mockSendReplay.mockImplementation( jest.fn(async () => { return; }), ); - jest.spyOn(replay, 'flush'); - mockFlush = replay.flush as MockFlush; + // @ts-ignore private API + mockFlush = jest.spyOn(replay, '_flush'); - jest.spyOn(replay, 'runFlush'); - mockRunFlush = replay.runFlush as MockRunFlush; + // @ts-ignore private API + mockRunFlush = jest.spyOn(replay, '_runFlush'); - jest.spyOn(replay, 'addPerformanceEntries'); - mockAddPerformanceEntries = replay.addPerformanceEntries as MockAddPerformanceEntries; + // @ts-ignore private API + mockAddPerformanceEntries = jest.spyOn(replay, '_addPerformanceEntries'); mockAddPerformanceEntries.mockImplementation(async () => { return []; @@ -93,7 +95,7 @@ describe('Integration | flush', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); clearSession(replay); - replay.loadSession({ expiry: SESSION_IDLE_DURATION }); + replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); mockRecord.takeFullSnapshot.mockClear(); Object.defineProperty(WINDOW, 'location', { value: prevLocation, @@ -116,19 +118,19 @@ describe('Integration | flush', () => { WINDOW.dispatchEvent(new Event('blur')); WINDOW.dispatchEvent(new Event('blur')); - expect(replay.flush).toHaveBeenCalledTimes(4); + expect(mockFlush).toHaveBeenCalledTimes(4); jest.runAllTimers(); await new Promise(process.nextTick); - expect(replay.runFlush).toHaveBeenCalledTimes(1); + expect(mockRunFlush).toHaveBeenCalledTimes(1); jest.runAllTimers(); await new Promise(process.nextTick); - expect(replay.runFlush).toHaveBeenCalledTimes(2); + expect(mockRunFlush).toHaveBeenCalledTimes(2); jest.runAllTimers(); await new Promise(process.nextTick); - expect(replay.runFlush).toHaveBeenCalledTimes(2); + expect(mockRunFlush).toHaveBeenCalledTimes(2); }); it('long first flush enqueues following events', async () => { @@ -141,8 +143,8 @@ describe('Integration | flush', () => { // flush #1 @ t=0s - due to blur WINDOW.dispatchEvent(new Event('blur')); - expect(replay.flush).toHaveBeenCalledTimes(1); - expect(replay.runFlush).toHaveBeenCalledTimes(1); + expect(mockFlush).toHaveBeenCalledTimes(1); + expect(mockRunFlush).toHaveBeenCalledTimes(1); // This will attempt to flush in 5 seconds (flushMinDelay) domHandler({ @@ -150,28 +152,28 @@ describe('Integration | flush', () => { }); await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); // flush #2 @ t=5s - due to click - expect(replay.flush).toHaveBeenCalledTimes(2); + expect(mockFlush).toHaveBeenCalledTimes(2); await advanceTimers(1000); // flush #3 @ t=6s - due to blur WINDOW.dispatchEvent(new Event('blur')); - expect(replay.flush).toHaveBeenCalledTimes(3); + expect(mockFlush).toHaveBeenCalledTimes(3); // NOTE: Blur also adds a breadcrumb which calls `addUpdate`, meaning it will // flush after `flushMinDelay`, but this gets cancelled by the blur await advanceTimers(8000); - expect(replay.flush).toHaveBeenCalledTimes(3); + expect(mockFlush).toHaveBeenCalledTimes(3); // flush #4 @ t=14s - due to blur WINDOW.dispatchEvent(new Event('blur')); - expect(replay.flush).toHaveBeenCalledTimes(4); + expect(mockFlush).toHaveBeenCalledTimes(4); - expect(replay.runFlush).toHaveBeenCalledTimes(1); + expect(mockRunFlush).toHaveBeenCalledTimes(1); await advanceTimers(6000); // t=20s // addPerformanceEntries is finished, `flushLock` promise is resolved, calls // debouncedFlush, which will call `flush` in 1 second - expect(replay.flush).toHaveBeenCalledTimes(4); + expect(mockFlush).toHaveBeenCalledTimes(4); // sendReplay is called with replayId, events, segment expect(mockSendReplay).toHaveBeenLastCalledWith({ events: expect.any(String), @@ -218,8 +220,8 @@ describe('Integration | flush', () => { // flush #5 @ t=25s - debounced flush calls `flush` // 20s + `flushMinDelay` which is 5 seconds await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(replay.flush).toHaveBeenCalledTimes(5); - expect(replay.runFlush).toHaveBeenCalledTimes(2); + expect(mockFlush).toHaveBeenCalledTimes(5); + expect(mockRunFlush).toHaveBeenCalledTimes(2); expect(mockSendReplay).toHaveBeenLastCalledWith({ events: expect.any(String), replayId: expect.any(String), @@ -245,10 +247,10 @@ describe('Integration | flush', () => { mockRecord._emitter(TEST_EVENT); await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(replay.flush).toHaveBeenCalledTimes(1); + expect(mockFlush).toHaveBeenCalledTimes(1); // Make sure there's nothing queued up after await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(replay.flush).toHaveBeenCalledTimes(1); + expect(mockFlush).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/replay/test/integration/sampling.test.ts b/packages/replay/test/integration/sampling.test.ts index d17052a4e7f6..049329ebda3e 100644 --- a/packages/replay/test/integration/sampling.test.ts +++ b/packages/replay/test/integration/sampling.test.ts @@ -16,8 +16,8 @@ describe('Integration | sampling', () => { }, }); - jest.spyOn(replay, 'loadSession'); - jest.spyOn(replay, 'addListeners'); + // @ts-ignore private API + const spyAddListeners = jest.spyOn(replay, '_addListeners'); jest.runAllTimers(); expect(replay.session?.sampled).toBe(false); @@ -28,6 +28,6 @@ describe('Integration | sampling', () => { }), ); expect(mockRecord).not.toHaveBeenCalled(); - expect(replay.addListeners).not.toHaveBeenCalled(); + expect(spyAddListeners).not.toHaveBeenCalled(); }); }); diff --git a/packages/replay/test/integration/sendReplayEvent.test.ts b/packages/replay/test/integration/sendReplayEvent.test.ts index d9db9ea42a28..ade380a1605a 100644 --- a/packages/replay/test/integration/sendReplayEvent.test.ts +++ b/packages/replay/test/integration/sendReplayEvent.test.ts @@ -17,7 +17,7 @@ async function advanceTimers(time: number) { } type MockTransportSend = jest.MockedFunction; -type MockSendReplayRequest = jest.MockedFunction; +type MockSendReplayRequest = jest.MockedFunction; describe('Integration | sendReplayEvent', () => { let replay: ReplayContainer; @@ -40,11 +40,11 @@ describe('Integration | sendReplayEvent', () => { }, })); - jest.spyOn(replay, 'sendReplayRequest'); + // @ts-ignore private API + mockSendReplayRequest = jest.spyOn(replay, '_sendReplayRequest'); jest.runAllTimers(); mockTransportSend = getCurrentHub()?.getClient()?.getTransport()?.send as MockTransportSend; - mockSendReplayRequest = replay.sendReplayRequest as MockSendReplayRequest; }); beforeEach(() => { @@ -55,7 +55,7 @@ describe('Integration | sendReplayEvent', () => { // Create a new session and clear mocks because a segment (from initial // checkout) will have already been uploaded by the time the tests run clearSession(replay); - replay.loadSession({ expiry: 0 }); + replay['_loadSession']({ expiry: 0 }); mockSendReplayRequest.mockClear(); }); @@ -65,7 +65,7 @@ describe('Integration | sendReplayEvent', () => { await new Promise(process.nextTick); jest.setSystemTime(new Date(BASE_TIMESTAMP)); clearSession(replay); - replay.loadSession({ expiry: SESSION_IDLE_DURATION }); + replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); }); afterAll(() => { @@ -382,13 +382,15 @@ describe('Integration | sendReplayEvent', () => { it('fails to upload data and hits retry max and stops', async () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - jest.spyOn(replay, 'sendReplay'); + + // @ts-ignore private API + const spySendReplay = jest.spyOn(replay, '_sendReplay'); // Suppress console.errors const mockConsole = jest.spyOn(console, 'error').mockImplementation(jest.fn()); - // Check errors - const spyHandleException = jest.spyOn(replay, 'handleException'); + // @ts-ignore privaye api - Check errors + const spyHandleException = jest.spyOn(replay, '_handleException'); expect(replay.session?.segmentId).toBe(0); @@ -402,24 +404,24 @@ describe('Integration | sendReplayEvent', () => { await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay.sendReplayRequest).toHaveBeenCalledTimes(1); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(1); await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(replay.sendReplayRequest).toHaveBeenCalledTimes(2); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(2); await advanceTimers(10000); - expect(replay.sendReplayRequest).toHaveBeenCalledTimes(3); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(3); await advanceTimers(30000); - expect(replay.sendReplayRequest).toHaveBeenCalledTimes(4); - expect(replay.sendReplay).toHaveBeenCalledTimes(4); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(4); + expect(spySendReplay).toHaveBeenCalledTimes(4); mockConsole.mockReset(); // Make sure it doesn't retry again jest.runAllTimers(); - expect(replay.sendReplayRequest).toHaveBeenCalledTimes(4); - expect(replay.sendReplay).toHaveBeenCalledTimes(4); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(4); + expect(spySendReplay).toHaveBeenCalledTimes(4); // Retries = 3 (total tries = 4 including initial attempt) // + last exception is max retries exceeded diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index 46ef58622372..c07bac150751 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -451,7 +451,7 @@ describe('Integration | session', () => { it('increases segment id after each event', async () => { clearSession(replay); - replay.loadSession({ expiry: 0 }); + replay['_loadSession']({ expiry: 0 }); Object.defineProperty(document, 'visibilityState', { configurable: true, diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index 0b0e164cc8b8..42ccbe09ca68 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -44,7 +44,7 @@ describe('Integration | stop', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); clearSession(replay); - replay.loadSession({ expiry: SESSION_IDLE_DURATION }); + replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); mockRecord.takeFullSnapshot.mockClear(); mockAddInstrumentationHandler.mockClear(); Object.defineProperty(WINDOW, 'location', { diff --git a/packages/replay/test/unit/util/getReplayEvent.test.ts b/packages/replay/test/unit/util/getReplayEvent.test.ts deleted file mode 100644 index a7d18971c20e..000000000000 --- a/packages/replay/test/unit/util/getReplayEvent.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -import type { Hub, Scope } from '@sentry/core'; -import { getCurrentHub } from '@sentry/core'; -import type { Client, ReplayEvent } from '@sentry/types'; - -import { REPLAY_EVENT_NAME } from '../../../src/constants'; -import { prepareReplayEvent } from '../../../src/util/prepareReplayEvent'; -import { getDefaultClientOptions, TestClient } from '../../utils/TestClient'; - -describe('Unit | util | getReplayEvent', () => { - let hub: Hub; - let client: Client; - let scope: Scope; - - beforeEach(() => { - hub = getCurrentHub(); - client = new TestClient(getDefaultClientOptions()); - hub.bindClient(client); - - client = hub.getClient()!; - scope = hub.getScope()!; - }); - - it('works', async () => { - expect(client).toBeDefined(); - expect(scope).toBeDefined(); - - const replayId = 'replay-ID'; - const event: ReplayEvent = { - // @ts-ignore private api - type: REPLAY_EVENT_NAME, - timestamp: 1670837008.634, - error_ids: ['error-ID'], - trace_ids: ['trace-ID'], - urls: ['https://sentry.io/'], - replay_id: replayId, - replay_type: 'session', - segment_id: 3, - }; - - const replayEvent = await prepareReplayEvent({ scope, client, replayId, event }); - - expect(replayEvent).toEqual({ - type: 'replay_event', - timestamp: 1670837008.634, - error_ids: ['error-ID'], - trace_ids: ['trace-ID'], - urls: ['https://sentry.io/'], - replay_id: 'replay-ID', - replay_type: 'session', - segment_id: 3, - platform: 'javascript', - event_id: 'replay-ID', - environment: 'production', - sdk: { - name: 'sentry.javascript.unknown', - version: 'version:Test', - }, - sdkProcessingMetadata: {}, - breadcrumbs: undefined, - }); - }); -});