Skip to content

Commit

Permalink
ref(various): Remove usage of deprecated event.stacktrace (#4885)
Browse files Browse the repository at this point in the history
Removes usage of deprecated `event.stacktrace` and updated the event interface to remove the deprecated property.
  • Loading branch information
timfish authored and AbhiPrasad committed May 30, 2022
1 parent 39a8901 commit 99498dc
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 117 deletions.
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 };
}
}

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 } }],
};
}
}

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;
}
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

0 comments on commit 99498dc

Please sign in to comment.