From 6df4376f07075c13abfbbda0b1c1e0be21857e45 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Dec 2022 09:17:56 +0100 Subject: [PATCH] feat(replay): Do not capture errors originating from rrweb Unless using `captureExceptions` experiment, we want to ignore these errors. --- .../src/coreHandlers/handleGlobalEvent.ts | 11 +- packages/replay/src/util/isRrwebError.ts | 16 + .../test/unit/index-handleGlobalEvent.test.ts | 351 +++++++++++++----- 3 files changed, 278 insertions(+), 100 deletions(-) create mode 100644 packages/replay/src/util/isRrwebError.ts diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 51eb667d1309..87dc6c790fad 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -1,13 +1,15 @@ import { addBreadcrumb } from '@sentry/core'; import { Event } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; import type { ReplayContainer } from '../types'; +import { isRrwebError } from '../util/isRrwebError'; /** * Returns a listener to be added to `addGlobalEventProcessor(listener)`. */ -export function handleGlobalEventListener(replay: ReplayContainer): (event: Event) => Event { +export function handleGlobalEventListener(replay: ReplayContainer): (event: Event) => Event | null { return (event: Event) => { // Do not apply replayId to the root event if ( @@ -20,6 +22,13 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even return event; } + // Unless `captureExceptions` is enabled, we want to ignore errors coming from rrweb + // As there can be a bunch of stuff going wrong in internals there, that we don't want to bubble up to users + if (isRrwebError(event) && !replay.getOptions()._experiments?.captureExceptions) { + __DEBUG_BUILD__ && logger.log('[Replay] Ignoring error from rrweb internals', event); + return null; + } + // Only tag transactions with replayId if not waiting for an error // @ts-ignore private if (event.type !== 'transaction' || replay.recordingMode === 'session') { diff --git a/packages/replay/src/util/isRrwebError.ts b/packages/replay/src/util/isRrwebError.ts new file mode 100644 index 000000000000..d9b065857062 --- /dev/null +++ b/packages/replay/src/util/isRrwebError.ts @@ -0,0 +1,16 @@ +import { Event } from '@sentry/types'; + +export function isRrwebError(event: Event): boolean { + if (event.type || !event.exception?.values?.length) { + return false; + } + + // Check if any exception originates from rrweb + return event.exception.values.some(exception => { + if (!exception.stacktrace?.frames?.length) { + return false; + } + + return exception.stacktrace.frames.some(frame => frame.filename?.includes('/rrweb/src/')); + }); +} diff --git a/packages/replay/test/unit/index-handleGlobalEvent.test.ts b/packages/replay/test/unit/index-handleGlobalEvent.test.ts index c85a3def2969..0103c1ab2240 100644 --- a/packages/replay/test/unit/index-handleGlobalEvent.test.ts +++ b/packages/replay/test/unit/index-handleGlobalEvent.test.ts @@ -1,4 +1,5 @@ import { getCurrentHub } from '@sentry/core'; +import { Event } from '@sentry/types'; import { REPLAY_EVENT_NAME } from '../../src/constants'; import { handleGlobalEventListener } from '../../src/coreHandlers/handleGlobalEvent'; @@ -12,117 +13,269 @@ import { useFakeTimers } from './../utils/use-fake-timers'; useFakeTimers(); let replay: ReplayContainer; -beforeEach(async () => { - ({ replay } = await resetSdkMock({ - replayOptions: { - stickySession: false, - }, - sentryOptions: { - replaysSessionSampleRate: 0.0, - replaysOnErrorSampleRate: 1.0, - }, - })); -}); +describe('handleGlobalEvent', () => { + beforeEach(async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + autoStart: false, + })); + }); -afterEach(() => { - replay.stop(); -}); + afterEach(() => { + replay.stop(); + }); -it('deletes breadcrumbs from replay events', () => { - const replayEvent = { - type: REPLAY_EVENT_NAME, - breadcrumbs: [{ type: 'fakecrumb' }], - }; + it('deletes breadcrumbs from replay events', () => { + const replayEvent = { + type: REPLAY_EVENT_NAME, + breadcrumbs: [{ type: 'fakecrumb' }], + }; - // @ts-ignore replay event type - expect(handleGlobalEventListener(replay)(replayEvent)).toEqual({ - type: REPLAY_EVENT_NAME, + // @ts-ignore replay event type + expect(handleGlobalEventListener(replay)(replayEvent)).toEqual({ + type: REPLAY_EVENT_NAME, + }); }); -}); -it('does not delete breadcrumbs from error and transaction events', () => { - expect( - handleGlobalEventListener(replay)({ - breadcrumbs: [{ type: 'fakecrumb' }], - }), - ).toEqual( - expect.objectContaining({ - breadcrumbs: [{ type: 'fakecrumb' }], - }), - ); - expect( - handleGlobalEventListener(replay)({ - type: 'transaction', - breadcrumbs: [{ type: 'fakecrumb' }], - }), - ).toEqual( - expect.objectContaining({ - breadcrumbs: [{ type: 'fakecrumb' }], - }), - ); -}); + it('does not delete breadcrumbs from error and transaction events', () => { + expect( + handleGlobalEventListener(replay)({ + breadcrumbs: [{ type: 'fakecrumb' }], + }), + ).toEqual( + expect.objectContaining({ + breadcrumbs: [{ type: 'fakecrumb' }], + }), + ); + expect( + handleGlobalEventListener(replay)({ + type: 'transaction', + breadcrumbs: [{ type: 'fakecrumb' }], + }), + ).toEqual( + expect.objectContaining({ + breadcrumbs: [{ type: 'fakecrumb' }], + }), + ); + }); -it('only tags errors with replay id, adds trace and error id to context for error samples', async () => { - const transaction = Transaction(); - const error = Error(); - // @ts-ignore idc - expect(handleGlobalEventListener(replay)(transaction)).toEqual( - expect.objectContaining({ - tags: expect.not.objectContaining({ replayId: expect.anything() }), - }), - ); - expect(handleGlobalEventListener(replay)(error)).toEqual( - expect.objectContaining({ - tags: expect.objectContaining({ replayId: expect.any(String) }), - }), - ); - - expect(replay.getContext().traceIds).toContain('trace_id'); - expect(replay.getContext().errorIds).toContain('event_id'); - - jest.runAllTimers(); - await new Promise(process.nextTick); // wait for flush - - // Rerverts `recordingMode` to session - expect(replay.recordingMode).toBe('session'); -}); + it('only tags errors with replay id, adds trace and error id to context for error samples', async () => { + const transaction = Transaction(); + const error = Error(); + // @ts-ignore idc + expect(handleGlobalEventListener(replay)(transaction)).toEqual( + expect.objectContaining({ + tags: expect.not.objectContaining({ replayId: expect.anything() }), + }), + ); + expect(handleGlobalEventListener(replay)(error)).toEqual( + expect.objectContaining({ + tags: expect.objectContaining({ replayId: expect.any(String) }), + }), + ); -it('strips out dropped events from errorIds', async () => { - const error1 = Error({ event_id: 'err1' }); - const error2 = Error({ event_id: 'err2' }); - const error3 = Error({ event_id: 'err3' }); + expect(replay.getContext().traceIds).toContain('trace_id'); + expect(replay.getContext().errorIds).toContain('event_id'); - // @ts-ignore private - overwriteRecordDroppedEvent(replay.getContext().errorIds); + jest.runAllTimers(); + await new Promise(process.nextTick); // wait for flush - const client = getCurrentHub().getClient()!; + // Rerverts `recordingMode` to session + expect(replay.recordingMode).toBe('session'); + }); - handleGlobalEventListener(replay)(error1); - handleGlobalEventListener(replay)(error2); - handleGlobalEventListener(replay)(error3); + it('strips out dropped events from errorIds', async () => { + const error1 = Error({ event_id: 'err1' }); + const error2 = Error({ event_id: 'err2' }); + const error3 = Error({ event_id: 'err3' }); - client.recordDroppedEvent('before_send', 'error', { event_id: 'err2' }); + // @ts-ignore private + overwriteRecordDroppedEvent(replay.getContext().errorIds); - // @ts-ignore private - expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err3']); + const client = getCurrentHub().getClient()!; - restoreRecordDroppedEvent(); -}); + handleGlobalEventListener(replay)(error1); + handleGlobalEventListener(replay)(error2); + handleGlobalEventListener(replay)(error3); + + client.recordDroppedEvent('before_send', 'error', { event_id: 'err2' }); + + // @ts-ignore private + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err3']); + + restoreRecordDroppedEvent(); + }); + + it('tags errors and transactions with replay id for session samples', async () => { + ({ replay } = await resetSdkMock({})); + replay.start(); + const transaction = Transaction(); + const error = Error(); + // @ts-ignore idc + expect(handleGlobalEventListener(replay)(transaction)).toEqual( + expect.objectContaining({ + tags: expect.objectContaining({ replayId: expect.any(String) }), + }), + ); + expect(handleGlobalEventListener(replay)(error)).toEqual( + expect.objectContaining({ + tags: expect.objectContaining({ replayId: expect.any(String) }), + }), + ); + }); + + it('does not skip non-rrweb errors', () => { + const errorEvent: Event = { + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'contains')", + stacktrace: { + frames: [ + { + filename: 'http://example.com/..node_modules/packages/replay/build/npm/some-other-file.js', + function: 'MutationBuffer.processMutations', + in_app: true, + lineno: 101, + colno: 23, + }, + { + filename: '', + function: 'Array.forEach', + in_app: true, + }, + ], + }, + mechanism: { + type: 'generic', + handled: true, + }, + }, + ], + }, + level: 'error', + event_id: 'ff1616b1e13744c6964281349aecc82a', + }; + + expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + }); + + it('skips rrweb internal errors', () => { + const errorEvent: Event = { + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'contains')", + stacktrace: { + frames: [ + { + filename: + 'http://example.com/..node_modules/packages/replay/build/npm/esm/node_modules/rrweb/es/rrweb/packages/rrweb/src/record/mutation.js?v=90704e8a', + function: 'MutationBuffer.processMutations', + in_app: true, + lineno: 101, + colno: 23, + }, + { + filename: '', + function: 'Array.forEach', + in_app: true, + }, + ], + }, + mechanism: { + type: 'generic', + handled: true, + }, + }, + ], + }, + level: 'error', + event_id: 'ff1616b1e13744c6964281349aecc82a', + }; + + expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(null); + }); -it('tags errors and transactions with replay id for session samples', async () => { - ({ replay } = await resetSdkMock({})); - replay.start(); - const transaction = Transaction(); - const error = Error(); - // @ts-ignore idc - expect(handleGlobalEventListener(replay)(transaction)).toEqual( - expect.objectContaining({ - tags: expect.objectContaining({ replayId: expect.any(String) }), - }), - ); - expect(handleGlobalEventListener(replay)(error)).toEqual( - expect.objectContaining({ - tags: expect.objectContaining({ replayId: expect.any(String) }), - }), - ); + it('does not skip rrweb internal errors with _experiments.captureExceptions', () => { + const errorEvent: Event = { + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'contains')", + stacktrace: { + frames: [ + { + filename: + 'http://example.com/..node_modules/packages/replay/build/npm/esm/node_modules/rrweb/es/rrweb/packages/rrweb/src/record/mutation.js?v=90704e8a', + function: 'MutationBuffer.processMutations', + in_app: true, + lineno: 101, + colno: 23, + }, + { + filename: '', + function: 'Array.forEach', + in_app: true, + }, + ], + }, + mechanism: { + type: 'generic', + handled: true, + }, + }, + ], + }, + level: 'error', + event_id: 'ff1616b1e13744c6964281349aecc82a', + }; + + replay.getOptions()._experiments = { captureExceptions: true }; + + expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + }); + + it('does not skip non-rrweb errors when no stacktrace exists', () => { + const errorEvent: Event = { + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'contains')", + stacktrace: { + frames: [], + }, + mechanism: { + type: 'generic', + handled: true, + }, + }, + ], + }, + level: 'error', + event_id: 'ff1616b1e13744c6964281349aecc82a', + }; + + expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + }); + + it('does not skip non-rrweb errors when no exception', () => { + const errorEvent: Event = { + exception: undefined, + level: 'error', + event_id: 'ff1616b1e13744c6964281349aecc82a', + }; + + expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + }); });