Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions packages/utils/src/stacktrace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 || '?',
Expand Down
48 changes: 48 additions & 0 deletions packages/utils/test/stacktrace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)', () => {
Expand All @@ -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' },
Expand All @@ -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');
});
});
});

Expand Down