Skip to content

feat(replay): Improve rrweb error ignoring #7087

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.

Sign up for GitHub

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
Feb 8, 2023
Merged

feat(replay): Improve rrweb error ignoring #7087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions packages/replay/src/coreHandlers/handleGlobalEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand All @@ -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;
}
Expand Down
9 changes: 7 additions & 2 deletions packages/replay/src/util/isRrwebError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
122 changes: 104 additions & 18 deletions packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' }],
Expand All @@ -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) }),
}),
Expand All @@ -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' });

Expand All @@ -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) }),
}),
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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: '<anonymous>',
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: '<anonymous>',
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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -278,6 +364,6 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {
event_id: 'ff1616b1e13744c6964281349aecc82a',
};

expect(handleGlobalEventListener(replay)(errorEvent)).toEqual(errorEvent);
expect(handleGlobalEventListener(replay)(errorEvent, {})).toEqual(errorEvent);
});
});