Skip to content

ref(browser): Replace TracekitStackFrame with Sentry StackFrame #4523

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.

Sign up for GitHub

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 6 commits into from
Feb 11, 2022
Merged

ref(browser): Replace TracekitStackFrame with Sentry StackFrame #4523

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0ec6249
Refactor tests and minor improvements
timfish Feb 7, 2022
5563471
Remove TraceKitStackFrame
timfish Feb 8, 2022
4db0290
Remove undefined
timfish Feb 8, 2022
8e09b5e
Merge remote-tracking branch 'upstream/master' into remove-tracekitst…
timfish Feb 10, 2022
87c5628
Merge opera parser to slim down code
timfish Feb 10, 2022
245eee9
Lil tidy
timfish Feb 10, 2022
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
24 changes: 11 additions & 13 deletions packages/browser/src/parsers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Event, Exception, StackFrame } from '@sentry/types';
import { extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils';

import { computeStackTrace, StackFrame as TraceKitStackFrame, StackTrace as TraceKitStackTrace } from './tracekit';
import { computeStackTrace, StackTrace as TraceKitStackTrace } from './tracekit';

const STACKTRACE_LIMIT = 50;

Expand Down Expand Up @@ -80,15 +80,15 @@ export function eventFromStacktrace(stacktrace: TraceKitStackTrace): Event {
/**
* @hidden
*/
export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[] {
export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] {
if (!stack || !stack.length) {
return [];
}

let localStack = stack;

const firstFrameFunction = localStack[0].func || '';
const lastFrameFunction = localStack[localStack.length - 1].func || '';
const firstFrameFunction = localStack[0].function || '';
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 (firstFrameFunction.indexOf('captureMessage') !== -1 || firstFrameFunction.indexOf('captureException') !== -1) {
Expand All @@ -103,14 +103,12 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
// The frame where the crash happened, should be the last entry in the array
return localStack
.slice(0, STACKTRACE_LIMIT)
.map(
(frame: TraceKitStackFrame): StackFrame => ({
colno: frame.column === null ? undefined : frame.column,
filename: frame.url || localStack[0].url,
function: frame.func || '?',
in_app: true,
lineno: frame.line === null ? undefined : frame.line,
}),
)
.map(frame => ({
colno: frame.colno,
filename: frame.filename || localStack[0].filename,
function: frame.function || '?',
in_app: true,
lineno: frame.lineno,
}))
.reverse();
}
99 changes: 38 additions & 61 deletions packages/browser/src/tracekit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,12 @@
import { StackFrame } from '@sentry/types';

/**
* This was originally forked from https://github.com/occ/TraceKit, but has since been
* largely modified and is now maintained as part of Sentry JS SDK.
*/

/* eslint-disable @typescript-eslint/no-unsafe-member-access, max-lines */

/**
* An object representing a single stack frame.
* {Object} StackFrame
* {string} url The JavaScript or HTML file URL.
* {string} func The function name, or empty for anonymous functions (if guessing did not work).
* {string[]?} args The arguments passed to the function, if known.
* {number=} line The line number, if known.
* {number=} column The column number, if known.
* {string[]} context An array of source code lines; the middle element corresponds to the correct line#.
*/
export interface StackFrame {
url: string;
func: string;
line: number | null;
column: number | null;
}

/**
* An object representing a JavaScript stack trace.
* {Object} StackTrace
Expand All @@ -32,9 +17,7 @@ export interface StackFrame {
export interface StackTrace {
name: string;
message: string;
mechanism?: string;
stack: StackFrame[];
failed?: boolean;
}

// global reference to slice
Expand All @@ -56,9 +39,8 @@ const chromeEval = /\((\S*)(?::(\d+))(?::(\d+))\)/;
const reactMinifiedRegexp = /Minified React error #\d+;/i;

/** JSDoc */
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
export function computeStackTrace(ex: any): StackTrace {
let stack = null;
export function computeStackTrace(ex: Error & { framesToPop?: number }): StackTrace {
let stack;
let popSize = 0;

if (ex) {
Expand Down Expand Up @@ -94,13 +76,12 @@ export function computeStackTrace(ex: any): StackTrace {
message: extractMessage(ex),
name: ex && ex.name,
stack: [],
failed: true,
};
}

/** JSDoc */
// eslint-disable-next-line @typescript-eslint/no-explicit-any, complexity
function computeStackTraceFromStackProp(ex: any): StackTrace | null {
// eslint-disable-next-line complexity
function computeStackTraceFromStackProp(ex: Error): StackTrace | null {
if (!ex || !ex.stack) {
return null;
}
Expand All @@ -110,7 +91,7 @@ function computeStackTraceFromStackProp(ex: any): StackTrace | null {
let isEval;
let submatch;
let parts;
let element;
let element: StackFrame | undefined = undefined;

for (const line of lines) {
if ((parts = chrome.exec(line))) {
Expand All @@ -124,20 +105,20 @@ function computeStackTraceFromStackProp(ex: any): StackTrace | null {

// Kamil: One more hack won't hurt us right? Understanding and adding more rules on top of these regexps right now
// would be way too time consuming. (TODO: Rewrite whole RegExp to be more readable)
const [func, url] = extractSafariExtensionDetails(parts[1] || UNKNOWN_FUNCTION, parts[2]);
const [func, filename] = extractSafariExtensionDetails(parts[1] || UNKNOWN_FUNCTION, parts[2]);

element = {
url,
func,
line: parts[3] ? +parts[3] : null,
column: parts[4] ? +parts[4] : null,
filename,
function: func,
lineno: parts[3] ? +parts[3] : undefined,
colno: parts[4] ? +parts[4] : undefined,
};
} else if ((parts = winjs.exec(line))) {
element = {
url: parts[2],
func: parts[1] || UNKNOWN_FUNCTION,
line: +parts[3],
column: parts[4] ? +parts[4] : null,
filename: parts[2],
function: parts[1] || UNKNOWN_FUNCTION,
lineno: +parts[3],
colno: parts[4] ? +parts[4] : undefined,
};
} else if ((parts = gecko.exec(line))) {
isEval = parts[3] && parts[3].indexOf(' > eval') > -1;
Expand All @@ -149,15 +130,15 @@ function computeStackTraceFromStackProp(ex: any): StackTrace | null {
parts[5] = ''; // no column when eval
}

let url = parts[3];
let filename = parts[3];
let func = parts[1] || UNKNOWN_FUNCTION;
[func, url] = extractSafariExtensionDetails(func, url);
[func, filename] = extractSafariExtensionDetails(func, filename);

element = {
url,
func,
line: parts[4] ? +parts[4] : null,
column: parts[5] ? +parts[5] : null,
filename,
function: func,
lineno: parts[4] ? +parts[4] : undefined,
colno: parts[5] ? +parts[5] : undefined,
};
} else {
continue;
Expand All @@ -178,8 +159,7 @@ function computeStackTraceFromStackProp(ex: any): StackTrace | null {
}

/** JSDoc */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function computeStackTraceFromStacktraceProp(ex: any): StackTrace | null {
function computeStackTraceFromStacktraceProp(ex: Error & { stacktrace?: string }): StackTrace | null {
if (!ex || !ex.stacktrace) {
return null;
}
Expand All @@ -194,28 +174,25 @@ function computeStackTraceFromStacktraceProp(ex: any): StackTrace | null {
const stack = [];
let parts;

for (let line = 0; line < lines.length; line += 2) {
let element = null;
if ((parts = opera10Regex.exec(lines[line]))) {
for (const line of lines) {
let element: StackFrame | undefined = undefined;

if ((parts = opera10Regex.exec(line))) {
element = {
url: parts[2],
func: parts[3],
line: +parts[1],
column: null,
filename: parts[2],
function: parts[3] || UNKNOWN_FUNCTION,
lineno: +parts[1],
};
} else if ((parts = opera11Regex.exec(lines[line]))) {
} else if ((parts = opera11Regex.exec(line))) {
element = {
url: parts[5],
func: parts[3] || parts[4],
line: +parts[1],
column: +parts[2],
filename: parts[5],
function: parts[3] || parts[4] || UNKNOWN_FUNCTION,
lineno: +parts[1],
colno: +parts[2],
};
}

if (element) {
if (!element.func && element.line) {
element.func = UNKNOWN_FUNCTION;
}
stack.push(element);
}
}
Expand Down Expand Up @@ -251,16 +228,16 @@ function computeStackTraceFromStacktraceProp(ex: any): StackTrace | null {
* Unfortunatelly "just" changing RegExp is too complicated now and making it pass all tests
* and fix this case seems like an impossible, or at least way too time-consuming task.
*/
const extractSafariExtensionDetails = (func: string, url: string): [string, string] => {
const extractSafariExtensionDetails = (func: string, filename: string): [string, string] => {
const isSafariExtension = func.indexOf('safari-extension') !== -1;
const isSafariWebExtension = func.indexOf('safari-web-extension') !== -1;

return isSafariExtension || isSafariWebExtension
? [
func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION,
isSafariExtension ? `safari-extension:${url}` : `safari-web-extension:${url}`,
isSafariExtension ? `safari-extension:${filename}` : `safari-web-extension:${filename}`,
]
: [func, url];
: [func, filename];
};

/** Remove N number of frames from the stack */
Expand Down
26 changes: 13 additions & 13 deletions packages/browser/test/unit/parsers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ describe('Parsers', () => {
describe('removed top frame if its internally reserved word (public API)', () => {
it('reserved captureException', () => {
const stack = [
{ context: ['x'], column: 1, line: 4, url: 'anything.js', func: 'captureException', args: [] },
{ context: ['x'], column: 1, line: 3, url: 'anything.js', func: 'foo', args: [] },
{ context: ['x'], column: 1, line: 2, url: 'anything.js', func: 'bar', args: [] },
{ 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' },
];

// Should remove `captureException` as its a name considered "internal"
Expand All @@ -20,9 +20,9 @@ describe('Parsers', () => {

it('reserved captureMessage', () => {
const stack = [
{ context: ['x'], column: 1, line: 4, url: 'anything.js', func: 'captureMessage', args: [] },
{ context: ['x'], column: 1, line: 3, url: 'anything.js', func: 'foo', args: [] },
{ context: ['x'], column: 1, line: 2, url: 'anything.js', func: 'bar', args: [] },
{ 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' },
];

// Should remove `captureMessage` as its a name considered "internal"
Expand All @@ -37,9 +37,9 @@ describe('Parsers', () => {
describe('removed bottom frame if its internally reserved word (internal API)', () => {
it('reserved sentryWrapped', () => {
const stack = [
{ context: ['x'], column: 1, line: 3, url: 'anything.js', func: 'foo', args: [] },
{ context: ['x'], column: 1, line: 2, url: 'anything.js', func: 'bar', args: [] },
{ context: ['x'], column: 1, line: 1, url: 'anything.js', func: 'sentryWrapped', args: [] },
{ colno: 1, lineno: 3, filename: 'anything.js', function: 'foo' },
{ colno: 1, lineno: 2, filename: 'anything.js', function: 'bar' },
{ colno: 1, lineno: 1, filename: 'anything.js', function: 'sentryWrapped' },
];

// Should remove `sentryWrapped` as its a name considered "internal"
Expand All @@ -53,10 +53,10 @@ describe('Parsers', () => {

it('removed top and bottom frame if they are internally reserved words', () => {
const stack = [
{ context: ['x'], column: 1, line: 4, url: 'anything.js', func: 'captureMessage', args: [] },
{ context: ['x'], column: 1, line: 3, url: 'anything.js', func: 'foo', args: [] },
{ context: ['x'], column: 1, line: 2, url: 'anything.js', func: 'bar', args: [] },
{ context: ['x'], column: 1, line: 1, url: 'anything.js', func: 'sentryWrapped', args: [] },
{ 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' },
{ colno: 1, lineno: 1, filename: 'anything.js', function: 'sentryWrapped' },
];

// Should remove `captureMessage` and `sentryWrapped` as its a name considered "internal"
Expand Down
Loading