Skip to content

ref: Deprecate top-level stacktrace #2222

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

Closed
wants to merge 2 commits into from
Closed
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

## Unreleased

- [browser] fix: Don't capture our own XHR events that somehow bubbled-up to global handler
- [browser] fix: Don't capture our own XHR events that somehow bubbled-up to global handler (#2221)
- [core] ref: Deprecate top-level stacktrace event attribute (#2214)

## 5.6.2

Expand Down
10 changes: 7 additions & 3 deletions packages/browser/src/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,13 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
if (this._options.attachStacktrace && hint && hint.syntheticException) {
const stacktrace = _computeStackTrace(hint.syntheticException);
const frames = prepareFramesForEvent(stacktrace.stack);
event.stacktrace = {
frames,
};
event.threads = [
{
stacktrace: {
frames,
},
},
];
}

return SyncPromise.resolve(event);
Expand Down
10 changes: 7 additions & 3 deletions packages/browser/src/parsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,13 @@ export function eventFromPlainObject(exception: {}, syntheticException: Error |
if (syntheticException) {
const stacktrace = _computeStackTrace(syntheticException);
const frames = prepareFramesForEvent(stacktrace.stack);
event.stacktrace = {
frames,
};
event.threads = [
{
stacktrace: {
frames,
},
},
];
}

return event;
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/integrations/inboundfilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ export class InboundFilters implements Integration {
/** JSDoc */
private _getEventFilterUrl(event: Event): string | null {
try {
// tslint:disable-next-line:deprecation
if (event.stacktrace) {
// tslint:disable:no-unsafe-any
const frames = (event as any).stacktrace.frames;
Expand All @@ -171,6 +172,11 @@ export class InboundFilters implements Integration {
const frames = (event as any).exception.values[0].stacktrace.frames;
return frames[frames.length - 1].filename;
}
if (event.threads) {
// tslint:disable:no-unsafe-any
const frames = (event as any).threads[0].stacktrace.frames;
return frames[frames.length - 1].filename;
}
return null;
} catch (oO) {
logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
Expand Down
23 changes: 4 additions & 19 deletions packages/integrations/src/dedupe.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types';

import { getFramesFromEvent } from './helpers';

/** Deduplication filter */
export class Dedupe implements Integration {
/**
Expand Down Expand Up @@ -86,27 +88,10 @@ export class Dedupe implements Integration {
return true;
}

/** JSDoc */
private _getFramesFromEvent(event: Event): StackFrame[] | undefined {
const exception = event.exception;

if (exception) {
try {
// @ts-ignore
return exception.values[0].stacktrace.frames;
} catch (_oO) {
return undefined;
}
} else if (event.stacktrace) {
return event.stacktrace.frames;
}
return undefined;
}

/** JSDoc */
private _isSameStacktrace(currentEvent: Event, previousEvent: Event): boolean {
let currentFrames = this._getFramesFromEvent(currentEvent);
let previousFrames = this._getFramesFromEvent(previousEvent);
let currentFrames = getFramesFromEvent(currentEvent);
let previousFrames = getFramesFromEvent(previousEvent);

// If no event has a fingerprint, they are assumed to be the same
if (!currentFrames && !previousFrames) {
Expand Down
27 changes: 27 additions & 0 deletions packages/integrations/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Event, StackFrame } from '@sentry/types';

/**
* Extract frames from the Event, independent of it's shape
*/
export function getFramesFromEvent(event: Event): StackFrame[] | undefined {
if (event.exception) {
try {
// @ts-ignore
return event.exception.values[0].stacktrace.frames;
} catch (_oO) {
return undefined;
}
// tslint:disable:deprecation
} else if (event.stacktrace) {
return event.stacktrace.frames;
// tslint:enable:deprecation
} else if (event.threads) {
try {
// @ts-ignore
return event.threads[0].stacktrace.frames;
} catch (_oO) {
return undefined;
}
}
return undefined;
}
29 changes: 6 additions & 23 deletions packages/integrations/src/rewriteframes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
import { basename, relative } from '@sentry/utils';

import { getFramesFromEvent } from './helpers';

type StackFrameIteratee = (frame: StackFrame) => StackFrame;

/** Rewrite event frames paths */
Expand Down Expand Up @@ -58,30 +60,11 @@ export class RewriteFrames implements Integration {

/** JSDoc */
public process(event: Event): Event {
const frames = this._getFramesFromEvent(event);
if (frames) {
for (const i in frames) {
// tslint:disable-next-line
frames[i] = this._iteratee(frames[i]);
}
const frames = getFramesFromEvent(event) || [];
for (const i in frames) {
// tslint:disable-next-line
frames[i] = this._iteratee(frames[i]);
}
return event;
}

/** JSDoc */
private _getFramesFromEvent(event: Event): StackFrame[] | undefined {
const exception = event.exception;

if (exception) {
try {
// tslint:disable-next-line:no-unsafe-any
return (exception as any).values[0].stacktrace.frames;
} catch (_oO) {
return undefined;
}
} else if (event.stacktrace) {
return event.stacktrace.frames;
}
return undefined;
}
}
10 changes: 3 additions & 7 deletions packages/integrations/src/transaction.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';

import { getFramesFromEvent } from './helpers';

/** Add node transaction to the event */
export class Transaction implements Integration {
/**
Expand Down Expand Up @@ -28,7 +30,7 @@ export class Transaction implements Integration {
* @inheritDoc
*/
public process(event: Event): Event {
const frames = this._getFramesFromEvent(event);
const frames = getFramesFromEvent(event) || [];

// use for loop so we don't have to reverse whole frames array
for (let i = frames.length - 1; i >= 0; i--) {
Expand All @@ -43,12 +45,6 @@ export class Transaction implements Integration {
return event;
}

/** JSDoc */
private _getFramesFromEvent(event: Event): StackFrame[] {
const exception = event.exception && event.exception.values && event.exception.values[0];
return (exception && exception.stacktrace && exception.stacktrace.frames) || [];
}

/** JSDoc */
private _getTransaction(frame: StackFrame): string {
return frame.module || frame.function ? `${frame.module || '?'}/${frame.function || '?'}` : '<unknown>';
Expand Down
100 changes: 100 additions & 0 deletions packages/integrations/test/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { getFramesFromEvent } from '../src/helpers';

describe('getFramesFromEvent', () => {
it('event.exception.values[0].stacktrace.frames', () => {
const frames = getFramesFromEvent({
exception: {
values: [
{
stacktrace: {
frames: [
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
],
},
},
],
},
});
expect(frames).toEqual([
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
]);
});

it('event.stacktrace.frames', () => {
const frames = getFramesFromEvent({
stacktrace: {
frames: [
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
],
},
});
expect(frames).toEqual([
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
]);
});

it('event.threads[0].stacktrace.frames', () => {
const frames = getFramesFromEvent({
threads: [
{
stacktrace: {
frames: [
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
],
},
},
],
});
expect(frames).toEqual([
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
]);
});

it('no frames', () => {
const frames = getFramesFromEvent({});
expect(frames).toEqual(undefined);
});

it('broken frames', () => {
const exceptionNoFrames = getFramesFromEvent({
exception: {
values: [],
},
});
const threadsNoFrames = getFramesFromEvent({
threads: [],
});
expect(exceptionNoFrames).toEqual(undefined);
expect(threadsNoFrames).toEqual(undefined);
});
});
39 changes: 39 additions & 0 deletions packages/integrations/test/rewriteframes.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// tslint:disable:deprecation

import { Event, StackFrame } from '@sentry/types';

import { RewriteFrames } from '../src/rewriteframes';

let rewriteFrames: RewriteFrames;
let messageEvent: Event;
let exceptionEvent: Event;
let threadEvent: Event;

describe('RewriteFrames', () => {
beforeEach(() => {
Expand Down Expand Up @@ -38,6 +41,22 @@ describe('RewriteFrames', () => {
],
},
};
threadEvent = {
threads: [
{
stacktrace: {
frames: [
{
filename: '/www/src/app/file1.js',
},
{
filename: '/www/src/app/file2.js',
},
],
},
},
],
};
});

describe('default iteratee appends basename to `app:///` if frame starts with `/`', () => {
Expand All @@ -56,6 +75,12 @@ describe('RewriteFrames', () => {
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js');
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js');
});

it('transforms threadEvent frames', () => {
const event = rewriteFrames.process(threadEvent);
expect(event.threads![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js');
expect(event.threads![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js');
});
});

describe('can use custom root to perform `relative` on filepaths', () => {
Expand All @@ -76,6 +101,12 @@ describe('RewriteFrames', () => {
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js');
});

it('transforms threadEvent frames', async () => {
const event = rewriteFrames.process(threadEvent);
expect(event.threads![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
expect(event.threads![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js');
});
});

describe('can use custom iteratee', () => {
Expand Down Expand Up @@ -103,5 +134,13 @@ describe('RewriteFrames', () => {
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js');
expect(event.exception!.values![0].stacktrace!.frames![1].function).toEqual('whoops');
});

it('transforms threadEvent frames', async () => {
const event = rewriteFrames.process(threadEvent);
expect(event.threads![0].stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
expect(event.threads![0].stacktrace!.frames![0].function).toEqual('whoops');
expect(event.threads![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js');
expect(event.threads![0].stacktrace!.frames![1].function).toEqual('whoops');
});
});
});
10 changes: 7 additions & 3 deletions packages/node/src/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,13 @@ export class NodeBackend extends BaseBackend<NodeOptions> {
if (this._options.attachStacktrace && hint && hint.syntheticException) {
const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : [];
parseStack(stack, this._options).then(frames => {
event.stacktrace = {
frames: prepareFramesForEvent(frames),
};
event.threads = [
{
stacktrace: {
frames: prepareFramesForEvent(frames),
},
},
];
resolve(event);
});
} else {
Expand Down
Loading