From 0d58347aa53971ef79175b7e21b5a4a7236e7f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Wed, 2 Aug 2023 11:18:56 +0200 Subject: [PATCH 1/2] fix(code): Filter internal API frames for synthetic frames --- packages/utils/src/stacktrace.ts | 23 ++++++++---- packages/utils/test/stacktrace.test.ts | 48 ++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/packages/utils/src/stacktrace.ts b/packages/utils/src/stacktrace.ts index f137b73264ba..0b32d084f28e 100644 --- a/packages/utils/src/stacktrace.ts +++ b/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): 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 || '?', diff --git a/packages/utils/test/stacktrace.test.ts b/packages/utils/test/stacktrace.test.ts index 295fa282d906..5523a2336226 100644 --- a/packages/utils/test/stacktrace.test.ts +++ b/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'); + }); }); }); From af24fd60af0ae7492a83677724fcc165e5a307a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Wed, 2 Aug 2023 11:52:27 +0200 Subject: [PATCH 2/2] lint --- packages/utils/src/stacktrace.ts | 2 +- packages/utils/test/stacktrace.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/utils/src/stacktrace.ts b/packages/utils/src/stacktrace.ts index 0b32d084f28e..ac9f2159221d 100644 --- a/packages/utils/src/stacktrace.ts +++ b/packages/utils/src/stacktrace.ts @@ -6,7 +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/ +const STRIP_FRAME_REGEXP = /captureMessage|captureException/; /** * Creates a stack parser with the supplied line parsers diff --git a/packages/utils/test/stacktrace.test.ts b/packages/utils/test/stacktrace.test.ts index 5523a2336226..4e87399b91db 100644 --- a/packages/utils/test/stacktrace.test.ts +++ b/packages/utils/test/stacktrace.test.ts @@ -98,12 +98,12 @@ describe('Stacktrace', () => { 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}` } + 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' }) + 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);