fix(core): Temporarily store debug IDs in stack frame and only put them into debug_meta before sending (#8347)

Author: Luca Forstner

packages/core/src/utils/prepareEvent.ts

@@ -38,9 +38,9 @@ export function prepareEvent(
   applyClientOptions(prepared, options);
   applyIntegrationsMetadata(prepared, integrations);
 
-  // Only apply debug metadata to error events.
+  // Only put debug IDs onto frames for error events.
   if (event.type === undefined) {
-    applyDebugMetadata(prepared, options.stackParser);
+    applyDebugIds(prepared, options.stackParser);
   }
 
   // If we have scope given to us, use it as the base for further modifications.
@@ -75,6 +75,14 @@ export function prepareEvent(
   }
 
   return result.then(evt => {
+    if (evt) {
+      // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified
+      // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed.
+      // This should not cause any PII issues, since we're only moving data that is already on the event and not adding
+      // any new data
+      applyDebugMeta(evt);
+    }
+
     if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
       return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth);
     }
@@ -121,9 +129,9 @@ function applyClientOptions(event: Event, options: ClientOptions): void {
 const debugIdStackParserCache = new WeakMap<StackParser, Map<string, StackFrame[]>>();
 
 /**
- * Applies debug metadata images to the event in order to apply source maps by looking up their debug ID.
+ * Puts debug IDs into the stack frames of an error event.
  */
-export function applyDebugMetadata(event: Event, stackParser: StackParser): void {
+export function applyDebugIds(event: Event, stackParser: StackParser): void {
   const debugIdMap = GLOBAL_OBJ._sentryDebugIds;
 
   if (!debugIdMap) {
@@ -160,34 +168,60 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void
     return acc;
   }, {});
 
-  // Get a Set of filenames in the stack trace
-  const errorFileNames = new Set<string>();
   try {
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     event!.exception!.values!.forEach(exception => {
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       exception.stacktrace!.frames!.forEach(frame => {
         if (frame.filename) {
-          errorFileNames.add(frame.filename);
+          frame.debug_id = filenameDebugIdMap[frame.filename];
+        }
+      });
+    });
+  } catch (e) {
+    // To save bundle size we're just try catching here instead of checking for the existence of all the different objects.
+  }
+}
+
+/**
+ * Moves debug IDs from the stack frames of an error event into the debug_meta field.
+ */
+export function applyDebugMeta(event: Event): void {
+  // Extract debug IDs and filenames from the stack frames on the event.
+  const filenameDebugIdMap: Record<string, string> = {};
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    event.exception!.values!.forEach(exception => {
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      exception.stacktrace!.frames!.forEach(frame => {
+        if (frame.debug_id) {
+          if (frame.abs_path) {
+            filenameDebugIdMap[frame.abs_path] = frame.debug_id;
+          } else if (frame.filename) {
+            filenameDebugIdMap[frame.filename] = frame.debug_id;
+          }
+          delete frame.debug_id;
         }
       });
     });
   } catch (e) {
     // To save bundle size we're just try catching here instead of checking for the existence of all the different objects.
   }
 
+  if (Object.keys(filenameDebugIdMap).length === 0) {
+    return;
+  }
+
   // Fill debug_meta information
   event.debug_meta = event.debug_meta || {};
   event.debug_meta.images = event.debug_meta.images || [];
   const images = event.debug_meta.images;
-  errorFileNames.forEach(filename => {
-    if (filenameDebugIdMap[filename]) {
-      images.push({
-        type: 'sourcemap',
-        code_file: filename,
-        debug_id: filenameDebugIdMap[filename],
-      });
-    }
+  Object.keys(filenameDebugIdMap).forEach(filename => {
+    images.push({
+      type: 'sourcemap',
+      code_file: filename,
+      debug_id: filenameDebugIdMap[filename],
+    });
   });
 }
 

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

@@ -1,14 +1,14 @@
 import type { Event } from '@sentry/types';
 import { createStackParser, GLOBAL_OBJ } from '@sentry/utils';
 
-import { applyDebugMetadata } from '../../src/utils/prepareEvent';
+import { applyDebugIds, applyDebugMeta } from '../../src/utils/prepareEvent';
 
-describe('applyDebugMetadata', () => {
+describe('applyDebugIds', () => {
   afterEach(() => {
     GLOBAL_OBJ._sentryDebugIds = undefined;
   });
 
-  it('should put debug source map images in debug_meta field', () => {
+  it("should put debug IDs into an event's stack frames", () => {
     GLOBAL_OBJ._sentryDebugIds = {
       'filename1.js\nfilename1.js': 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa',
       'filename2.js\nfilename2.js': 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb',
@@ -34,35 +34,74 @@ describe('applyDebugMetadata', () => {
       },
     };
 
-    applyDebugMetadata(event, stackParser);
+    applyDebugIds(event, stackParser);
 
-    expect(event.debug_meta?.images).toContainEqual({
-      type: 'sourcemap',
-      code_file: 'filename1.js',
+    expect(event.exception?.values?.[0].stacktrace?.frames).toContainEqual({
+      filename: 'filename1.js',
       debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa',
     });
 
-    expect(event.debug_meta?.images).toContainEqual({
-      type: 'sourcemap',
-      code_file: 'filename2.js',
+    expect(event.exception?.values?.[0].stacktrace?.frames).toContainEqual({
+      filename: 'filename2.js',
       debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb',
     });
 
     // expect not to contain an image for the stack frame that doesn't have a corresponding debug id
-    expect(event.debug_meta?.images).not.toContainEqual(
+    expect(event.exception?.values?.[0].stacktrace?.frames).not.toContainEqual(
       expect.objectContaining({
-        type: 'sourcemap',
-        code_file: 'filename3.js',
+        filename3: 'filename3.js',
+        debug_id: expect.any(String),
       }),
     );
 
     // expect not to contain an image for the debug id mapping that isn't contained in the stack trace
-    expect(event.debug_meta?.images).not.toContainEqual(
+    expect(event.exception?.values?.[0].stacktrace?.frames).not.toContainEqual(
       expect.objectContaining({
-        type: 'sourcemap',
-        code_file: 'filename4.js',
+        filename3: 'filename4.js',
         debug_id: 'cccccccc-cccc-4ccc-cccc-cccccccccc',
       }),
     );
   });
 });
+
+describe('applyDebugMeta', () => {
+  it("should move the debug IDs inside an event's stack frame into the debug_meta field", () => {
+    const event: Event = {
+      exception: {
+        values: [
+          {
+            stacktrace: {
+              frames: [
+                { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' },
+                { filename: 'filename2.js', debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb' },
+                { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' },
+                { filename: 'filename3.js' },
+              ],
+            },
+          },
+        ],
+      },
+    };
+
+    applyDebugMeta(event);
+
+    expect(event.exception?.values?.[0].stacktrace?.frames).toEqual([
+      { filename: 'filename1.js' },
+      { filename: 'filename2.js' },
+      { filename: 'filename1.js' },
+      { filename: 'filename3.js' },
+    ]);
+
+    expect(event.debug_meta?.images).toContainEqual({
+      type: 'sourcemap',
+      code_file: 'filename1.js',
+      debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa',
+    });
+
+    expect(event.debug_meta?.images).toContainEqual({
+      type: 'sourcemap',
+      code_file: 'filename2.js',
+      debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb',
+    });
+  });
+});