ref: Deprecate top-level stacktrace (#2214)

kamilogorek · web-flow · commit 36d206cd3003 · 2019-08-29T14:01:40.000+02:00
* ref: Deprecate top-level stacktrace
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,8 @@
 
 ## Unreleased
 
-- [browser] fix: Don't capture our own XHR events that somehow bubbled-up to global handler
+- [browser] fix: Don't capture our own XHR events that somehow bubbled-up to global handler (#2221)
+- [core] ref: Deprecate top-level stacktrace event attribute (#2214)
 
 ## 5.6.2
 
diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts
@@ -151,9 +151,13 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
     if (this._options.attachStacktrace && hint && hint.syntheticException) {
       const stacktrace = _computeStackTrace(hint.syntheticException);
       const frames = prepareFramesForEvent(stacktrace.stack);
-      event.stacktrace = {
-        frames,
-      };
+      event.threads = [
+        {
+          stacktrace: {
+            frames,
+          },
+        },
+      ];
     }
 
     return SyncPromise.resolve(event);
diff --git a/packages/browser/src/parsers.ts b/packages/browser/src/parsers.ts
@@ -45,9 +45,13 @@ export function eventFromPlainObject(exception: {}, syntheticException: Error |
   if (syntheticException) {
     const stacktrace = _computeStackTrace(syntheticException);
     const frames = prepareFramesForEvent(stacktrace.stack);
-    event.stacktrace = {
-      frames,
-    };
+    event.threads = [
+      {
+        stacktrace: {
+          frames,
+        },
+      },
+    ];
   }
 
   return event;
diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts
@@ -161,6 +161,7 @@ export class InboundFilters implements Integration {
   /** JSDoc */
   private _getEventFilterUrl(event: Event): string | null {
     try {
+      // tslint:disable-next-line:deprecation
       if (event.stacktrace) {
         // tslint:disable:no-unsafe-any
         const frames = (event as any).stacktrace.frames;
@@ -171,6 +172,11 @@ export class InboundFilters implements Integration {
         const frames = (event as any).exception.values[0].stacktrace.frames;
         return frames[frames.length - 1].filename;
       }
+      if (event.threads) {
+        // tslint:disable:no-unsafe-any
+        const frames = (event as any).threads[0].stacktrace.frames;
+        return frames[frames.length - 1].filename;
+      }
       return null;
     } catch (oO) {
       logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
diff --git a/packages/integrations/src/dedupe.ts b/packages/integrations/src/dedupe.ts
@@ -1,5 +1,7 @@
 import { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types';
 
+import { getFramesFromEvent } from './helpers';
+
 /** Deduplication filter */
 export class Dedupe implements Integration {
   /**
@@ -86,27 +88,10 @@ export class Dedupe implements Integration {
     return true;
   }
 
-  /** JSDoc */
-  private _getFramesFromEvent(event: Event): StackFrame[] | undefined {
-    const exception = event.exception;
-
-    if (exception) {
-      try {
-        // @ts-ignore
-        return exception.values[0].stacktrace.frames;
-      } catch (_oO) {
-        return undefined;
-      }
-    } else if (event.stacktrace) {
-      return event.stacktrace.frames;
-    }
-    return undefined;
-  }
-
   /** JSDoc */
   private _isSameStacktrace(currentEvent: Event, previousEvent: Event): boolean {
-    let currentFrames = this._getFramesFromEvent(currentEvent);
-    let previousFrames = this._getFramesFromEvent(previousEvent);
+    let currentFrames = getFramesFromEvent(currentEvent);
+    let previousFrames = getFramesFromEvent(previousEvent);
 
     // If no event has a fingerprint, they are assumed to be the same
     if (!currentFrames && !previousFrames) {
diff --git a/packages/integrations/src/helpers.ts b/packages/integrations/src/helpers.ts
@@ -0,0 +1,27 @@
+import { Event, StackFrame } from '@sentry/types';
+
+/**
+ * Extract frames from the Event, independent of it's shape
+ */
+export function getFramesFromEvent(event: Event): StackFrame[] | undefined {
+  if (event.exception) {
+    try {
+      // @ts-ignore
+      return event.exception.values[0].stacktrace.frames;
+    } catch (_oO) {
+      return undefined;
+    }
+    // tslint:disable:deprecation
+  } else if (event.stacktrace) {
+    return event.stacktrace.frames;
+    // tslint:enable:deprecation
+  } else if (event.threads) {
+    try {
+      // @ts-ignore
+      return event.threads[0].stacktrace.frames;
+    } catch (_oO) {
+      return undefined;
+    }
+  }
+  return undefined;
+}
diff --git a/packages/integrations/src/rewriteframes.ts b/packages/integrations/src/rewriteframes.ts
@@ -1,6 +1,8 @@
 import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
 import { basename, relative } from '@sentry/utils';
 
+import { getFramesFromEvent } from './helpers';
+
 type StackFrameIteratee = (frame: StackFrame) => StackFrame;
 
 /** Rewrite event frames paths */
@@ -58,30 +60,11 @@ export class RewriteFrames implements Integration {
 
   /** JSDoc */
   public process(event: Event): Event {
-    const frames = this._getFramesFromEvent(event);
-    if (frames) {
-      for (const i in frames) {
-        // tslint:disable-next-line
-        frames[i] = this._iteratee(frames[i]);
-      }
+    const frames = getFramesFromEvent(event) || [];
+    for (const i in frames) {
+      // tslint:disable-next-line
+      frames[i] = this._iteratee(frames[i]);
     }
     return event;
   }
-
-  /** JSDoc */
-  private _getFramesFromEvent(event: Event): StackFrame[] | undefined {
-    const exception = event.exception;
-
-    if (exception) {
-      try {
-        // tslint:disable-next-line:no-unsafe-any
-        return (exception as any).values[0].stacktrace.frames;
-      } catch (_oO) {
-        return undefined;
-      }
-    } else if (event.stacktrace) {
-      return event.stacktrace.frames;
-    }
-    return undefined;
-  }
 }
diff --git a/packages/integrations/src/transaction.ts b/packages/integrations/src/transaction.ts
@@ -1,5 +1,7 @@
 import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
 
+import { getFramesFromEvent } from './helpers';
+
 /** Add node transaction to the event */
 export class Transaction implements Integration {
   /**
@@ -28,7 +30,7 @@ export class Transaction implements Integration {
    * @inheritDoc
    */
   public process(event: Event): Event {
-    const frames = this._getFramesFromEvent(event);
+    const frames = getFramesFromEvent(event) || [];
 
     // use for loop so we don't have to reverse whole frames array
     for (let i = frames.length - 1; i >= 0; i--) {
@@ -43,12 +45,6 @@ export class Transaction implements Integration {
     return event;
   }
 
-  /** JSDoc */
-  private _getFramesFromEvent(event: Event): StackFrame[] {
-    const exception = event.exception && event.exception.values && event.exception.values[0];
-    return (exception && exception.stacktrace && exception.stacktrace.frames) || [];
-  }
-
   /** JSDoc */
   private _getTransaction(frame: StackFrame): string {
     return frame.module || frame.function ? `${frame.module || '?'}/${frame.function || '?'}` : '<unknown>';
diff --git a/packages/integrations/test/helpers.test.ts b/packages/integrations/test/helpers.test.ts
@@ -0,0 +1,100 @@
+import { getFramesFromEvent } from '../src/helpers';
+
+describe('getFramesFromEvent', () => {
+  it('event.exception.values[0].stacktrace.frames', () => {
+    const frames = getFramesFromEvent({
+      exception: {
+        values: [
+          {
+            stacktrace: {
+              frames: [
+                {
+                  filename: '/www/src/app/file1.js',
+                },
+                {
+                  filename: '/www/src/app/file2.js',
+                },
+              ],
+            },
+          },
+        ],
+      },
+    });
+    expect(frames).toEqual([
+      {
+        filename: '/www/src/app/file1.js',
+      },
+      {
+        filename: '/www/src/app/file2.js',
+      },
+    ]);
+  });
+
+  it('event.stacktrace.frames', () => {
+    const frames = getFramesFromEvent({
+      stacktrace: {
+        frames: [
+          {
+            filename: '/www/src/app/file1.js',
+          },
+          {
+            filename: '/www/src/app/file2.js',
+          },
+        ],
+      },
+    });
+    expect(frames).toEqual([
+      {
+        filename: '/www/src/app/file1.js',
+      },
+      {
+        filename: '/www/src/app/file2.js',
+      },
+    ]);
+  });
+
+  it('event.threads[0].stacktrace.frames', () => {
+    const frames = getFramesFromEvent({
+      threads: [
+        {
+          stacktrace: {
+            frames: [
+              {
+                filename: '/www/src/app/file1.js',
+              },
+              {
+                filename: '/www/src/app/file2.js',
+              },
+            ],
+          },
+        },
+      ],
+    });
+    expect(frames).toEqual([
+      {
+        filename: '/www/src/app/file1.js',
+      },
+      {
+        filename: '/www/src/app/file2.js',
+      },
+    ]);
+  });
+
+  it('no frames', () => {
+    const frames = getFramesFromEvent({});
+    expect(frames).toEqual(undefined);
+  });
+
+  it('broken frames', () => {
+    const exceptionNoFrames = getFramesFromEvent({
+      exception: {
+        values: [],
+      },
+    });
+    const threadsNoFrames = getFramesFromEvent({
+      threads: [],
+    });
+    expect(exceptionNoFrames).toEqual(undefined);
+    expect(threadsNoFrames).toEqual(undefined);
+  });
+});
diff --git a/packages/integrations/test/rewriteframes.test.ts b/packages/integrations/test/rewriteframes.test.ts
@@ -1,10 +1,13 @@
+// tslint:disable:deprecation
+
 import { Event, StackFrame } from '@sentry/types';
 
 import { RewriteFrames } from '../src/rewriteframes';
 
 let rewriteFrames: RewriteFrames;
 let messageEvent: Event;
 let exceptionEvent: Event;
+let threadEvent: Event;
 
 describe('RewriteFrames', () => {
   beforeEach(() => {
@@ -38,6 +41,22 @@ describe('RewriteFrames', () => {
         ],
       },
     };
+    threadEvent = {
+      threads: [
+        {
+          stacktrace: {
+            frames: [
+              {
+                filename: '/www/src/app/file1.js',
+              },
+              {
+                filename: '/www/src/app/file2.js',
+              },
+            ],
+          },
+        },
+      ],
+    };
   });
 
   describe('default iteratee appends basename to `app:///` if frame starts with `/`', () => {
@@ -56,6 +75,12 @@ describe('RewriteFrames', () => {
       expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js');
       expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js');
     });
+
+    it('transforms threadEvent frames', () => {
+      const event = rewriteFrames.process(threadEvent);
+      expect(event.threads![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js');
+      expect(event.threads![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js');
+    });
   });
 
   describe('can use custom root to perform `relative` on filepaths', () => {
@@ -76,6 +101,12 @@ describe('RewriteFrames', () => {
       expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
       expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js');
     });
+
+    it('transforms threadEvent frames', async () => {
+      const event = rewriteFrames.process(threadEvent);
+      expect(event.threads![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
+      expect(event.threads![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js');
+    });
   });
 
   describe('can use custom iteratee', () => {
@@ -103,5 +134,13 @@ describe('RewriteFrames', () => {
       expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js');
       expect(event.exception!.values![0].stacktrace!.frames![1].function).toEqual('whoops');
     });
+
+    it('transforms threadEvent frames', async () => {
+      const event = rewriteFrames.process(threadEvent);
+      expect(event.threads![0].stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
+      expect(event.threads![0].stacktrace!.frames![0].function).toEqual('whoops');
+      expect(event.threads![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js');
+      expect(event.threads![0].stacktrace!.frames![1].function).toEqual('whoops');
+    });
   });
 });
diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts
@@ -130,9 +130,13 @@ export class NodeBackend extends BaseBackend<NodeOptions> {
       if (this._options.attachStacktrace && hint && hint.syntheticException) {
         const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : [];
         parseStack(stack, this._options).then(frames => {
-          event.stacktrace = {
-            frames: prepareFramesForEvent(frames),
-          };
+          event.threads = [
+            {
+              stacktrace: {
+                frames: prepareFramesForEvent(frames),
+              },
+            },
+          ];
           resolve(event);
         });
       } else {
diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts
