fix(replay): Improve capture of errorIds/traceIds

packages/replay/src/coreHandlers/handleAfterSendEvent.ts

@@ -1,5 +1,5 @@
 import { getCurrentHub } from '@sentry/core';
-import type { Event, Transport, TransportMakeRequestResponse } from '@sentry/types';
+import type { ErrorEvent, Event, TransactionEvent, Transport, TransportMakeRequestResponse } from '@sentry/types';
 
 import type { ReplayContainer } from '../types';
 import { isErrorEvent, isTransactionEvent } from '../util/eventUtils';
@@ -15,7 +15,7 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal
   const enforceStatusCode = isBaseTransportSend();
 
   return (event: Event, sendResponse: TransportMakeRequestResponse | void) => {
-    if (!isErrorEvent(event) && !isTransactionEvent(event)) {
+    if (!replay.isEnabled() || (!isErrorEvent(event) && !isTransactionEvent(event))) {
       return;
     }
 
@@ -28,36 +28,47 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal
       return;
     }
 
-    // Collect traceIds in _context regardless of `recordingMode`
-    // In error mode, _context gets cleared on every checkout
-    if (isTransactionEvent(event) && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) {
-      replay.getContext().traceIds.add(event.contexts.trace.trace_id as string);
+    if (isTransactionEvent(event)) {
+      handleTransactionEvent(replay, event);
       return;
     }
 
-    // Everything below is just for error events
-    if (!isErrorEvent(event)) {
-      return;
-    }
+    handleErrorEvent(replay, event);
+  };
+}
 
-    // Add error to list of errorIds of replay. This is ok to do even if not
-    // sampled because context will get reset at next checkout.
-    // XXX: There is also a race condition where it's possible to capture an
-    // error to Sentry before Replay SDK has loaded, but response returns after
-    // it was loaded, and this gets called.
-    if (event.event_id) {
-      replay.getContext().errorIds.add(event.event_id);
-    }
+function handleTransactionEvent(replay: ReplayContainer, event: TransactionEvent): void {
+  const replayContext = replay.getContext();
 
-    // If error event is tagged with replay id it means it was sampled (when in buffer mode)
-    // Need to be very careful that this does not cause an infinite loop
-    if (replay.recordingMode === 'buffer' && event.tags && event.tags.replayId) {
-      setTimeout(() => {
-        // Capture current event buffer as new replay
-        void replay.sendBufferedReplayOrFlush();
-      });
-    }
-  };
+  // Collect traceIds in _context regardless of `recordingMode`
+  // In error mode, _context gets cleared on every checkout
+  // We limit to max. 100 transactions linked
+  if (event.contexts && event.contexts.trace && event.contexts.trace.trace_id && replayContext.traceIds.size < 100) {
+    replayContext.traceIds.add(event.contexts.trace.trace_id as string);
+  }
+}
+
+function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void {
+  const replayContext = replay.getContext();
+
+  // Add error to list of errorIds of replay. This is ok to do even if not
+  // sampled because context will get reset at next checkout.
+  // XXX: There is also a race condition where it's possible to capture an
+  // error to Sentry before Replay SDK has loaded, but response returns after
+  // it was loaded, and this gets called.
+  // We limit to max. 100 errors linked
+  if (event.event_id && replayContext.errorIds.size < 100) {
+    replayContext.errorIds.add(event.event_id);
+  }
+
+  // If error event is tagged with replay id it means it was sampled (when in buffer mode)
+  // Need to be very careful that this does not cause an infinite loop
+  if (replay.recordingMode === 'buffer' && event.tags && event.tags.replayId) {
+    setTimeout(() => {
+      // Capture current event buffer as new replay
+      void replay.sendBufferedReplayOrFlush();
+    });
+  }
 }
 
 function isBaseTransportSend(): boolean {

packages/replay/src/coreHandlers/handleGlobalEvent.ts

@@ -17,6 +17,11 @@ export function handleGlobalEventListener(
   const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined;
 
   return (event: Event, hint: EventHint) => {
+    // Do nothing if replay has been disabled
+    if (!replay.isEnabled()) {
+      return event;
+    }
+
     if (isReplayEvent(event)) {
       // Replays have separate set of breadcrumbs, do not include breadcrumbs
       // from core SDK

packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts

@@ -89,6 +89,58 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     expect(replay.recordingMode).toBe('buffer');
   });
 
+  it('limits errorIds to max. 100', async () => {
+    ({ replay } = await resetSdkMock({
+      replayOptions: {
+        stickySession: false,
+      },
+      sentryOptions: {
+        replaysSessionSampleRate: 1.0,
+        replaysOnErrorSampleRate: 0.0,
+      },
+    }));
+
+    const handler = handleAfterSendEvent(replay);
+
+    for (let i = 0; i < 150; i++) {
+      const error = Error({ event_id: `err-${i}` });
+      handler(error, { statusCode: 200 });
+    }
+
+    expect(Array.from(replay.getContext().errorIds)).toEqual(
+      Array(100)
+        .fill(undefined)
+        .map((_, i) => `err-${i}`),
+    );
+    expect(Array.from(replay.getContext().traceIds)).toEqual([]);
+  });
+
+  it('limits traceIds to max. 100', async () => {
+    ({ replay } = await resetSdkMock({
+      replayOptions: {
+        stickySession: false,
+      },
+      sentryOptions: {
+        replaysSessionSampleRate: 0.0,
+        replaysOnErrorSampleRate: 1.0,
+      },
+    }));
+
+    const handler = handleAfterSendEvent(replay);
+
+    for (let i = 0; i < 150; i++) {
+      const transaction = Transaction(`tr-${i}`);
+      handler(transaction, { statusCode: 200 });
+    }
+
+    expect(Array.from(replay.getContext().errorIds)).toEqual([]);
+    expect(Array.from(replay.getContext().traceIds)).toEqual(
+      Array(100)
+        .fill(undefined)
+        .map((_, i) => `tr-${i}`),
+    );
+  });
+
   it('allows undefined send response when using custom transport', async () => {
     ({ replay } = await resetSdkMock({
       replayOptions: {
@@ -123,7 +175,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err2', 'err3', 'err4']);
   });
 
-  it('flushes when in error mode', async () => {
+  it('flushes when in buffer mode', async () => {
     ({ replay } = await resetSdkMock({
       replayOptions: {
         stickySession: false,
@@ -231,7 +283,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     expect(replay.recordingMode).toBe('buffer');
   });
 
-  it('does not flush in error mode when failing to send the error', async () => {
+  it('does not flush in buffer mode when failing to send the error', async () => {
     ({ replay } = await resetSdkMock({
       replayOptions: {
         stickySession: false,
@@ -257,7 +309,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     jest.runAllTimers();
     await new Promise(process.nextTick);
 
-    // Remains in error mode & without flushing
+    // Remains in buffer mode & without flushing
     expect(mockSend).toHaveBeenCalledTimes(0);
     expect(Array.from(replay.getContext().errorIds)).toEqual([]);
     expect(replay.isEnabled()).toBe(true);
@@ -291,7 +343,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     jest.runAllTimers();
     await new Promise(process.nextTick);
 
-    // Remains in error mode & without flushing
+    // Remains in buffer mode & without flushing
     expect(mockSend).toHaveBeenCalledTimes(0);
     expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']);
     expect(replay.isEnabled()).toBe(true);
@@ -325,11 +377,38 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
     jest.runAllTimers();
     await new Promise(process.nextTick);
 
-    // Remains in error mode & without flushing
+    // Remains in buffer mode & without flushing
     expect(mockSend).toHaveBeenCalledTimes(0);
     expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']);
     expect(replay.isEnabled()).toBe(true);
     expect(replay.isPaused()).toBe(false);
     expect(replay.recordingMode).toBe('buffer');
   });
+
+  it('does not flush if replay is not enabled anymore', async () => {
+    ({ replay } = await resetSdkMock({
+      replayOptions: {
+        stickySession: false,
+      },
+      sentryOptions: {
+        replaysSessionSampleRate: 0.0,
+        replaysOnErrorSampleRate: 1.0,
+      },
+    }));
+
+    const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>;
+
+    const error1 = Error({ event_id: 'err1', tags: { replayId: 'replayid1' } });
+
+    const handler = handleAfterSendEvent(replay);
+
+    handler(error1, { statusCode: 200 });
+
+    replay['_isEnabled'] = false;
+
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+
+    expect(mockSend).toHaveBeenCalledTimes(0);
+  });
 });

packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts

@@ -84,6 +84,24 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {
     );
   });
 
+  it('does not add replayId if replay is not enabled', async () => {
+    const transaction = Transaction();
+    const error = Error();
+
+    replay['_isEnabled'] = false;
+
+    expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
+      expect.objectContaining({
+        tags: expect.not.objectContaining({ replayId: expect.anything() }),
+      }),
+    );
+    expect(handleGlobalEventListener(replay)(error, {})).toEqual(
+      expect.objectContaining({
+        tags: expect.not.objectContaining({ replayId: expect.anything() }),
+      }),
+    );
+  });
+
   it('tags errors and transactions with replay id for session samples', async () => {
     let integration: ReplayIntegration;
     ({ replay, integration } = await resetSdkMock({}));