fix(core): Ensure debugIds are applied to all exceptions in an event (#14881)

While investigating
#14841, I noticed
that we had some brittle non-null assertions in our `applyDebugIds`
function which would cause the debug id application logic to terminate
early, in case we'd encounter an `event.exception.values` item without a
stack trace. The added regression test illustrates the scenario in which
debug ids would not have been applied to the second exception prior to
this fix.

Author: Lms24

packages/core/src/utils/prepareEvent.ts

@@ -166,19 +166,13 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void {
   // Build a map of filename -> debug_id
   const filenameDebugIdMap = getFilenameToDebugIdMap(stackParser);
 
-  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) {
-          frame.debug_id = filenameDebugIdMap[frame.filename];
-        }
-      });
+  event.exception?.values?.forEach(exception => {
+    exception.stacktrace?.frames?.forEach(frame => {
+      if (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.
-  }
+  });
 }
 
 /**
@@ -187,24 +181,18 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void {
 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;
+  event.exception?.values?.forEach(exception => {
+    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;

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

@@ -71,6 +71,45 @@ describe('applyDebugIds', () => {
       }),
     );
   });
+
+  it('handles multiple exception values where not all events have valid stack traces', () => {
+    GLOBAL_OBJ._sentryDebugIds = {
+      'filename1.js\nfilename1.js': 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa',
+      'filename2.js\nfilename2.js': 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb',
+    };
+    const stackParser = createStackParser([0, line => ({ filename: line })]);
+
+    const event: Event = {
+      exception: {
+        values: [
+          {
+            value: 'first exception without stack trace',
+          },
+          {
+            stacktrace: {
+              frames: [{ filename: 'filename1.js' }, { filename: 'filename2.js' }],
+            },
+          },
+        ],
+      },
+    };
+
+    applyDebugIds(event, stackParser);
+
+    expect(event.exception?.values?.[0]).toEqual({
+      value: 'first exception without stack trace',
+    });
+
+    expect(event.exception?.values?.[1]?.stacktrace?.frames).toContainEqual({
+      filename: 'filename1.js',
+      debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa',
+    });
+
+    expect(event.exception?.values?.[1]?.stacktrace?.frames).toContainEqual({
+      filename: 'filename2.js',
+      debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb',
+    });
+  });
 });
 
 describe('applyDebugMeta', () => {
@@ -113,6 +152,49 @@ describe('applyDebugMeta', () => {
       debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb',
     });
   });
+
+  it('handles multiple exception values where not all events have valid stack traces', () => {
+    const event: Event = {
+      exception: {
+        values: [
+          {
+            value: 'first exception without stack trace',
+          },
+          {
+            stacktrace: {
+              frames: [
+                { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' },
+                { filename: 'filename2.js', debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb' },
+              ],
+            },
+          },
+        ],
+      },
+    };
+
+    applyDebugMeta(event);
+
+    expect(event.exception?.values?.[0]).toEqual({
+      value: 'first exception without stack trace',
+    });
+
+    expect(event.exception?.values?.[1]?.stacktrace?.frames).toEqual([
+      { filename: 'filename1.js' },
+      { filename: 'filename2.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',
+    });
+  });
 });
 
 describe('parseEventHintOrCaptureContext', () => {