ref(browser): Refactor browser integrations to use processEvent (#9022

) This refactors browser integrations to use the new `processEvent` hook on the Integrations interface. It also updates Replay to register it's event processor on the client, not globally. This is also needed to ensure the order is somewhat stable, as global processors are run separately from the client ones.
getsentry · Sep 21, 2023 · 94c68e0 · 94c68e0
1 parent 27cf07f
commit 94c68e0
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 90 deletions.
diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts
@@ -1,4 +1,4 @@
-import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types';
+import type { Event, Exception, Integration, StackFrame } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
 /** Deduplication filter */
@@ -22,36 +22,30 @@ export class Dedupe implements Integration {
     this.name = Dedupe.id;
   }
 
+  /** @inheritDoc */
+  public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void {
+    // noop
+  }
+
   /**
    * @inheritDoc
    */
-  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
-    const eventProcessor: EventProcessor = currentEvent => {
-      // We want to ignore any non-error type events, e.g. transactions or replays
-      // These should never be deduped, and also not be compared against as _previousEvent.
-      if (currentEvent.type) {
-        return currentEvent;
-      }
+  public processEvent(currentEvent: Event): Event | null {
+    // We want to ignore any non-error type events, e.g. transactions or replays
+    // These should never be deduped, and also not be compared against as _previousEvent.
+    if (currentEvent.type) {
+      return currentEvent;
+    }
 
-      const self = getCurrentHub().getIntegration(Dedupe);
-      if (self) {
-        // Juuust in case something goes wrong
-        try {
-          if (_shouldDropEvent(currentEvent, self._previousEvent)) {
-            __DEBUG_BUILD__ && logger.warn('Event dropped due to being a duplicate of previously captured event.');
-            return null;
-          }
-        } catch (_oO) {
-          return (self._previousEvent = currentEvent);
-        }
-
-        return (self._previousEvent = currentEvent);
+    // Juuust in case something goes wrong
+    try {
+      if (_shouldDropEvent(currentEvent, this._previousEvent)) {
+        __DEBUG_BUILD__ && logger.warn('Event dropped due to being a duplicate of previously captured event.');
+        return null;
       }
-      return currentEvent;
-    };
+    } catch (_oO) {} // eslint-disable-line no-empty
 
-    eventProcessor.id = this.name;
-    addGlobalEventProcessor(eventProcessor);
+    return (this._previousEvent = currentEvent);
   }
 }
 

diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts
@@ -1,4 +1,3 @@
-import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
 import type { Event, Integration } from '@sentry/types';
 
 import { WINDOW } from '../helpers';
@@ -23,28 +22,28 @@ export class HttpContext implements Integration {
    * @inheritDoc
    */
   public setupOnce(): void {
-    addGlobalEventProcessor((event: Event) => {
-      if (getCurrentHub().getIntegration(HttpContext)) {
-        // if none of the information we want exists, don't bother
-        if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) {
-          return event;
-        }
-
-        // grab as much info as exists and add it to the event
-        const url = (event.request && event.request.url) || (WINDOW.location && WINDOW.location.href);
-        const { referrer } = WINDOW.document || {};
-        const { userAgent } = WINDOW.navigator || {};
-
-        const headers = {
-          ...(event.request && event.request.headers),
-          ...(referrer && { Referer: referrer }),
-          ...(userAgent && { 'User-Agent': userAgent }),
-        };
-        const request = { ...event.request, ...(url && { url }), headers };
-
-        return { ...event, request };
-      }
-      return event;
-    });
+    // noop
+  }
+
+  /** @inheritDoc */
+  public preprocessEvent(event: Event): void {
+    // if none of the information we want exists, don't bother
+    if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) {
+      return;
+    }
+
+    // grab as much info as exists and add it to the event
+    const url = (event.request && event.request.url) || (WINDOW.location && WINDOW.location.href);
+    const { referrer } = WINDOW.document || {};
+    const { userAgent } = WINDOW.navigator || {};
+
+    const headers = {
+      ...(event.request && event.request.headers),
+      ...(referrer && { Referer: referrer }),
+      ...(userAgent && { 'User-Agent': userAgent }),
+    };
+    const request = { ...event.request, ...(url && { url }), headers };
+
+    event.request = request;
   }
 }
diff --git a/packages/browser/src/profiling/integration.ts b/packages/browser/src/profiling/integration.ts
@@ -35,7 +35,7 @@ export class BrowserProfilingIntegration implements Integration {
   /**
    * @inheritDoc
    */
-  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
+  public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
     this.getCurrentHub = getCurrentHub;
     const client = this.getCurrentHub().getClient() as BrowserClient;
 

diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts
@@ -16,51 +16,54 @@ export function handleGlobalEventListener(
 ): (event: Event, hint: EventHint) => Event | null {
   const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined;
 
-  return (event: Event, hint: EventHint) => {
-    // Do nothing if replay has been disabled
-    if (!replay.isEnabled()) {
-      return event;
-    }
+  return Object.assign(
+    (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
-      delete event.breadcrumbs;
-      return event;
-    }
+      if (isReplayEvent(event)) {
+        // Replays have separate set of breadcrumbs, do not include breadcrumbs
+        // from core SDK
+        delete event.breadcrumbs;
+        return event;
+      }
 
-    // We only want to handle errors & transactions, nothing else
-    if (!isErrorEvent(event) && !isTransactionEvent(event)) {
-      return event;
-    }
+      // We only want to handle errors & transactions, nothing else
+      if (!isErrorEvent(event) && !isTransactionEvent(event)) {
+        return event;
+      }
 
-    // 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, hint) && !replay.getOptions()._experiments.captureExceptions) {
-      __DEBUG_BUILD__ && logger.log('[Replay] Ignoring error from rrweb internals', event);
-      return null;
-    }
+      // 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, hint) && !replay.getOptions()._experiments.captureExceptions) {
+        __DEBUG_BUILD__ && logger.log('[Replay] Ignoring error from rrweb internals', event);
+        return null;
+      }
 
-    // When in buffer mode, we decide to sample here.
-    // Later, in `handleAfterSendEvent`, if the replayId is set, we know that we sampled
-    // And convert the buffer session to a full session
-    const isErrorEventSampled = shouldSampleForBufferEvent(replay, event);
+      // When in buffer mode, we decide to sample here.
+      // Later, in `handleAfterSendEvent`, if the replayId is set, we know that we sampled
+      // And convert the buffer session to a full session
+      const isErrorEventSampled = shouldSampleForBufferEvent(replay, event);
 
-    // Tag errors if it has been sampled in buffer mode, or if it is session mode
-    // Only tag transactions if in session mode
-    const shouldTagReplayId = isErrorEventSampled || replay.recordingMode === 'session';
+      // Tag errors if it has been sampled in buffer mode, or if it is session mode
+      // Only tag transactions if in session mode
+      const shouldTagReplayId = isErrorEventSampled || replay.recordingMode === 'session';
 
-    if (shouldTagReplayId) {
-      event.tags = { ...event.tags, replayId: replay.getSessionId() };
-    }
+      if (shouldTagReplayId) {
+        event.tags = { ...event.tags, replayId: replay.getSessionId() };
+      }
 
-    // In cases where a custom client is used that does not support the new hooks (yet),
-    // we manually call this hook method here
-    if (afterSendHandler) {
-      // Pretend the error had a 200 response so we always capture it
-      afterSendHandler(event, { statusCode: 200 });
-    }
+      // In cases where a custom client is used that does not support the new hooks (yet),
+      // we manually call this hook method here
+      if (afterSendHandler) {
+        // Pretend the error had a 200 response so we always capture it
+        afterSendHandler(event, { statusCode: 200 });
+      }
 
-    return event;
-  };
+      return event;
+    },
+    { id: 'Replay' },
+  );
 }
diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts
@@ -26,7 +26,12 @@ export function addGlobalListeners(replay: ReplayContainer): void {
 
   // Tag all (non replay) events that get sent to Sentry with the current
   // replay ID so that we can reference them later in the UI
-  addGlobalEventProcessor(handleGlobalEventListener(replay, !hasHooks(client)));
+  const eventProcessor = handleGlobalEventListener(replay, !hasHooks(client));
+  if (client && client.addEventProcessor) {
+    client.addEventProcessor(eventProcessor);
+  } else {
+    addGlobalEventProcessor(eventProcessor);
+  }
 
   // If a custom client has no hooks yet, we continue to use the "old" implementation
   if (hasHooks(client)) {