feat(replay): Allow to opt-in to capture replay exceptions (#6482)

You can define `_experiments: { captureExceptions: true }`.

Author: mydea

packages/replay/src/integration.ts

@@ -5,7 +5,6 @@ import { Integration } from '@sentry/types';
 import { DEFAULT_ERROR_SAMPLE_RATE, DEFAULT_SESSION_SAMPLE_RATE } from './constants';
 import { ReplayContainer } from './replay';
 import type { RecordingOptions, ReplayConfiguration, ReplayPluginOptions } from './types';
-import { captureInternalException } from './util/captureInternalException';
 import { isBrowser } from './util/isBrowser';
 
 const MEDIA_SELECTORS = 'img,image,svg,path,rect,area,video,object,picture,embed,map,audio';
@@ -51,6 +50,7 @@ export class Replay implements Integration {
     maskAllText = true,
     maskAllInputs = true,
     blockAllMedia = true,
+    _experiments = {},
     blockClass = 'sentry-block',
     ignoreClass = 'sentry-ignore',
     maskTextClass = 'sentry-mask',
@@ -76,6 +76,7 @@ export class Replay implements Integration {
       useCompression,
       maskAllText,
       blockAllMedia,
+      _experiments,
     };
 
     if (typeof sessionSampleRate === 'number') {
@@ -118,9 +119,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`,
     }
 
     if (isBrowser() && this._isInitialized) {
-      const error = new Error('Multiple Sentry Session Replay instances are not supported');
-      captureInternalException(error);
-      throw error;
+      throw new Error('Multiple Sentry Session Replay instances are not supported');
     }
 
     this._isInitialized = true;

packages/replay/src/replay.ts

@@ -1,5 +1,5 @@
 /* eslint-disable max-lines */ // TODO: We might want to split this file up
-import { addGlobalEventProcessor, getCurrentHub, Scope, setContext } from '@sentry/core';
+import { addGlobalEventProcessor, captureException, getCurrentHub, Scope, setContext } from '@sentry/core';
 import { Breadcrumb, Client, Event } from '@sentry/types';
 import { addInstrumentationHandler, createEnvelope, logger } from '@sentry/utils';
 import debounce from 'lodash.debounce';
@@ -40,7 +40,6 @@ import type {
 } from './types';
 import { addEvent } from './util/addEvent';
 import { addMemoryEntry } from './util/addMemoryEntry';
-import { captureInternalException } from './util/captureInternalException';
 import { createBreadcrumb } from './util/createBreadcrumb';
 import { createPayload } from './util/createPayload';
 import { createPerformanceSpans } from './util/createPerformanceSpans';
@@ -160,7 +159,7 @@ export class ReplayContainer implements ReplayContainerInterface {
 
     // If there is no session, then something bad has happened - can't continue
     if (!this.session) {
-      captureInternalException(new Error('Invalid session'));
+      this.handleException(new Error('No session found'));
       return;
     }
 
@@ -208,8 +207,7 @@ export class ReplayContainer implements ReplayContainerInterface {
         emit: this.handleRecordingEmit,
       });
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error('[Replay]', err);
-      captureInternalException(err);
+      this.handleException(err);
     }
   }
 
@@ -239,8 +237,7 @@ export class ReplayContainer implements ReplayContainerInterface {
       this.eventBuffer?.destroy();
       this.eventBuffer = null;
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error('[Replay]', err);
-      captureInternalException(err);
+      this.handleException(err);
     }
   }
 
@@ -257,8 +254,7 @@ export class ReplayContainer implements ReplayContainerInterface {
         this._stopRecording = undefined;
       }
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error('[Replay]', err);
-      captureInternalException(err);
+      this.handleException(err);
     }
   }
 
@@ -273,14 +269,22 @@ export class ReplayContainer implements ReplayContainerInterface {
     this.startRecording();
   }
 
+  /** A wrapper to conditionally capture exceptions. */
+  handleException(error: unknown): void {
+    __DEBUG_BUILD__ && logger.error('[Replay]', error);
+
+    if (this.options._experiments && this.options._experiments.captureExceptions) {
+      captureException(error);
+    }
+  }
+
   /** for tests only */
   clearSession(): void {
     try {
       deleteSession();
       this.session = undefined;
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error('[Replay]', err);
-      captureInternalException(err);
+      this.handleException(err);
     }
   }
 
@@ -358,8 +362,7 @@ export class ReplayContainer implements ReplayContainerInterface {
         this._hasInitializedCoreListeners = true;
       }
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error('[Replay]', err);
-      captureInternalException(err);
+      this.handleException(err);
     }
 
     // _performanceObserver //
@@ -387,8 +390,7 @@ export class ReplayContainer implements ReplayContainerInterface {
         this._performanceObserver = null;
       }
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error('[Replay]', err);
-      captureInternalException(err);
+      this.handleException(err);
     }
   }
 
@@ -827,8 +829,7 @@ export class ReplayContainer implements ReplayContainerInterface {
         eventContext,
       });
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error(err);
-      captureInternalException(err);
+      this.handleException(err);
     }
   }
 
@@ -1021,12 +1022,11 @@ export class ReplayContainer implements ReplayContainerInterface {
       this.resetRetries();
       return true;
     } catch (err) {
-      __DEBUG_BUILD__ && logger.error(err);
       // Capture error for every failed replay
       setContext('Replays', {
         _retryCount: this._retryCount,
       });
-      captureInternalException(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

packages/replay/src/types.ts

@@ -105,6 +105,15 @@ export interface ReplayPluginOptions extends SessionOptions {
    * Block all media (e.g. images, svg, video) in recordings.
    */
   blockAllMedia: boolean;
+
+  /**
+   * _experiments allows users to enable experimental or internal features.
+   * We don't consider such features as part of the public API and hence we don't guarantee semver for them.
+   * Experimental features can be added, changed or removed at any time.
+   *
+   * Default: undefined
+   */
+  _experiments?: Partial<{ captureExceptions: boolean }>;
 }
 
 // These are optional for ReplayPluginOptions because the plugin sets default values

packages/replay/src/util/captureInternalException.ts

@@ -10,7 +10,6 @@ export async function resetSdkMock({ replayOptions, sentryOptions }: MockSdkPara
   mockRecord: RecordMock;
   mockTransportSend: MockTransportSend;
   replay: ReplayContainer;
-  spyCaptureException: jest.SpyInstance;
 }> {
   let domHandler: DomHandler;
 
@@ -29,13 +28,6 @@ export async function resetSdkMock({ replayOptions, sentryOptions }: MockSdkPara
   const { mockRrweb } = await import('./mockRrweb');
   const { record: mockRecord } = mockRrweb();
 
-  // Because of `resetModules`, we need to import and add a spy for
-  // `@sentry/core` here before `mockSdk` is called
-  // XXX: This is probably going to make writing future tests difficult and/or
-  // bloat this area of code
-  const SentryCore = await import('@sentry/core');
-  const spyCaptureException = jest.spyOn(SentryCore, 'captureException');
-
   const { replay } = await mockSdk({
     replayOptions,
     sentryOptions,
@@ -54,6 +46,5 @@ export async function resetSdkMock({ replayOptions, sentryOptions }: MockSdkPara
     mockRecord,
     mockTransportSend,
     replay,
-    spyCaptureException,
   };
 }

packages/replay/test/mocks/resetSdkMock.ts

@@ -1,6 +1,4 @@
-jest.unmock('@sentry/browser');
-
-import { captureException } from '@sentry/browser';
+import { captureException } from '@sentry/core';
 
 import { REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants';
 import { addEvent } from '../../src/util/addEvent';

packages/replay/test/unit/index-errorSampleRate.test.ts

@@ -189,4 +189,24 @@ describe('integration settings', () => {
       expect(replay.recordingOptions.maskTextSelector).toBe('*');
     });
   });
+
+  describe('_experiments', () => {
+    it('works with defining _experiments in integration', async () => {
+      const { replay } = await mockSdk({
+        replayOptions: { _experiments: { captureExceptions: true } },
+        sentryOptions: {},
+      });
+
+      expect(replay.options._experiments).toEqual({ captureExceptions: true });
+    });
+
+    it('works without defining _experiments in integration', async () => {
+      const { replay } = await mockSdk({
+        replayOptions: {},
+        sentryOptions: {},
+      });
+
+      expect(replay.options._experiments).toEqual({});
+    });
+  });
 });

packages/replay/test/unit/index-integrationSettings.test.ts

@@ -1,7 +1,3 @@
-jest.mock('./../../src/util/isInternal', () => ({
-  isInternal: jest.fn(() => true),
-}));
-
 import { EventType } from 'rrweb';
 
 import { MAX_SESSION_LIFE, REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants';
@@ -98,7 +94,6 @@ describe('Replay', () => {
   let mockRecord: RecordMock;
   let mockTransportSend: MockTransportSend;
   let domHandler: DomHandler;
-  let spyCaptureException: jest.MockedFunction<any>;
   const prevLocation = WINDOW.location;
 
   type MockSendReplayRequest = jest.MockedFunction<typeof replay.sendReplayRequest>;
@@ -110,7 +105,7 @@ describe('Replay', () => {
   });
 
   beforeEach(async () => {
-    ({ mockRecord, mockTransportSend, domHandler, replay, spyCaptureException } = await resetSdkMock({
+    ({ mockRecord, mockTransportSend, domHandler, replay } = await resetSdkMock({
       replayOptions: {
         stickySession: false,
       },
@@ -631,6 +626,9 @@ describe('Replay', () => {
     // Suppress console.errors
     const mockConsole = jest.spyOn(console, 'error').mockImplementation(jest.fn());
 
+    // Check errors
+    const spyHandleException = jest.spyOn(replay, 'handleException');
+
     expect(replay.session?.segmentId).toBe(0);
 
     // fail the first and second requests and pass the third one
@@ -662,11 +660,10 @@ describe('Replay', () => {
     expect(replay.sendReplayRequest).toHaveBeenCalledTimes(4);
     expect(replay.sendReplay).toHaveBeenCalledTimes(4);
 
-    expect(spyCaptureException).toHaveBeenCalledTimes(5);
     // Retries = 3 (total tries = 4 including initial attempt)
     // + last exception is max retries exceeded
-    expect(spyCaptureException).toHaveBeenCalledTimes(5);
-    expect(spyCaptureException).toHaveBeenLastCalledWith(new Error('Unable to send Replay - max retries exceeded'));
+    expect(spyHandleException).toHaveBeenCalledTimes(5);
+    expect(spyHandleException).toHaveBeenLastCalledWith(new Error('Unable to send Replay - max retries exceeded'));
 
     // No activity has occurred, session's last activity should remain the same
     expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP);