feat(replay): Use new prepareEvent util & ensure dropping replays works (#6522)

Author: mydea

packages/replay/jest.setup.ts

@@ -15,27 +15,6 @@ jest.mock('./src/util/isBrowser', () => {
   };
 });
 
-afterEach(() => {
-  const hub = getCurrentHub();
-  if (typeof hub?.getClient !== 'function') {
-    // Potentially not a function due to partial mocks
-    return;
-  }
-
-  const client = hub?.getClient();
-  // This can be weirded up by mocks/tests
-  if (
-    !client ||
-    !client.getTransport ||
-    typeof client.getTransport !== 'function' ||
-    typeof client.getTransport()?.send !== 'function'
-  ) {
-    return;
-  }
-
-  (client.getTransport()?.send as MockTransport).mockClear();
-});
-
 type EnvelopeHeader = {
   event_id: string;
   sent_at: string;

packages/replay/src/replay.ts

@@ -941,7 +941,9 @@ export class ReplayContainer implements ReplayContainerInterface {
     const replayEvent = await getReplayEvent({ scope, client, replayId, event: baseEvent });
 
     if (!replayEvent) {
-      __DEBUG_BUILD__ && logger.error('[Replay] An event processor returned null, will not send replay.');
+      // Taken from baseclient's `_processEvent` method, where this is handled for errors/transactions
+      client.recordDroppedEvent('event_processor', 'replay_event', baseEvent);
+      __DEBUG_BUILD__ && logger.log('An event processor returned `null`, will not send event.');
       return;
     }
 

packages/replay/src/util/getReplayEvent.ts

@@ -1,4 +1,4 @@
-import { Scope } from '@sentry/core';
+import { prepareEvent, Scope } from '@sentry/core';
 import { Client, ReplayEvent } from '@sentry/types';
 
 export async function getReplayEvent({
@@ -12,21 +12,27 @@ export async function getReplayEvent({
   replayId: string;
   event: ReplayEvent;
 }): Promise<ReplayEvent | null> {
-  // XXX: This event does not trigger `beforeSend` in SDK
-  // @ts-ignore private api
-  const preparedEvent: ReplayEvent | null = await client._prepareEvent(event, { event_id }, scope);
+  const preparedEvent = (await prepareEvent(client.getOptions(), event, { event_id }, scope)) as ReplayEvent | null;
 
-  if (preparedEvent) {
-    // extract the SDK name because `client._prepareEvent` doesn't add it to the event
-    const metadata = client.getOptions() && client.getOptions()._metadata;
-    const { name } = (metadata && metadata.sdk) || {};
-
-    preparedEvent.sdk = {
-      ...preparedEvent.sdk,
-      version: __SENTRY_REPLAY_VERSION__,
-      name,
-    };
+  // If e.g. a global event processor returned null
+  if (!preparedEvent) {
+    return null;
   }
 
+  // This normally happens in browser client "_prepareEvent"
+  // but since we do not use this private method from the client, but rather the plain import
+  // we need to do this manually.
+  preparedEvent.platform = preparedEvent.platform || 'javascript';
+
+  // extract the SDK name because `client._prepareEvent` doesn't add it to the event
+  const metadata = client.getOptions() && client.getOptions()._metadata;
+  const { name } = (metadata && metadata.sdk) || {};
+
+  preparedEvent.sdk = {
+    ...preparedEvent.sdk,
+    version: __SENTRY_REPLAY_VERSION__,
+    name,
+  };
+
   return preparedEvent;
 }

packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts

@@ -17,7 +17,7 @@ export function overwriteRecordDroppedEvent(errorIds: Set<string>): void {
     category: DataCategory,
     event?: Event,
   ): void => {
-    if (event && event.event_id) {
+    if (event && !event.type && event.event_id) {
       errorIds.delete(event.event_id);
     }
 

packages/replay/test/unit/index.test.ts

@@ -1,4 +1,5 @@
-import { getCurrentHub } from '@sentry/core';
+import { getCurrentHub, Hub } from '@sentry/core';
+import { Event, Scope } from '@sentry/types';
 import { EventType } from 'rrweb';
 
 import {
@@ -747,54 +748,6 @@ describe('Replay', () => {
     });
   });
 
-  // TODO: ... this doesn't really test anything anymore since replay event and recording are sent in the same envelope
-  it('does not create replay event if recording upload completely fails', async () => {
-    const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
-    // Suppress console.errors
-    const mockConsole = jest.spyOn(console, 'error').mockImplementation(jest.fn());
-    // fail the first and second requests and pass the third one
-    mockSendReplayRequest.mockImplementationOnce(() => {
-      throw new Error('Something bad happened');
-    });
-    mockRecord._emitter(TEST_EVENT);
-
-    await advanceTimers(5000);
-
-    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
-
-    // Reset console.error mock to minimize the amount of time we are hiding
-    // console messages in case an error happens after
-    mockConsole.mockClear();
-    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
-
-    mockSendReplayRequest.mockImplementationOnce(() => {
-      throw new Error('Something bad happened');
-    });
-    await advanceTimers(5000);
-    expect(replay.sendReplayRequest).toHaveBeenCalledTimes(2);
-
-    // next tick should retry and fail
-    mockConsole.mockClear();
-
-    mockSendReplayRequest.mockImplementationOnce(() => {
-      throw new Error('Something bad happened');
-    });
-    await advanceTimers(10000);
-    expect(replay.sendReplayRequest).toHaveBeenCalledTimes(3);
-
-    mockSendReplayRequest.mockImplementationOnce(() => {
-      throw new Error('Something bad happened');
-    });
-    await advanceTimers(30000);
-    expect(replay.sendReplayRequest).toHaveBeenCalledTimes(4);
-
-    // No activity has occurred, session's last activity should remain the same
-    expect(replay.session?.lastActivity).toBeGreaterThanOrEqual(BASE_TIMESTAMP);
-    expect(replay.session?.segmentId).toBe(1);
-
-    // TODO: Recording should stop and next event should do nothing
-  });
-
   it('has correct timestamps when there events earlier than initial timestamp', async function () {
     replay.clearSession();
     replay.loadSession({ expiry: 0 });
@@ -920,3 +873,77 @@ describe('Replay', () => {
     expect(replay.flush).toHaveBeenCalledTimes(1);
   });
 });
+
+describe('eventProcessors', () => {
+  let hub: Hub;
+  let scope: Scope;
+
+  beforeEach(() => {
+    hub = getCurrentHub();
+    scope = hub.pushScope();
+  });
+
+  afterEach(() => {
+    hub.popScope();
+    jest.resetAllMocks();
+  });
+
+  it('handles event processors properly', async () => {
+    const MUTATED_TIMESTAMP = BASE_TIMESTAMP + 3000;
+
+    const { mockRecord } = await resetSdkMock({
+      replayOptions: {
+        stickySession: false,
+      },
+    });
+
+    const client = hub.getClient()!;
+
+    jest.runAllTimers();
+    const mockTransportSend = jest.spyOn(client.getTransport()!, 'send');
+    mockTransportSend.mockReset();
+
+    const handler1 = jest.fn((event: Event): Event | null => {
+      event.timestamp = MUTATED_TIMESTAMP;
+
+      return event;
+    });
+
+    const handler2 = jest.fn((): Event | null => {
+      return null;
+    });
+
+    scope.addEventProcessor(handler1);
+
+    const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
+
+    mockRecord._emitter(TEST_EVENT);
+    jest.runAllTimers();
+    jest.advanceTimersByTime(1);
+    await new Promise(process.nextTick);
+
+    expect(mockTransportSend).toHaveBeenCalledTimes(1);
+
+    scope.addEventProcessor(handler2);
+
+    const TEST_EVENT2 = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
+
+    mockRecord._emitter(TEST_EVENT2);
+    jest.runAllTimers();
+    jest.advanceTimersByTime(1);
+    await new Promise(process.nextTick);
+
+    expect(mockTransportSend).toHaveBeenCalledTimes(1);
+
+    expect(handler1).toHaveBeenCalledTimes(2);
+    expect(handler2).toHaveBeenCalledTimes(1);
+
+    // This receives an envelope, which is a deeply nested array
+    // We only care about the fact that the timestamp was mutated
+    expect(mockTransportSend).toHaveBeenCalledWith(
+      expect.arrayContaining([
+        expect.arrayContaining([expect.arrayContaining([expect.objectContaining({ timestamp: MUTATED_TIMESTAMP })])]),
+      ]),
+    );
+  });
+});

packages/replay/test/utils/getDefaultBrowserClientOptions.ts

@@ -5,6 +5,7 @@ import { resolvedSyncPromise } from '@sentry/utils';
 export function getDefaultBrowserClientOptions(options: Partial<ClientOptions> = {}): ClientOptions {
   return {
     integrations: [],
+    dsn: 'https://username@domain/123',
     transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})),
     stackParser: () => [],
     ...options,

rollup/plugins/bundlePlugins.js

@@ -108,7 +108,6 @@ export function makeTerserPlugin() {
           '_initStorage',
           '_support',
           // TODO: Get rid of these once we use the SDK to send replay events
-          '_prepareEvent', // replay uses client._prepareEvent
           '_metadata', // replay uses client.getOptions()._metadata
         ],
       },