8000 fix: Fixes stack trace order + es5 tracekit + basebackend impl (#1512) · rossleonardy/sentry-javascript@755c1e6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 755c1e6

Browse files
HazATHazAT
authored
fix: Fixes stack trace order + es5 tracekit + basebackend impl (getsentry#1512)
* fix: Fixes stack trace order + es5 tracekit + basebackend impl * feat: Add tests for stacktrace order
1 parent ebd4fff commit 755c1e6

File tree

6 files changed

+111
-30
lines changed

6 files changed

+111
-30
lines changed

‎packages/browser/src/parsers.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
6868
localStack = localStack.slice(1);
6969
}
7070

71+
// The frame where the crash happened, should be the last entry in the array
7172
return localStack
7273
.map(
7374
(frame: TraceKitStackFrame): StackFrame => ({
@@ -78,5 +79,6 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
7879
lineno: frame.line,
7980
}),
8081
)
81-
.slice(0, STACKTRACE_LIMIT);
82+
.slice(0, STACKTRACE_LIMIT)
83+
.reverse();
8284
}

‎packages/browser/src/tracekit/index.js

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,25 @@
1-
import { isUndefined, isError, isErrorEvent } from '@sentry/utils/is';
1+
'use strict';
2+
3+
Object.defineProperty(exports, '__esModule', {
4+
value: true,
5+
});
6+
exports.computeStackTrace = exports.installGlobalUnhandledRejectionHandler = exports.installGlobalHandler = exports.subscribe = undefined;
7+
8+
var _extends =
9+
Object.assign ||
10+
function(target) {
11+
for (var i = 1; i < arguments.length; i++) {
12+
var source = arguments[i];
13+
for (var key in source) {
14+
if (Object.prototype.hasOwnProperty.call(source, key)) {
15+
target[key] = source[key];
16+
}
17+
}
18+
}
19+
return target;
20+
};
21+
22+
var _is = require('@sentry/utils/is');
223

324
/**
425
* TraceKit - Cross brower stack traces
@@ -232,14 +253,14 @@ TraceKit.report = (function reportModuleWrapper() {
232253
function traceKitWindowOnError(message, url, lineNo, columnNo, errorObj) {
233254
var stack = null;
234255
// If 'errorObj' is ErrorEvent, get real Error from inside
235-
errorObj = isErrorEvent(errorObj) ? errorObj.error : errorObj;
256+
errorObj = (0, _is.isErrorEvent)(errorObj) ? errorObj.error : errorObj;
236257
// If 'message' is ErrorEvent, get real message from inside
237-
message = isErrorEvent(message) ? message.message : message;
258+
message = (0, _is.isErrorEvent)(message) ? message.message : message;
238259

239260
if (lastExceptionStack) {
240261
TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message);
241262
processLastException();
242-
} else if (errorObj && isError(errorObj)) {
263+
} else if (errorObj && (0, _is.isError)(errorObj)) {
243264
stack = TraceKit.computeStackTrace(errorObj);
244265
notifyHandlers(stack, true, errorObj);
245266
} else {
@@ -266,13 +287,12 @@ TraceKit.report = (function reportModuleWrapper() {
266287
message: msg,
267288
mode: 'onerror',
268289
stack: [
269-
{
270-
...location,
290+
_extends({}, location, {
271291
// Firefox sometimes doesn't return url correctly and this is an old behavior
272292
// that I prefer to port here as well.
273293
// It can be altered only here, as previously it's using `location.url` for other things — Kamil
274294
url: location.url || getLocationHref(),
275-
},
295+
}),
276296
],
277297
};
278298

@@ -502,7 +522,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
502522
return '';
503523
}
504524
try {
505-
var getXHR = function() {
525+
var getXHR = function getXHR() {
506526
try {
507527
return new window.XMLHttpRequest();
508528
} catch (e) {
@@ -583,7 +603,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
583603
for (var i = 0; i < maxLines; ++i) {
584604
line = source[lineNo - i] + line;
585605

586-
if (!isUndefined(line)) {
606+
if (!(0, _is.isUndefined)(line)) {
587607
if ((m = reGuessFunction.exec(line))) {
588608
return m[1];
589609
} else if ((m = reFunctionArgNames.exec(line))) {
@@ -622,7 +642,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
622642
line -= 1; // convert to 0-based index
623643

624644
for (var i = start; i < end; ++i) {
625-
if (!isUndefined(source[i])) {
645+
if (!(0, _is.isUndefined)(source[i])) {
626646
context.push(source[i]);
627647
}
628648< 10000 div class="diff-text-inner"> }
@@ -716,7 +736,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
716736
* @memberof TraceKit.computeStackTrace
717737
*/
718738
function findSourceByFunctionBody(func) {
719-
if (isUndefined(window && window.document)) {
739+
if ((0, _is.isUndefined)(window && window.document)) {
720740
return;
721741
}
722742

@@ -876,7 +896,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
876896
// NOTE: It's messing out our integration tests in Karma, let's see if we can live with it – Kamil
877897
// parts[4] = submatch[2];
878898
// parts[5] = null; // no column when eval
879-
} else if (i === 0 && !parts[5] && !isUndefined(ex.columnNumber)) {
899+
} else if (i === 0 && !parts[5] && !(0, _is.isUndefined)(ex.columnNumber)) {
880900
// FireFox uses this awesome columnNumber property for its top frame
881901
// Also note, Firefox's column number is 0-based and everything else expects 1-based,
882902
// so adding 1
@@ -977,7 +997,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
977997
opera11Regex = / line (\d+), column (\d+)\s*(?:in (?:<anonymous function: ([^>]+)>|([^\)]+))\((.*)\))? in (.*):\s*$/i,
978998
lines = stacktrace.split('\n'),
979999
stack = [],
980-
parts;
1000+
parts = void 0;
9811001

9821002
for (var line = 0; line < lines.length; line += 2) {
9831003
var element = null;
@@ -1066,7 +1086,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
10661086
stack = [],
10671087
scripts = window && window.document && window.document.getElementsByTagName('script'),
10681088
inlineScriptBlocks = [],
1069-
parts;
1089+
parts = void 0;
10701090

10711091
for (var s in scripts) {
10721092
if (_has(scripts, s) && !scripts[s].src) {
@@ -1217,9 +1237,9 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
12171237
stack = [],
12181238
funcs = {},
12191239
recursion = false,
1220-
parts,
1221-
item,
1222-
source;
1240+
parts = void 0,
1241+
item = void 0,
1242+
source = void 0;
12231243

12241244
for (var curr = computeStackTraceByWalkingCallerChain.caller; curr && !recursion; curr = curr.caller) {
12251245
if (curr === computeStackTrace || curr === TraceKit.report) {
@@ -1407,7 +1427,7 @@ TraceKit.extendToAsynchronousCallbacks = function() {
14071427
_helper('setInterval');
14081428
};
14091429

1410-
//Default options:
1430+
// Default options:
14111431
if (!TraceKit.remoteFetching) {
14121432
TraceKit.remoteFetching = true;
14131433
}
@@ -1419,9 +1439,12 @@ if (!TraceKit.linesOfContext || TraceKit.linesOfContext < 1) {
14191439
TraceKit.linesOfContext = 11;
14201440
}
14211441

1422-
const subscribe = TraceKit.report.subscribe;
1423-
const installGlobalHandler = TraceKit.report.installGlobalHandler;
1424-
const installGlobalUnhandledRejectionHandler = TraceKit.report.installGlobalUnhandledRejectionHandler;
1425-
const computeStackTrace = TraceKit.computeStackTrace;
1442+
var subscribe = TraceKit.report.subscribe;
1443+
var installGlobalHandler = TraceKit.report.installGlobalHandler;
1444+
var installGlobalUnhandledRejectionHandler = TraceKit.report.installGlobalUnhandledRejectionHandler;
1445+
var computeStackTrace = TraceKit.computeStackTrace;
14261446

1427-
export { subscribe, installGlobalHandler, installGlobalUnhandledRejectionHandler, computeStackTrace };
1447+
exports.subscribe = subscribe;
1448+
exports.installGlobalHandler = installGlobalHandler;
1449+
exports.installGlobalUnhandledRejectionHandler = installGlobalUnhandledRejectionHandler;
1450+
exports.computeStackTrace = computeStackTrace;

‎packages/browser/test/integration/test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,33 @@ describe('integration', function() {
116116
);
117117
});
118118

119+
it('should have correct stacktrace order', function(done) {
120+
var iframe = this.iframe;
121+
iframeExecute(
122+
iframe,
123+
done,
124+
function() {
125+
setTimeout(done);
126+
try {
127+
foo();
128+
} catch (e) {
129+
Sentry.captureException(e);
130+
}
131+
},
132+
function() {
133+
var sentryData = iframe.contentWindow.sentryData[0];
134+
assert.equal(
135+
sentryData.exception.values[0].stacktrace.frames[
136+
sentryData.exception.values[0].stacktrace.frames.length - 1
137+
].function,
138+
'bar',
139+
);
140+
assert.isAtLeast(sentryData.exception.values[0].stacktrace.frames.length, 2);
141+
assert.isAtMost(sentryData.exception.values[0].stacktrace.frames.length, 4);
142+
},
143+
);
144+
});
145+
119146
it('should reject duplicate, back-to-back errors from captureError', function(done) {
120147
var iframe = this.iframe;
121148
iframeExecute(

‎packages/core/src/basebackend.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { SentryEvent, SentryEventHint, SentryResponse, Severity, Transport } from '@sentry/types';
1+
import { Scope } from '@sentry/hub';
2+
import { Breadcrumb, SentryEvent, SentryEventHint, SentryResponse, Severity, Tra 10000 nsport } from '@sentry/types';
23
import { SentryError } from './error';
34
import { Backend, Options } from './interfaces';
45
import { logger } from './logger';
@@ -54,14 +55,14 @@ export abstract class BaseBackend<O extends Options> implements Backend {
5455
/**
5556
* @inheritDoc
5657
*/
57-
public storeBreadcrumb(): boolean {
58+
public storeBreadcrumb(_: Breadcrumb): boolean {
5859
return true;
5960
}
6061

6162
/**
6263
* @inheritDoc
6364
*/
64-
public storeScope(): void {
65+
public storeScope(_: Scope): void {
6566
// Noop
6667
}
6768

‎packages/node/src/parsers.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] {
236236
localStack = localStack.slice(1);
237237
}
238238

239-
return localStack;
240-
// Sentry expects the stack trace to be oldest -> newest, v8 provides newest -> oldest
241-
// return filteredFrames.reverse();
239+
// The frame where the crash happened, should be the last entry in the array
240+
return localStack.reverse();
242241
}

‎packages/node/test/index.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,5 +239,34 @@ describe('SentryNode', () => {
239239
getCurrentHub().captureEvent({ message: 'test' });
240240
});
241241
}));
242+
243+
test('stackt E0F9 race order', done => {
244+
expect.assertions(1);
245+
getCurrentHub().pushScope();
246+
getCurrentHub().bindClient(
247+
new NodeClient({
248+
beforeSend: (event: SentryEvent) => {
249+
expect(
250+
event.exception!.values![0].stacktrace!.frames![
251+
event.exception!.values![0].stacktrace!.frames!.length - 1
252+
].function,
253+
).toEqual('testy');
254+
done();
255+
return event;
256+
},
257+
dsn,
258+
}),
259+
);
260+
try {
261+
// @ts-ignore
262+
function testy(): void {
263+
throw new Error('test');
264+
}
265+
testy();
266+
} catch (e) {
267+
captureException(e);
268+
}
269+
getCurrentHub().popScope();
270+
});
242271
});
243272
});

0 commit comments

Comments
 (0)
0