From 990cd1e75101deb1b28d0e6f8e2def48fae5bc9c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 8 Feb 2023 10:27:52 +0100 Subject: [PATCH] feat(replay): Improve rrweb error ignoring Based on https://github.com/getsentry/rrweb/pull/41, we can now ignore rrweb errors even when the code is minified. --- .../src/coreHandlers/handleGlobalEvent.ts | 8 +- packages/replay/src/util/isRrwebError.ts | 9 +- .../coreHandlers/handleGlobalEvent.test.ts | 122 +++++++++++++++--- 3 files changed, 115 insertions(+), 24 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 4bdb3e5c1a54..35ecba8c43e3 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -1,5 +1,5 @@ import { addBreadcrumb } from '@sentry/core'; -import type { Event } from '@sentry/types'; +import type { Event, EventHint } from '@sentry/types'; import { logger } from '@sentry/utils'; import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; @@ -9,8 +9,8 @@ import { isRrwebError } from '../util/isRrwebError'; /** * Returns a listener to be added to `addGlobalEventProcessor(listener)`. */ -export function handleGlobalEventListener(replay: ReplayContainer): (event: Event) => Event | null { - return (event: Event) => { +export function handleGlobalEventListener(replay: ReplayContainer): (event: Event, hint: EventHint) => Event | null { + return (event: Event, hint: EventHint) => { // Do not apply replayId to the root event if (event.type === REPLAY_EVENT_NAME) { // Replays have separate set of breadcrumbs, do not include breadcrumbs @@ -21,7 +21,7 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even // 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) { + if (isRrwebError(event, hint) && !replay.getOptions()._experiments.captureExceptions) { __DEBUG_BUILD__ && logger.log('[Replay] Ignoring error from rrweb internals', event); return null; } diff --git a/packages/replay/src/util/isRrwebError.ts b/packages/replay/src/util/isRrwebError.ts index 24f612185b87..a8d097e73af9 100644 --- a/packages/replay/src/util/isRrwebError.ts +++ b/packages/replay/src/util/isRrwebError.ts @@ -1,13 +1,18 @@ -import type { Event } from '@sentry/types'; +import type { Event, EventHint } from '@sentry/types'; /** * Returns true if we think the given event is an error originating inside of rrweb. */ -export function isRrwebError(event: Event): boolean { +export function isRrwebError(event: Event, hint: EventHint): boolean { if (event.type || !event.exception || !event.exception.values || !event.exception.values.length) { return false; } + // @ts-ignore this may be set by rrweb when it finds errors + if (hint.originalException && hint.originalException.__rrweb__) { + return true; + } + // Check if any exception originates from rrweb return event.exception.values.some(exception => { if (!exception.stacktrace || !exception.stacktrace.frames || !exception.stacktrace.frames.length) { diff --git a/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts index a59dfe838051..1bce8c11572a 100644 --- a/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts @@ -40,26 +40,32 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { }; // @ts-ignore replay event type - expect(handleGlobalEventListener(replay)(replayEvent)).toEqual({ + 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' }], - }), + handleGlobalEventListener(replay)( + { + breadcrumbs: [{ type: 'fakecrumb' }], + }, + {}, + ), ).toEqual( expect.objectContaining({ breadcrumbs: [{ type: 'fakecrumb' }], }), ); expect( - handleGlobalEventListener(replay)({ - type: 'transaction', - breadcrumbs: [{ type: 'fakecrumb' }], - }), + handleGlobalEventListener(replay)( + { + type: 'transaction', + breadcrumbs: [{ type: 'fakecrumb' }], + }, + {}, + ), ).toEqual( expect.objectContaining({ breadcrumbs: [{ type: 'fakecrumb' }], @@ -76,7 +82,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { tags: expect.not.objectContaining({ replayId: expect.anything() }), }), ); - expect(handleGlobalEventListener(replay)(error)).toEqual( + expect(handleGlobalEventListener(replay)(error, {})).toEqual( expect.objectContaining({ tags: expect.objectContaining({ replayId: expect.any(String) }), }), @@ -102,9 +108,9 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { const client = getCurrentHub().getClient()!; - handleGlobalEventListener(replay)(error1); - handleGlobalEventListener(replay)(error2); - handleGlobalEventListener(replay)(error3); + handleGlobalEventListener(replay)(error1, {}); + handleGlobalEventListener(replay)(error2, {}); + handleGlobalEventListener(replay)(error3, {}); client.recordDroppedEvent('before_send', 'error', { event_id: 'err2' }); @@ -125,7 +131,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { tags: expect.objectContaining({ replayId: expect.any(String) }), }), ); - expect(handleGlobalEventListener(replay)(error)).toEqual( + expect(handleGlobalEventListener(replay)(error, {})).toEqual( expect.objectContaining({ tags: expect.objectContaining({ replayId: expect.any(String) }), }), @@ -166,7 +172,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { event_id: 'ff1616b1e13744c6964281349aecc82a', }; - expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + expect(handleGlobalEventListener(replay)(errorEvent, {})).toEqual(errorEvent); }); it('skips rrweb internal errors', () => { @@ -204,7 +210,87 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { event_id: 'ff1616b1e13744c6964281349aecc82a', }; - expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(null); + expect(handleGlobalEventListener(replay)(errorEvent, {})).toEqual(null); + }); + + it('skips exception with __rrweb__ set', () => { + const errorEvent: Event = { + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'contains')", + stacktrace: { + frames: [ + { + filename: 'scrambled.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', + }; + + const originalException = new window.Error('some exception'); + // @ts-ignore this could be set by rrweb + originalException.__rrweb__ = true; + + expect(handleGlobalEventListener(replay)(errorEvent, { originalException })).toEqual(null); + }); + + it('handles string exceptions', () => { + const errorEvent: Event = { + exception: { + values: [ + { + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'contains')", + stacktrace: { + frames: [ + { + filename: 'scrambled.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', + }; + + const originalException = 'some string exception'; + + expect(handleGlobalEventListener(replay)(errorEvent, { originalException })).toEqual(errorEvent); }); it('does not skip rrweb internal errors with _experiments.captureExceptions', () => { @@ -244,7 +330,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { replay.getOptions()._experiments = { captureExceptions: true }; - expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + expect(handleGlobalEventListener(replay)(errorEvent, {})).toEqual(errorEvent); }); it('does not skip non-rrweb errors when no stacktrace exists', () => { @@ -268,7 +354,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { event_id: 'ff1616b1e13744c6964281349aecc82a', }; - expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + expect(handleGlobalEventListener(replay)(errorEvent, {})).toEqual(errorEvent); }); it('does not skip non-rrweb errors when no exception', () => { @@ -278,6 +364,6 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { event_id: 'ff1616b1e13744c6964281349aecc82a', }; - expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent); + expect(handleGlobalEventListener(replay)(errorEvent, {})).toEqual(errorEvent); }); });