Skip to content
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

Remove usage of deprecated event.stacktrace #4885

Merged
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
7 changes: 5 additions & 2 deletions packages/browser/src/eventbuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ export function eventFromPlainObject(
if (syntheticException) {
const frames = parseStackFrames(syntheticException);
if (frames.length) {
event.stacktrace = { frames };
// event.exception.values[0] has been set above
(event.exception as { values: Exception[] }).values[0].stacktrace = { frames };
AbhiPrasad marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -273,7 +274,9 @@ export function eventFromString(input: string, syntheticException?: Error, attac
if (attachStacktrace && syntheticException) {
const frames = parseStackFrames(syntheticException);
if (frames.length) {
event.stacktrace = { frames };
event.exception = {
values: [{ value: input, stacktrace: { frames } }],
timfish marked this conversation as resolved.
Show resolved Hide resolved
};
}
}

Expand Down
2 changes: 0 additions & 2 deletions packages/browser/src/integrations/dedupe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,6 @@ function _getFramesFromEvent(event: Event): StackFrame[] | undefined {
} catch (_oO) {
return undefined;
}
} else if (event.stacktrace) {
return event.stacktrace.frames;
timfish marked this conversation as resolved.
Show resolved Hide resolved
}
return undefined;
}
4 changes: 2 additions & 2 deletions packages/browser/test/integration/suites/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ describe('API', function () {
return runInSandbox(sandbox, function () {
throwNonError();
}).then(function (summary) {
assert.isAtLeast(summary.events[0].stacktrace.frames.length, 1);
assert.isAtMost(summary.events[0].stacktrace.frames.length, 3);
assert.isAtLeast(summary.events[0].exception.values[0].stacktrace.frames.length, 1);
assert.isAtMost(summary.events[0].exception.values[0].stacktrace.frames.length, 3);
});
});

Expand Down
13 changes: 0 additions & 13 deletions packages/browser/test/integration/suites/breadcrumbs.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ describe('breadcrumbs', function () {
assert.equal(summary.breadcrumbHints.length, 1);
assert.equal(summary.breadcrumbHints[0].name, 'click');
assert.equal(summary.breadcrumbHints[0].event.target.tagName, 'INPUT');
// There should be no expection, if there is one it means we threw it
assert.isUndefined(summary.events[0].exception);
}
});
});
Expand Down Expand Up @@ -265,9 +263,6 @@ describe('breadcrumbs', function () {

assert.equal(summary.breadcrumbs[1].category, 'ui.input');
assert.equal(summary.breadcrumbs[1].message, 'body > form#foo-form > input[name="foo"]');

// There should be no expection, if there is one it means we threw it
assert.isUndefined(summary.events[0].exception);
}
});
});
Expand All @@ -288,8 +283,6 @@ describe('breadcrumbs', function () {
// The async loader doesn't wrap event listeners, but we should receive the event without breadcrumbs
assert.lengthOf(summary.events, 1);
} else {
// There should be no expection, if there is one it means we threw it
assert.isUndefined(summary.events[0].exception);
assert.equal(summary.breadcrumbs.length, 0);
}
});
Expand All @@ -309,8 +302,6 @@ describe('breadcrumbs', function () {
// The async loader doesn't wrap event listeners, but we should receive the event without breadcrumbs
assert.lengthOf(summary.events, 1);
} else {
// There should be no expection, if there is one it means we threw it
assert.isUndefined(summary.events[0].exception);
assert.equal(summary.breadcrumbs.length, 0);
}
});
Expand Down Expand Up @@ -472,7 +463,6 @@ describe('breadcrumbs', function () {
assert.equal(summary.breadcrumbs[0].message, 'body > form#foo-form > input[name="foo"]');
assert.equal(summary.breadcrumbHints[0].global, false);
assert.equal(summary.breadcrumbHints[1].global, false);
assert.isUndefined(summary.events[0].exception);
}
});
});
Expand Down Expand Up @@ -507,7 +497,6 @@ describe('breadcrumbs', function () {
assert.equal(summary.breadcrumbs[0].message, 'body > form#foo-form > input[name="foo"]');
assert.equal(summary.breadcrumbHints[0].global, false);
assert.equal(summary.breadcrumbHints[1].global, false);
assert.isUndefined(summary.events[0].exception);
}
});
});
Expand Down Expand Up @@ -538,7 +527,6 @@ describe('breadcrumbs', function () {
assert.equal(summary.breadcrumbs[1].message, 'body > form#foo-form > div.contenteditable');
assert.equal(summary.breadcrumbHints[0].global, false);
assert.equal(summary.breadcrumbHints[1].global, false);
assert.isUndefined(summary.events[0].exception);
}
});
});
Expand Down Expand Up @@ -706,7 +694,6 @@ describe('breadcrumbs', function () {
assert.equal(summary.breadcrumbs.length, 2);
assert.equal(summary.breadcrumbHints[0].global, true);
assert.equal(summary.breadcrumbHints[1].global, true);
assert.isUndefined(summary.events[0].exception);
}
});
});
Expand Down
3 changes: 0 additions & 3 deletions packages/core/src/integrations/inboundfilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,6 @@ function _getLastValidUrl(frames: StackFrame[] = []): string | null {

function _getEventFilterUrl(event: Event): string | null {
try {
if (event.stacktrace) {
return _getLastValidUrl(event.stacktrace.frames);
}
let frames;
try {
// @ts-ignore we only care about frames if the whole thing here is defined
Expand Down
84 changes: 54 additions & 30 deletions packages/core/test/lib/integrations/inboundfilters.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EventProcessor } from '@sentry/types';
import { Event, EventProcessor } from '@sentry/types';

import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations/inboundfilters';

Expand Down Expand Up @@ -52,50 +52,68 @@ function createInboundFiltersEventProcessor(

// Fixtures

const MESSAGE_EVENT = {
const MESSAGE_EVENT: Event = {
message: 'captureMessage',
};

const MESSAGE_EVENT_2 = {
const MESSAGE_EVENT_2: Event = {
message: 'captureMessageSomething',
};

const MESSAGE_EVENT_WITH_STACKTRACE = {
const MESSAGE_EVENT_WITH_STACKTRACE: Event = {
message: 'wat',
stacktrace: {
// Frames are always in the reverse order, as this is how Sentry expect them to come.
// Frame that crashed is the last one, the one from awesome-analytics
frames: [
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://awesome-analytics.io/some/file.js' },
exception: {
values: [
{
stacktrace: {
// Frames are always in the reverse order, as this is how Sentry expect them to come.
// Frame that crashed is the last one, the one from awesome-analytics
frames: [
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://awesome-analytics.io/some/file.js' },
],
},
},
],
},
};

const MESSAGE_EVENT_WITH_ANON_LAST_FRAME = {
const MESSAGE_EVENT_WITH_ANON_LAST_FRAME: Event = {
message: 'any',
stacktrace: {
frames: [
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://awesome-analytics.io/some/file.js' },
{ filename: '<anonymous>' },
exception: {
values: [
{
stacktrace: {
frames: [
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://awesome-analytics.io/some/file.js' },
{ filename: '<anonymous>' },
],
},
},
],
},
};

const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME = {
const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME: Event = {
message: 'any',
stacktrace: {
frames: [
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://awesome-analytics.io/some/file.js' },
{ filename: '[native code]' },
exception: {
values: [
{
stacktrace: {
frames: [
{ filename: 'https://our-side.com/js/bundle.js' },
{ filename: 'https://awesome-analytics.io/some/file.js' },
{ filename: '[native code]' },
],
},
},
],
},
};

const EXCEPTION_EVENT = {
const EXCEPTION_EVENT: Event = {
exception: {
values: [
{
Expand All @@ -106,7 +124,7 @@ const EXCEPTION_EVENT = {
},
};

const EXCEPTION_EVENT_WITH_FRAMES = {
const EXCEPTION_EVENT_WITH_FRAMES: Event = {
exception: {
values: [
{
Expand All @@ -124,7 +142,7 @@ const EXCEPTION_EVENT_WITH_FRAMES = {
},
};

const SENTRY_EVENT = {
const SENTRY_EVENT: Event = {
exception: {
values: [
{
Expand All @@ -135,7 +153,7 @@ const SENTRY_EVENT = {
},
};

const SCRIPT_ERROR_EVENT = {
const SCRIPT_ERROR_EVENT: Event = {
exception: {
values: [
{
Expand All @@ -146,9 +164,15 @@ const SCRIPT_ERROR_EVENT = {
},
};

const MALFORMED_EVENT = {
stacktrace: {
frames: undefined,
const MALFORMED_EVENT: Event = {
exception: {
values: [
{
stacktrace: {
frames: undefined,
},
},
],
},
};

Expand Down
2 changes: 0 additions & 2 deletions packages/integrations/src/dedupe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,6 @@ function _getFramesFromEvent(event: Event): StackFrame[] | undefined {
} catch (_oO) {
return undefined;
}
} else if (event.stacktrace) {
return event.stacktrace.frames;
}
return undefined;
}
16 changes: 0 additions & 16 deletions packages/integrations/src/rewriteframes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ export class RewriteFrames implements Integration {
processedEvent = this._processExceptionsEvent(processedEvent);
}

if (originalEvent.stacktrace) {
processedEvent = this._processStacktraceEvent(processedEvent);
}

return processedEvent;
}

Expand Down Expand Up @@ -110,18 +106,6 @@ export class RewriteFrames implements Integration {
}
}

/** JSDoc */
private _processStacktraceEvent(event: Event): Event {
try {
return {
...event,
stacktrace: this._processStacktrace(event.stacktrace),
};
} catch (_oO) {
return event;
}
}

/** JSDoc */
private _processStacktrace(stacktrace?: Stacktrace): Stacktrace {
return {
Expand Down
40 changes: 25 additions & 15 deletions packages/integrations/test/dedupe.test.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,40 @@
import { Event } from '@sentry/types';

import { _shouldDropEvent } from '../src/dedupe';

/** JSDoc */
function clone<T>(data: T): T {
return JSON.parse(JSON.stringify(data));
}

const messageEvent = {
const messageEvent: Event = {
fingerprint: ['MrSnuffles'],
message: 'PickleRick',
stacktrace: {
frames: [
{
colno: 1,
filename: 'filename.js',
function: 'function',
lineno: 1,
},
exception: {
values: [
{
colno: 2,
filename: 'filename.js',
function: 'function',
lineno: 2,
value: 'PickleRick',
stacktrace: {
frames: [
{
colno: 1,
filename: 'filename.js',
function: 'function',
lineno: 1,
},
{
colno: 2,
filename: 'filename.js',
function: 'function',
lineno: 2,
},
],
},
},
],
},
};
const exceptionEvent = {
const exceptionEvent: Event = {
exception: {
values: [
{
Expand Down Expand Up @@ -64,13 +73,14 @@ describe('Dedupe', () => {
const eventA = clone(messageEvent);
const eventB = clone(messageEvent);
eventB.message = 'EvilMorty';
eventB.exception.values[0].value = 'EvilMorty';
expect(_shouldDropEvent(eventA, eventB)).toBe(false);
});

it('should not drop if events have same messages, but different stacktraces', () => {
const eventA = clone(messageEvent);
const eventB = clone(messageEvent);
eventB.stacktrace.frames[0].colno = 1337;
eventB.exception.values[0].stacktrace.frames[0].colno = 1337;
expect(_shouldDropEvent(eventA, eventB)).toBe(false);
});

Expand Down
Loading