Skip to content

Commit c2b1bfd

Browse files
authored
fix(replay): Handle errors in beforeAddRecordingEvent callback (#8548)
When an error occurs in `beforeAddRecordingEvent`, we just skip this event and log the error. Closes #8542
1 parent 69a4fa3 commit c2b1bfd

File tree

2 files changed

+63
-12
lines changed

2 files changed

+63
-12
lines changed

packages/replay/src/util/addEvent.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { getCurrentHub } from '@sentry/core';
33
import { logger } from '@sentry/utils';
44

55
import { EventBufferSizeExceededError } from '../eventBuffer/error';
6-
import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent } from '../types';
6+
import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent, ReplayPluginOptions } from '../types';
77
import { timestampToMs } from './timestampToMs';
88

99
function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent {
@@ -46,10 +46,7 @@ export async function addEvent(
4646

4747
const replayOptions = replay.getOptions();
4848

49-
const eventAfterPossibleCallback =
50-
typeof replayOptions.beforeAddRecordingEvent === 'function' && isCustomEvent(event)
51-
? replayOptions.beforeAddRecordingEvent(event)
52-
: event;
49+
const eventAfterPossibleCallback = maybeApplyCallback(event, replayOptions.beforeAddRecordingEvent);
5350

5451
if (!eventAfterPossibleCallback) {
5552
return;
@@ -69,3 +66,20 @@ export async function addEvent(
6966
}
7067
}
7168
}
69+
70+
function maybeApplyCallback(
71+
event: RecordingEvent,
72+
callback: ReplayPluginOptions['beforeAddRecordingEvent'],
73+
): RecordingEvent | null | undefined {
74+
try {
75+
if (typeof callback === 'function' && isCustomEvent(event)) {
76+
return callback(event);
77+
}
78+
} catch (error) {
79+
__DEBUG_BUILD__ &&
80+
logger.error('[Replay] An error occured in the `beforeAddRecordingEvent` callback, skipping the event...', error);
81+
return null;
82+
}
83+
84+
return event;
85+
}

packages/replay/test/integration/beforeAddRecordingEvent.test.ts

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import * as SentryUtils from '@sentry/utils';
55
import type { Replay } from '../../src';
66
import type { ReplayContainer } from '../../src/replay';
77
import { clearSession } from '../../src/session/clearSession';
8-
import type { EventType } from '../../src/types';
8+
import { createPerformanceEntries } from '../../src/util/createPerformanceEntries';
9+
import { createPerformanceSpans } from '../../src/util/createPerformanceSpans';
910
import * as SendReplayRequest from '../../src/util/sendReplayRequest';
1011
import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index';
1112
import { useFakeTimers } from '../utils/use-fake-timers';
@@ -40,6 +41,10 @@ describe('Integration | beforeAddRecordingEvent', () => {
4041
beforeAddRecordingEvent: event => {
4142
const eventData = event.data;
4243

44+
if (eventData.tag === 'performanceSpan') {
45+
throw new Error('test error in callback');
46+
}
47+
4348
if (eventData.tag === 'breadcrumb' && eventData.payload.category === 'ui.click') {
4449
return {
4550
...event,
@@ -53,12 +58,6 @@ describe('Integration | beforeAddRecordingEvent', () => {
5358
};
5459
}
5560

56-
// This should not do anything because callback should not be called
57-
// for `event.type != 5` - but we guard anyhow to be safe
58-
if ((event.type as EventType) === 2) {
59-
return null;
60-
}
61-
6261
if (eventData.tag === 'options') {
6362
return null;
6463
}
@@ -143,4 +142,42 @@ describe('Integration | beforeAddRecordingEvent', () => {
143142
recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }]),
144143
});
145144
});
145+
146+
it('handles error in callback', async () => {
147+
createPerformanceSpans(
148+
replay,
149+
createPerformanceEntries([
150+
{
151+
name: 'https://sentry.io/foo.js',
152+
entryType: 'resource',
153+
startTime: 176.59999990463257,
154+
duration: 5.600000023841858,
155+
initiatorType: 'link',
156+
nextHopProtocol: 'h2',
157+
workerStart: 177.5,
158+
redirectStart: 0,
159+
redirectEnd: 0,
160+
fetchStart: 177.69999992847443,
161+
domainLookupStart: 177.69999992847443,
162+
domainLookupEnd: 177.69999992847443,
163+
connectStart: 177.69999992847443,
164+
connectEnd: 177.69999992847443,
165+
secureConnectionStart: 177.69999992847443,
166+
requestStart: 177.5,
167+
responseStart: 181,
168+
responseEnd: 182.19999992847443,
169+
transferSize: 0,
170+
encodedBodySize: 0,
171+
decodedBodySize: 0,
172+
serverTiming: [],
173+
} as unknown as PerformanceResourceTiming,
174+
]),
175+
);
176+
177+
jest.runAllTimers();
178+
await new Promise(process.nextTick);
179+
180+
expect(replay).not.toHaveLastSentReplay();
181+
expect(replay.isEnabled()).toBe(true);
182+
});
146183
});

0 commit comments

Comments
 (0)