From c2b1bfdbd7db4ef036333c9750cdf134991b8a61 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 17 Jul 2023 14:04:01 +0200
Subject: [PATCH] fix(replay): Handle errors in `beforeAddRecordingEvent`
 callback (#8548)

When an error occurs in `beforeAddRecordingEvent`, we just skip this
event and log the error.

Closes https://github.com/getsentry/sentry-javascript/issues/8542
---
 packages/replay/src/util/addEvent.ts          | 24 +++++++--
 .../beforeAddRecordingEvent.test.ts           | 51 ++++++++++++++++---
 2 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts
index 9533c1690dd8..fdc755ada91c 100644
--- a/packages/replay/src/util/addEvent.ts
+++ b/packages/replay/src/util/addEvent.ts
@@ -3,7 +3,7 @@ import { getCurrentHub } from '@sentry/core';
 import { logger } from '@sentry/utils';
 
 import { EventBufferSizeExceededError } from '../eventBuffer/error';
-import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent } from '../types';
+import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent, ReplayPluginOptions } from '../types';
 import { timestampToMs } from './timestampToMs';
 
 function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent {
@@ -46,10 +46,7 @@ export async function addEvent(
 
     const replayOptions = replay.getOptions();
 
-    const eventAfterPossibleCallback =
-      typeof replayOptions.beforeAddRecordingEvent === 'function' && isCustomEvent(event)
-        ? replayOptions.beforeAddRecordingEvent(event)
-        : event;
+    const eventAfterPossibleCallback = maybeApplyCallback(event, replayOptions.beforeAddRecordingEvent);
 
     if (!eventAfterPossibleCallback) {
       return;
@@ -69,3 +66,20 @@ export async function addEvent(
     }
   }
 }
+
+function maybeApplyCallback(
+  event: RecordingEvent,
+  callback: ReplayPluginOptions['beforeAddRecordingEvent'],
+): RecordingEvent | null | undefined {
+  try {
+    if (typeof callback === 'function' && isCustomEvent(event)) {
+      return callback(event);
+    }
+  } catch (error) {
+    __DEBUG_BUILD__ &&
+      logger.error('[Replay] An error occured in the `beforeAddRecordingEvent` callback, skipping the event...', error);
+    return null;
+  }
+
+  return event;
+}
diff --git a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts
index af552e6104d8..6bf33f182e66 100644
--- a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts
+++ b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts
@@ -5,7 +5,8 @@ import * as SentryUtils from '@sentry/utils';
 import type { Replay } from '../../src';
 import type { ReplayContainer } from '../../src/replay';
 import { clearSession } from '../../src/session/clearSession';
-import type { EventType } from '../../src/types';
+import { createPerformanceEntries } from '../../src/util/createPerformanceEntries';
+import { createPerformanceSpans } from '../../src/util/createPerformanceSpans';
 import * as SendReplayRequest from '../../src/util/sendReplayRequest';
 import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index';
 import { useFakeTimers } from '../utils/use-fake-timers';
@@ -40,6 +41,10 @@ describe('Integration | beforeAddRecordingEvent', () => {
         beforeAddRecordingEvent: event => {
           const eventData = event.data;
 
+          if (eventData.tag === 'performanceSpan') {
+            throw new Error('test error in callback');
+          }
+
           if (eventData.tag === 'breadcrumb' && eventData.payload.category === 'ui.click') {
             return {
               ...event,
@@ -53,12 +58,6 @@ describe('Integration | beforeAddRecordingEvent', () => {
             };
           }
 
-          // This should not do anything because callback should not be called
-          // for `event.type != 5` - but we guard anyhow to be safe
-          if ((event.type as EventType) === 2) {
-            return null;
-          }
-
           if (eventData.tag === 'options') {
             return null;
           }
@@ -143,4 +142,42 @@ describe('Integration | beforeAddRecordingEvent', () => {
       recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }]),
     });
   });
+
+  it('handles error in callback', async () => {
+    createPerformanceSpans(
+      replay,
+      createPerformanceEntries([
+        {
+          name: 'https://sentry.io/foo.js',
+          entryType: 'resource',
+          startTime: 176.59999990463257,
+          duration: 5.600000023841858,
+          initiatorType: 'link',
+          nextHopProtocol: 'h2',
+          workerStart: 177.5,
+          redirectStart: 0,
+          redirectEnd: 0,
+          fetchStart: 177.69999992847443,
+          domainLookupStart: 177.69999992847443,
+          domainLookupEnd: 177.69999992847443,
+          connectStart: 177.69999992847443,
+          connectEnd: 177.69999992847443,
+          secureConnectionStart: 177.69999992847443,
+          requestStart: 177.5,
+          responseStart: 181,
+          responseEnd: 182.19999992847443,
+          transferSize: 0,
+          encodedBodySize: 0,
+          decodedBodySize: 0,
+          serverTiming: [],
+        } as unknown as PerformanceResourceTiming,
+      ]),
+    );
+
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+
+    expect(replay).not.toHaveLastSentReplay();
+    expect(replay.isEnabled()).toBe(true);
+  });
 });