fix: Apply stack frame metadata before event processors (#12799)

Author: Luca Forstner

dev-packages/browser-integration-tests/suites/integrations/moduleMetadata/appliesMetadata/init.js

@@ -0,0 +1,28 @@
+import * as Sentry from '@sentry/browser';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [Sentry.moduleMetadataIntegration()],
+  beforeSend(event) {
+    const moduleMetadataEntries = [];
+
+    if (event.type === undefined) {
+      try {
+        event.exception.values.forEach(value => {
+          value.stacktrace.frames.forEach(frame => {
+            moduleMetadataEntries.push(frame.module_metadata);
+          });
+        });
+      } catch (e) {
+        // noop
+      }
+    }
+
+    event.extra = {
+      ...event.extra,
+      module_metadata_entries: moduleMetadataEntries,
+    };
+
+    return event;
+  },
+});

dev-packages/browser-integration-tests/suites/integrations/moduleMetadata/appliesMetadata/subject.js

@@ -0,0 +1,22 @@
+var _sentryModuleMetadataGlobal =
+  typeof window !== 'undefined'
+    ? window
+    : typeof global !== 'undefined'
+      ? global
+      : typeof self !== 'undefined'
+        ? self
+        : {};
+
+_sentryModuleMetadataGlobal._sentryModuleMetadata = _sentryModuleMetadataGlobal._sentryModuleMetadata || {};
+
+_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign(
+  {},
+  _sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack],
+  {
+    foo: 'bar',
+  },
+);
+
+setTimeout(() => {
+  throw new Error('I am a module metadata Error');
+}, 0);

dev-packages/browser-integration-tests/suites/integrations/moduleMetadata/appliesMetadata/test.ts

@@ -0,0 +1,17 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest('should provide module_metadata on stack frames in beforeSend', async ({ getLocalTestPath, page }) => {
+  // moduleMetadataIntegration is not included in any CDN bundles
+  if (process.env.PW_BUNDLE?.startsWith('bundle')) {
+    sentryTest.skip();
+  }
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const errorEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
+  expect(errorEvent.extra?.['module_metadata_entries']).toEqual([{ foo: 'bar' }]);
+});

dev-packages/browser-integration-tests/suites/integrations/moduleMetadata/appliesMetadataWithRewriteFrames/init.js

@@ -0,0 +1,38 @@
+import * as Sentry from '@sentry/browser';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [
+    Sentry.moduleMetadataIntegration(),
+    Sentry.rewriteFramesIntegration({
+      iteratee: frame => {
+        return {
+          ...frame,
+          filename: 'bloop', // something that should completely mess with module metadata association
+        };
+      },
+    }),
+  ],
+  beforeSend(event) {
+    const moduleMetadataEntries = [];
+
+    if (event.type === undefined) {
+      try {
+        event.exception.values.forEach(value => {
+          value.stacktrace.frames.forEach(frame => {
+            moduleMetadataEntries.push(frame.module_metadata);
+          });
+        });
+      } catch (e) {
+        // noop
+      }
+    }
+
+    event.extra = {
+      ...event.extra,
+      module_metadata_entries: moduleMetadataEntries,
+    };
+
+    return event;
+  },
+});

dev-packages/browser-integration-tests/suites/integrations/moduleMetadata/appliesMetadataWithRewriteFrames/subject.js

@@ -0,0 +1,22 @@
+var _sentryModuleMetadataGlobal =
+  typeof window !== 'undefined'
+    ? window
+    : typeof global !== 'undefined'
+      ? global
+      : typeof self !== 'undefined'
+        ? self
+        : {};
+
+_sentryModuleMetadataGlobal._sentryModuleMetadata = _sentryModuleMetadataGlobal._sentryModuleMetadata || {};
+
+_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign(
+  {},
+  _sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack],
+  {
+    foo: 'baz',
+  },
+);
+
+setTimeout(() => {
+  throw new Error('I am a module metadata Error');
+}, 0);

dev-packages/browser-integration-tests/suites/integrations/moduleMetadata/appliesMetadataWithRewriteFrames/test.ts

@@ -0,0 +1,20 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest(
+  'should provide module_metadata on stack frames in beforeSend even though an event processor (rewriteFramesIntegration) modified the filename',
+  async ({ getLocalTestPath, page }) => {
+    // moduleMetadataIntegration is not included in any CDN bundles
+    if (process.env.PW_BUNDLE?.startsWith('bundle')) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    const errorEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
+    expect(errorEvent?.extra?.['module_metadata_entries']).toEqual([{ foo: 'baz' }]);
+  },
+);

packages/core/src/baseclient.ts

@@ -455,6 +455,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
 
   public on(hook: 'close', callback: () => void): () => void;
 
+  public on(hook: 'applyFrameMetadata', callback: (event: Event) => void): () => void;
+
   /** @inheritdoc */
   public on(hook: string, callback: unknown): () => void {
     // Note that the code below, with nullish coalescing assignment,
@@ -541,6 +543,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /** @inheritdoc */
   public emit(hook: 'close'): void;
 
+  /** @inheritdoc */
+  public emit(hook: 'applyFrameMetadata', event: Event): void;
+
   /** @inheritdoc */
   public emit(hook: string, ...rest: unknown[]): void {
     const callbacks = this._hooks[hook];

packages/core/src/integrations/metadata.ts

@@ -1,14 +1,21 @@
-import type { EventItem, IntegrationFn } from '@sentry/types';
+import type { EventItem } from '@sentry/types';
 import { forEachEnvelopeItem } from '@sentry/utils';
 import { defineIntegration } from '../integration';
 
 import { addMetadataToStackFrames, stripMetadataFromStackFrames } from '../metadata';
 
-const INTEGRATION_NAME = 'ModuleMetadata';
-
-const _moduleMetadataIntegration = (() => {
+/**
+ * Adds module metadata to stack frames.
+ *
+ * Metadata can be injected by the Sentry bundler plugins using the `moduleMetadata` config option.
+ *
+ * When this integration is added, the metadata passed to the bundler plugin is added to the stack frames of all events
+ * under the `module_metadata` property. This can be used to help in tagging or routing of events from different teams
+ * our sources
+ */
+export const moduleMetadataIntegration = defineIntegration(() => {
   return {
-    name: INTEGRATION_NAME,
+    name: 'ModuleMetadata',
     setup(client) {
       // We need to strip metadata from stack frames before sending them to Sentry since these are client side only.
       client.on('beforeEnvelope', envelope => {
@@ -23,23 +30,16 @@ const _moduleMetadataIntegration = (() => {
           }
         });
       });
-    },
 
-    processEvent(event, _hint, client) {
-      const stackParser = client.getOptions().stackParser;
-      addMetadataToStackFrames(stackParser, event);
-      return event;
+      client.on('applyFrameMetadata', event => {
+        // Only apply stack frame metadata to error events
+        if (event.type !== undefined) {
+          return;
+        }
+
+        const stackParser = client.getOptions().stackParser;
+        addMetadataToStackFrames(stackParser, event);
+      });
     },
   };
-}) satisfies IntegrationFn;
-
-/**
- * Adds module metadata to stack frames.
- *
- * Metadata can be injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option.
- *
- * When this integration is added, the metadata passed to the bundler plugin is added to the stack frames of all events
- * under the `module_metadata` property. This can be used to help in tagging or routing of events from different teams
- * our sources
- */
-export const moduleMetadataIntegration = defineIntegration(_moduleMetadataIntegration);
+});

packages/core/src/integrations/third-party-errors-filter.ts

@@ -53,11 +53,19 @@ export const thirdPartyErrorFilterIntegration = defineIntegration((options: Opti
           }
         });
       });
+
+      client.on('applyFrameMetadata', event => {
+        // Only apply stack frame metadata to error events
+        if (event.type !== undefined) {
+          return;
+        }
+
+        const stackParser = client.getOptions().stackParser;
+        addMetadataToStackFrames(stackParser, event);
+      });
     },
-    processEvent(event, _hint, client) {
-      const stackParser = client.getOptions().stackParser;
-      addMetadataToStackFrames(stackParser, event);
 
+    processEvent(event) {
       const frameKeys = getBundleKeysForAllFramesWithFilenames(event);
 
       if (frameKeys) {

packages/core/src/utils/prepareEvent.ts

@@ -60,6 +60,10 @@ export function prepareEvent(
   applyClientOptions(prepared, options);
   applyIntegrationsMetadata(prepared, integrations);
 
+  if (client) {
+    client.emit('applyFrameMetadata', event);
+  }
+
   // Only put debug IDs onto frames for error events.
   if (event.type === undefined) {
     applyDebugIds(prepared, options.stackParser);

packages/core/test/lib/integrations/third-party-errors-filter.test.ts

@@ -1,12 +1,14 @@
 import type { Client, Event } from '@sentry/types';
 import { GLOBAL_OBJ, createStackParser, nodeStackLineParser } from '@sentry/utils';
 import { thirdPartyErrorFilterIntegration } from '../../../src/integrations/third-party-errors-filter';
+import { addMetadataToStackFrames } from '../../../src/metadata';
 
 function clone<T>(data: T): T {
   return JSON.parse(JSON.stringify(data));
 }
 
 const stack = new Error().stack || '';
+const stackParser = createStackParser(nodeStackLineParser());
 
 const eventWithThirdAndFirstPartyFrames: Event = {
   exception: {
@@ -90,11 +92,7 @@ const eventWithOnlyThirdPartyFrames: Event = {
 };
 
 // This only needs the stackParser
-const MOCK_CLIENT = {
-  getOptions: () => ({
-    stackParser: createStackParser(nodeStackLineParser()),
-  }),
-} as unknown as Client;
+const MOCK_CLIENT = {} as unknown as Client;
 
 describe('ThirdPartyErrorFilter', () => {
   beforeEach(() => {
@@ -103,6 +101,10 @@ describe('ThirdPartyErrorFilter', () => {
       '_sentryBundlerPluginAppKey:some-key': true,
       '_sentryBundlerPluginAppKey:some-other-key': true,
     };
+
+    addMetadataToStackFrames(stackParser, eventWithThirdAndFirstPartyFrames);
+    addMetadataToStackFrames(stackParser, eventWithOnlyFirstPartyFrames);
+    addMetadataToStackFrames(stackParser, eventWithOnlyThirdPartyFrames);
   });
 
   describe('drop-error-if-contains-third-party-frames', () => {

packages/core/test/lib/prepareEvent.test.ts

@@ -205,10 +205,13 @@ describe('prepareEvent', () => {
 
     const options = {} as ClientOptions;
     const client = {
+      emit() {
+        // noop
+      },
       getEventProcessors() {
         return [eventProcessor];
       },
-    } as Client;
+    } as unknown as Client;
     const processedEvent = await prepareEvent(
       options,
       event,
@@ -393,10 +396,13 @@ describe('prepareEvent', () => {
 
       const options = {} as ClientOptions;
       const client = {
+        emit() {
+          // noop
+        },
         getEventProcessors() {
           return [] as EventProcessor[];
         },
-      } as Client;
+      } as unknown as Client;
 
       const processedEvent = await prepareEvent(
         options,
@@ -430,10 +436,13 @@ describe('prepareEvent', () => {
 
       const options = {} as ClientOptions;
       const client = {
+        emit() {
+          // noop
+        },
         getEventProcessors() {
           return [] as EventProcessor[];
         },
-      } as Client;
+      } as unknown as Client;
 
       const captureContext = new Scope();
       captureContext.setTags({ foo: 'bar' });
@@ -470,10 +479,13 @@ describe('prepareEvent', () => {
 
       const options = {} as ClientOptions;
       const client = {
+        emit() {
+          // noop
+        },
         getEventProcessors() {
           return [] as EventProcessor[];
         },
-      } as Client;
+      } as unknown as Client;
 
       const captureContextScope = new Scope();
       captureContextScope.setTags({ foo: 'bar' });

packages/node/test/sdk/scope.test.ts

@@ -88,7 +88,11 @@ describe('Unit | Scope', () => {
   it('allows to set & get a client', () => {
     const scope = new Scope();
     expect(scope.getClient()).toBeUndefined();
-    const client = {} as Client;
+    const client = {
+      emit() {
+        // noop
+      },
+    } as unknown as Client;
     scope.setClient(client);
     expect(scope.getClient()).toBe(client);
   });
@@ -108,7 +112,10 @@ describe('Unit | Scope', () => {
         getEventProcessors() {
           return [eventProcessor];
         },
-      } as Client;
+        emit() {
+          // noop
+        },
+      } as unknown as Client;
       const processedEvent = await prepareEvent(
         options,
         event,