feat: Remove usage of deprecated event.stacktrace (#4885)

Removes usage of deprecated `event.stacktrace` and updated the event interface to remove the deprecated property.
getsentry · Apr 13, 2022 · 744252a · 744252a
1 parent a93ed58
commit 744252a
Show file tree

Hide file tree

Showing 13 changed files with 94 additions and 117 deletions.
diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts
@@ -74,7 +74,8 @@ export function eventFromPlainObject(
   if (syntheticException) {
     const frames = parseStackFrames(syntheticException);
     if (frames.length) {
-      event.stacktrace = { frames };
+      // event.exception.values[0] has been set above
+      (event.exception as { values: Exception[] }).values[0].stacktrace = { frames };
     }
   }
 
@@ -273,7 +274,9 @@ export function eventFromString(input: string, syntheticException?: Error, attac
   if (attachStacktrace && syntheticException) {
     const frames = parseStackFrames(syntheticException);
     if (frames.length) {
-      event.stacktrace = { frames };
+      event.exception = {
+        values: [{ value: input, stacktrace: { frames } }],
+      };
     }
   }
 

diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts
@@ -198,8 +198,6 @@ function _getFramesFromEvent(event: Event): StackFrame[] | undefined {
     } catch (_oO) {
       return undefined;
     }
-  } else if (event.stacktrace) {
-    return event.stacktrace.frames;
   }
   return undefined;
 }
diff --git a/packages/browser/test/integration/suites/api.js b/packages/browser/test/integration/suites/api.js
@@ -65,8 +65,8 @@ describe('API', function () {
     return runInSandbox(sandbox, function () {
       throwNonError();
     }).then(function (summary) {
-      assert.isAtLeast(summary.events[0].stacktrace.frames.length, 1);
-      assert.isAtMost(summary.events[0].stacktrace.frames.length, 3);
+      assert.isAtLeast(summary.events[0].exception.values[0].stacktrace.frames.length, 1);
+      assert.isAtMost(summary.events[0].exception.values[0].stacktrace.frames.length, 3);
     });
   });
 

diff --git a/packages/browser/test/integration/suites/breadcrumbs.js b/packages/browser/test/integration/suites/breadcrumbs.js
@@ -234,8 +234,6 @@ describe('breadcrumbs', function () {
         assert.equal(summary.breadcrumbHints.length, 1);
         assert.equal(summary.breadcrumbHints[0].name, 'click');
         assert.equal(summary.breadcrumbHints[0].event.target.tagName, 'INPUT');
-        // There should be no expection, if there is one it means we threw it
-        assert.isUndefined(summary.events[0].exception);
       }
     });
   });
@@ -265,9 +263,6 @@ describe('breadcrumbs', function () {
 
         assert.equal(summary.breadcrumbs[1].category, 'ui.input');
         assert.equal(summary.breadcrumbs[1].message, 'body > form#foo-form > input[name="foo"]');
-
-        // There should be no expection, if there is one it means we threw it
-        assert.isUndefined(summary.events[0].exception);
       }
     });
   });
@@ -288,8 +283,6 @@ describe('breadcrumbs', function () {
         // The async loader doesn't wrap event listeners, but we should receive the event without breadcrumbs
         assert.lengthOf(summary.events, 1);
       } else {
-        // There should be no expection, if there is one it means we threw it
-        assert.isUndefined(summary.events[0].exception);
         assert.equal(summary.breadcrumbs.length, 0);
       }
     });
@@ -309,8 +302,6 @@ describe('breadcrumbs', function () {
         // The async loader doesn't wrap event listeners, but we should receive the event without breadcrumbs
         assert.lengthOf(summary.events, 1);
       } else {
-        // There should be no expection, if there is one it means we threw it
-        assert.isUndefined(summary.events[0].exception);
         assert.equal(summary.breadcrumbs.length, 0);
       }
     });
@@ -472,7 +463,6 @@ describe('breadcrumbs', function () {
         assert.equal(summary.breadcrumbs[0].message, 'body > form#foo-form > input[name="foo"]');
         assert.equal(summary.breadcrumbHints[0].global, false);
         assert.equal(summary.breadcrumbHints[1].global, false);
-        assert.isUndefined(summary.events[0].exception);
       }
     });
   });
@@ -507,7 +497,6 @@ describe('breadcrumbs', function () {
         assert.equal(summary.breadcrumbs[0].message, 'body > form#foo-form > input[name="foo"]');
         assert.equal(summary.breadcrumbHints[0].global, false);
         assert.equal(summary.breadcrumbHints[1].global, false);
-        assert.isUndefined(summary.events[0].exception);
       }
     });
   });
@@ -538,7 +527,6 @@ describe('breadcrumbs', function () {
         assert.equal(summary.breadcrumbs[1].message, 'body > form#foo-form > div.contenteditable');
         assert.equal(summary.breadcrumbHints[0].global, false);
         assert.equal(summary.breadcrumbHints[1].global, false);
-        assert.isUndefined(summary.events[0].exception);
       }
     });
   });
@@ -706,7 +694,6 @@ describe('breadcrumbs', function () {
         assert.equal(summary.breadcrumbs.length, 2);
         assert.equal(summary.breadcrumbHints[0].global, true);
         assert.equal(summary.breadcrumbHints[1].global, true);
-        assert.isUndefined(summary.events[0].exception);
       }
     });
   });

diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts
@@ -170,9 +170,6 @@ function _getLastValidUrl(frames: StackFrame[] = []): string | null {
 
 function _getEventFilterUrl(event: Event): string | null {
   try {
-    if (event.stacktrace) {
-      return _getLastValidUrl(event.stacktrace.frames);
-    }
     let frames;
     try {
       // @ts-ignore we only care about frames if the whole thing here is defined

diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts
@@ -1,4 +1,4 @@
-import { EventProcessor } from '@sentry/types';
+import { Event, EventProcessor } from '@sentry/types';
 
 import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations/inboundfilters';
 
@@ -52,50 +52,68 @@ function createInboundFiltersEventProcessor(
 
 // Fixtures
 
-const MESSAGE_EVENT = {
+const MESSAGE_EVENT: Event = {
   message: 'captureMessage',
 };
 
-const MESSAGE_EVENT_2 = {
+const MESSAGE_EVENT_2: Event = {
   message: 'captureMessageSomething',
 };
 
-const MESSAGE_EVENT_WITH_STACKTRACE = {
+const MESSAGE_EVENT_WITH_STACKTRACE: Event = {
   message: 'wat',
-  stacktrace: {
-    // Frames are always in the reverse order, as this is how Sentry expect them to come.
-    // Frame that crashed is the last one, the one from awesome-analytics
-    frames: [
-      { filename: 'https://our-side.com/js/bundle.js' },
-      { filename: 'https://our-side.com/js/bundle.js' },
-      { filename: 'https://awesome-analytics.io/some/file.js' },
+  exception: {
+    values: [
+      {
+        stacktrace: {
+          // Frames are always in the reverse order, as this is how Sentry expect them to come.
+          // Frame that crashed is the last one, the one from awesome-analytics
+          frames: [
+            { filename: 'https://our-side.com/js/bundle.js' },
+            { filename: 'https://our-side.com/js/bundle.js' },
+            { filename: 'https://awesome-analytics.io/some/file.js' },
+          ],
+        },
+      },
     ],
   },
 };
 
-const MESSAGE_EVENT_WITH_ANON_LAST_FRAME = {
+const MESSAGE_EVENT_WITH_ANON_LAST_FRAME: Event = {
   message: 'any',
-  stacktrace: {
-    frames: [
-      { filename: 'https://our-side.com/js/bundle.js' },
-      { filename: 'https://awesome-analytics.io/some/file.js' },
-      { filename: '<anonymous>' },
+  exception: {
+    values: [
+      {
+        stacktrace: {
+          frames: [
+            { filename: 'https://our-side.com/js/bundle.js' },
+            { filename: 'https://awesome-analytics.io/some/file.js' },
+            { filename: '<anonymous>' },
+          ],
+        },
+      },
     ],
   },
 };
 
-const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME = {
+const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME: Event = {
   message: 'any',
-  stacktrace: {
-    frames: [
-      { filename: 'https://our-side.com/js/bundle.js' },
-      { filename: 'https://awesome-analytics.io/some/file.js' },
-      { filename: '[native code]' },
+  exception: {
+    values: [
+      {
+        stacktrace: {
+          frames: [
+            { filename: 'https://our-side.com/js/bundle.js' },
+            { filename: 'https://awesome-analytics.io/some/file.js' },
+            { filename: '[native code]' },
+          ],
+        },
+      },
     ],
   },
 };
 
-const EXCEPTION_EVENT = {
+const EXCEPTION_EVENT: Event = {
   exception: {
     values: [
       {
@@ -106,7 +124,7 @@ const EXCEPTION_EVENT = {
   },
 };
 
-const EXCEPTION_EVENT_WITH_FRAMES = {
+const EXCEPTION_EVENT_WITH_FRAMES: Event = {
   exception: {
     values: [
       {
@@ -124,7 +142,7 @@ const EXCEPTION_EVENT_WITH_FRAMES = {
   },
 };
 
-const SENTRY_EVENT = {
+const SENTRY_EVENT: Event = {
   exception: {
     values: [
       {
@@ -135,7 +153,7 @@ const SENTRY_EVENT = {
   },
 };
 
-const SCRIPT_ERROR_EVENT = {
+const SCRIPT_ERROR_EVENT: Event = {
   exception: {
     values: [
       {
@@ -146,9 +164,15 @@ const SCRIPT_ERROR_EVENT = {
   },
 };
 
-const MALFORMED_EVENT = {
-  stacktrace: {
-    frames: undefined,
+const MALFORMED_EVENT: Event = {
+  exception: {
+    values: [
+      {
+        stacktrace: {
+          frames: undefined,
+        },
+      },
+    ],
   },
 };
 

diff --git a/packages/integrations/src/dedupe.ts b/packages/integrations/src/dedupe.ts
@@ -198,8 +198,6 @@ function _getFramesFromEvent(event: Event): StackFrame[] | undefined {
     } catch (_oO) {
       return undefined;
     }
-  } else if (event.stacktrace) {
-    return event.stacktrace.frames;
   }
   return undefined;
 }
diff --git a/packages/integrations/src/rewriteframes.ts b/packages/integrations/src/rewriteframes.ts
@@ -61,10 +61,6 @@ export class RewriteFrames implements Integration {
       processedEvent = this._processExceptionsEvent(processedEvent);
     }
 
-    if (originalEvent.stacktrace) {
-      processedEvent = this._processStacktraceEvent(processedEvent);
-    }
-
     return processedEvent;
   }
 
@@ -110,18 +106,6 @@ export class RewriteFrames implements Integration {
     }
   }
 
-  /** JSDoc */
-  private _processStacktraceEvent(event: Event): Event {
-    try {
-      return {
-        ...event,
-        stacktrace: this._processStacktrace(event.stacktrace),
-      };
-    } catch (_oO) {
-      return event;
-    }
-  }
-
   /** JSDoc */
   private _processStacktrace(stacktrace?: Stacktrace): Stacktrace {
     return {

diff --git a/packages/integrations/test/dedupe.test.ts b/packages/integrations/test/dedupe.test.ts
@@ -1,31 +1,40 @@
+import { Event } from '@sentry/types';
+
 import { _shouldDropEvent } from '../src/dedupe';
 
 /** JSDoc */
 function clone<T>(data: T): T {
   return JSON.parse(JSON.stringify(data));
 }
 
-const messageEvent = {
+const messageEvent: Event = {
   fingerprint: ['MrSnuffles'],
   message: 'PickleRick',
-  stacktrace: {
-    frames: [
-      {
-        colno: 1,
-        filename: 'filename.js',
-        function: 'function',
-        lineno: 1,
-      },
+  exception: {
+    values: [
       {
-        colno: 2,
-        filename: 'filename.js',
-        function: 'function',
-        lineno: 2,
+        value: 'PickleRick',
+        stacktrace: {
+          frames: [
+            {
+              colno: 1,
+              filename: 'filename.js',
+              function: 'function',
+              lineno: 1,
+            },
+            {
+              colno: 2,
+              filename: 'filename.js',
+              function: 'function',
+              lineno: 2,
+            },
+          ],
+        },
       },
     ],
   },
 };
-const exceptionEvent = {
+const exceptionEvent: Event = {
   exception: {
     values: [
       {
@@ -64,13 +73,14 @@ describe('Dedupe', () => {
       const eventA = clone(messageEvent);
       const eventB = clone(messageEvent);
       eventB.message = 'EvilMorty';
+      eventB.exception.values[0].value = 'EvilMorty';
       expect(_shouldDropEvent(eventA, eventB)).toBe(false);
     });
 
     it('should not drop if events have same messages, but different stacktraces', () => {
       const eventA = clone(messageEvent);
       const eventB = clone(messageEvent);
-      eventB.stacktrace.frames[0].colno = 1337;
+      eventB.exception.values[0].stacktrace.frames[0].colno = 1337;
       expect(_shouldDropEvent(eventA, eventB)).toBe(false);
     });