Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions packages/replay/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand All @@ -76,6 +76,7 @@ export class Replay implements Integration {
useCompression,
maskAllText,
blockAllMedia,
_experiments,
};

if (typeof sessionSampleRate === 'number') {
Expand Down Expand Up @@ -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;
Expand Down
38 changes: 19 additions & 19 deletions packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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 //
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -827,8 +829,7 @@ export class ReplayContainer implements ReplayContainerInterface {
eventContext,
});
} catch (err) {
__DEBUG_BUILD__ && logger.error(err);
captureInternalException(err);
this.handleException(err);
}
}

Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions packages/replay/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 0 additions & 14 deletions packages/replay/src/util/captureInternalException.ts

This file was deleted.

9 changes: 0 additions & 9 deletions packages/replay/test/mocks/resetSdkMock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export async function resetSdkMock({ replayOptions, sentryOptions }: MockSdkPara
mockRecord: RecordMock;
mockTransportSend: MockTransportSend;
replay: ReplayContainer;
spyCaptureException: jest.SpyInstance;
}> {
let domHandler: DomHandler;

Expand All @@ -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,
Expand All @@ -54,6 +46,5 @@ export async function resetSdkMock({ replayOptions, sentryOptions }: MockSdkPara
mockRecord,
mockTransportSend,
replay,
spyCaptureException,
};
}
4 changes: 1 addition & 3 deletions packages/replay/test/unit/index-errorSampleRate.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
20 changes: 20 additions & 0 deletions packages/replay/test/unit/index-integrationSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
});
});
});
15 changes: 6 additions & 9 deletions packages/replay/test/unit/index.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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>;
Expand All @@ -110,7 +105,7 @@ describe('Replay', () => {
});

beforeEach(async () => {
({ mockRecord, mockTransportSend, domHandler, replay, spyCaptureException } = await resetSdkMock({
({ mockRecord, mockTransportSend, domHandler, replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down