fix(core): Filter internal API frames for synthetic frames (#8710)

As well as fixes the ordering of operations and removes some redundant
code.
The stack should be trimmed _after_ filtering.

Fixes #8396

Author: kamilogorek

packages/utils/src/stacktrace.ts

@@ -6,6 +6,7 @@ import { node } from './node-stack-trace';
 const STACKTRACE_FRAME_LIMIT = 50;
 // Used to sanitize webpack (error: *) wrapped stack errors
 const WEBPACK_ERROR_REGEXP = /\(error: (.*)\)/;
+const STRIP_FRAME_REGEXP = /captureMessage|captureException/;
 
 /**
  * Creates a stack parser with the supplied line parsers
@@ -83,24 +84,34 @@ export function stripSentryFramesAndReverse(stack: ReadonlyArray<StackFrame>): S
     return [];
   }
 
-  const localStack = stack.slice(0, STACKTRACE_FRAME_LIMIT);
+  const localStack = Array.from(stack);
 
-  const lastFrameFunction = localStack[localStack.length - 1].function;
   // If stack starts with one of our API calls, remove it (starts, meaning it's the top of the stack - aka last call)
-  if (lastFrameFunction && /sentryWrapped/.test(lastFrameFunction)) {
+  if (/sentryWrapped/.test(localStack[localStack.length - 1].function || '')) {
     localStack.pop();
   }
 
   // Reversing in the middle of the procedure allows us to just pop the values off the stack
   localStack.reverse();
 
-  const firstFrameFunction = localStack[localStack.length - 1].function;
   // If stack ends with one of our internal API calls, remove it (ends, meaning it's the bottom of the stack - aka top-most call)
-  if (firstFrameFunction && /captureMessage|captureException/.test(firstFrameFunction)) {
+  if (STRIP_FRAME_REGEXP.test(localStack[localStack.length - 1].function || '')) {
     localStack.pop();
+
+    // When using synthetic events, we will have a 2 levels deep stack, as `new Error('Sentry syntheticException')`
+    // is produced within the hub itself, making it:
+    //
+    //   Sentry.captureException()
+    //   getCurrentHub().captureException()
+    //
+    // instead of just the top `Sentry` call itself.
+    // This forces us to possibly strip an additional frame in the exact same was as above.
+    if (STRIP_FRAME_REGEXP.test(localStack[localStack.length - 1].function || '')) {
+      localStack.pop();
+    }
   }
 
-  return localStack.map(frame => ({
+  return localStack.slice(0, STACKTRACE_FRAME_LIMIT).map(frame => ({
     ...frame,
     filename: frame.filename || localStack[localStack.length - 1].filename,
     function: frame.function || '?',

packages/utils/test/stacktrace.test.ts

@@ -32,6 +32,34 @@ describe('Stacktrace', () => {
         expect(frames[0].function).toBe('bar');
         expect(frames[1].function).toBe('foo');
       });
+
+      it('remove two occurences if they are present', () => {
+        const exceptionStack = [
+          { colno: 1, lineno: 4, filename: 'anything.js', function: 'captureException' },
+          { colno: 1, lineno: 4, filename: 'anything.js', function: 'captureException' },
+          { colno: 1, lineno: 3, filename: 'anything.js', function: 'foo' },
+          { colno: 1, lineno: 2, filename: 'anything.js', function: 'bar' },
+        ];
+
+        const exceptionFrames = stripSentryFramesAndReverse(exceptionStack);
+
+        expect(exceptionFrames.length).toBe(2);
+        expect(exceptionFrames[0].function).toBe('bar');
+        expect(exceptionFrames[1].function).toBe('foo');
+
+        const messageStack = [
+          { colno: 1, lineno: 4, filename: 'anything.js', function: 'captureMessage' },
+          { colno: 1, lineno: 4, filename: 'anything.js', function: 'captureMessage' },
+          { colno: 1, lineno: 3, filename: 'anything.js', function: 'foo' },
+          { colno: 1, lineno: 2, filename: 'anything.js', function: 'bar' },
+        ];
+
+        const messageFrames = stripSentryFramesAndReverse(messageStack);
+
+        expect(messageFrames.length).toBe(2);
+        expect(messageFrames[0].function).toBe('bar');
+        expect(messageFrames[1].function).toBe('foo');
+      });
     });
 
     describe('removed bottom frame if its internally reserved word (internal API)', () => {
@@ -53,6 +81,7 @@ describe('Stacktrace', () => {
 
     it('removed top and bottom frame if they are internally reserved words', () => {
       const stack = [
+        { colno: 1, lineno: 4, filename: 'anything.js', function: 'captureMessage' },
         { colno: 1, lineno: 4, filename: 'anything.js', function: 'captureMessage' },
         { colno: 1, lineno: 3, filename: 'anything.js', function: 'foo' },
         { colno: 1, lineno: 2, filename: 'anything.js', function: 'bar' },
@@ -66,6 +95,25 @@ describe('Stacktrace', () => {
       expect(frames[0].function).toBe('bar');
       expect(frames[1].function).toBe('foo');
     });
+
+    it('applies frames limit after the stripping, not before', () => {
+      const stack = Array.from({ length: 55 }).map((_, i) => {
+        return { colno: 1, lineno: 4, filename: 'anything.js', function: `${i}` };
+      });
+
+      stack.unshift({ colno: 1, lineno: 4, filename: 'anything.js', function: 'captureMessage' });
+      stack.unshift({ colno: 1, lineno: 4, filename: 'anything.js', function: 'captureMessage' });
+      stack.push({ colno: 1, lineno: 4, filename: 'anything.js', function: 'sentryWrapped' });
+
+      // Should remove 2x `captureMessage`, `sentryWrapped`, and then limit frames to default 50.
+      const frames = stripSentryFramesAndReverse(stack);
+
+      expect(frames.length).toBe(50);
+
+      // Frames are named 0-54, thus after reversal and trimming, we should have frames 54-5, 50 in total.
+      expect(frames[0].function).toBe('54');
+      expect(frames[49].function).toBe('5');
+    });
   });
 });