Skip to content

Commit 858554b

Browse files
kamilogorekkamilogorek
committed
fix: update logic for message-based fingerprint deduped events
1 parent 653f4de commit 858554b

File tree

3 files changed

+200
-48
lines changed

3 files changed

+200
-48
lines changed

‎packages/browser/src/integrations/dedupe.ts‎

Lines changed: 70 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -19,51 +19,45 @@ export class Dedupe implements Integration {
1919
*/
2020
public install(): void {
2121
getCurrentHub().configureScope((scope: Scope) => {
22-
scope.addEventProcessor(async (event: SentryEvent) => {
22+
scope.addEventProcessor(async (currentEvent: SentryEvent) => {
2323
// Juuust in case something goes wrong
2424
try {
25-
if (this.shouldDropEvent(event)) {
25+
if (this.shouldDropEvent(currentEvent, this.previousEvent)) {
2626
return null;
2727
}
2828
} catch (_oO) {
29-
return (this.previousEvent = event);
29+
return (this.previousEvent = currentEvent);
3030
}
3131

32-
return (this.previousEvent = event);
32+
return (this.previousEvent = currentEvent);
3333
});
3434
});
3535
}
3636

3737
/** JSDoc */
38-
public shouldDropEvent(event: SentryEvent): boolean {
39-
if (!this.previousEvent) {
38+
public shouldDropEvent(currentEvent: SentryEvent, previousEvent?: SentryEvent): boolean {
39+
if (!previousEvent) {
4040
return false;
4141
}
4242

43-
if (this.isSameMessage(event)) {
44-
logger.warn(
45-
`Event dropped due to being a duplicate of previous event (same message).\n Event: ${event.event_id}`,
46-
);
47-
return true;
48-
}
43+
if (this.isSameMessage(currentEvent, previousEvent)) {
44+
if (!this.isSameFingerprint(currentEvent, previousEvent)) {
45+
return false;
46+
}
4947

50-
if (this.isSameException(event)) {
51-
logger.warn(
52-
`Event dropped due to being a duplicate of previous event (same exception).\n Event: ${event.event_id}`,
53-
);
54-
return true;
55-
}
48+
if (!this.isSameStacktrace(currentEvent, previousEvent)) {
49+
return false;
50+
}
5651

57-
if (this.isSameStacktrace(event)) {
5852
logger.warn(
59-
`Event dropped due to being a duplicate of previous event (same stacktrace).\n Event: ${event.event_id}`,
53+
`Event dropped due to being a duplicate of previous event (same message).\n Event: ${currentEvent.event_id}`,
6054
);
6155
return true;
6256
}
6357

64-
if (this.isSameFingerprint(event)) {
58+
if (this.isSameException(currentEvent, previousEvent)) {
6559
logger.warn(
66-
`Event dropped due to being a duplicate of previous event (same fingerprint).\n Event: ${event.event_id}`,
60+
`Event dropped due to being a duplicate of previous event (same exception).\n Event: ${currentEvent.event_id}`,
6761
);
6862
return true;
6963
}
@@ -72,11 +66,22 @@ export class Dedupe implements Integration {
7266
}
7367

7468
/** JSDoc */
75-
private isSameMessage(event: SentryEvent): boolean {
76-
if (!this.previousEvent) {
69+
private isSameMessage(currentEvent: SentryEvent, previousEvent: SentryEvent): boolean {
70+
const currentMessage = currentEvent.message;
71+
const previousMessage = previousEvent.message;
72+
73+
// If no event has a message, they were both exceptions, so bail out
74+
if (!currentMessage && !previousMessage) {
75+
return false;
76+
}
77+
78+
// If only one event has a stacktrace, but not the other one, they are not the same
79+
if ((currentMessage && !previousMessage) || (!currentMessage && previousMessage)) {
7780
return false;
7881
}
79-
return !!(event.message && this.previousEvent.message && event.message === this.previousEvent.message);
82+
83+
// Otherwise, compare the two
84+
return currentMessage === previousMessage;
8085
}
8186

8287
/** JSDoc */
@@ -98,18 +103,29 @@ export class Dedupe implements Integration {
98103
}
99104

100105
/** JSDoc */
101-
private isSameStacktrace(event: SentryEvent): boolean {
102-
if (!this.previousEvent) {
106+
private isSameStacktrace(currentEvent: SentryEvent, previousEvent: SentryEvent): boolean {
107+
let currentFrames = this.getFramesFromEvent(currentEvent);
108+
let previousFrames = this.getFramesFromEvent(previousEvent);
109+
110+
// If no event has a fingerprint, they are assumed to be the same
111+
if (!currentFrames && !previousFrames) {
112+
return true;
113+
}
114+
115+
// If only one event has a stacktrace, but not the other one, they are not the same
116+
if ((currentFrames && !previousFrames) || (!currentFrames && previousFrames)) {
103117
return false;
104118
}
105119

106-
const previousFrames = this.getFramesFromEvent(this.previousEvent);
107-
const currentFrames = this.getFramesFromEvent(event);
120+
currentFrames = currentFrames as StackFrame[];
121+
previousFrames = previousFrames as StackFrame[];
108122

109-
if (!previousFrames || !currentFrames || previousFrames.length !== currentFrames.length) {
123+
// If number of frames differ, they are not the same
124+
if (previousFrames.length !== currentFrames.length) {
110125
return false;
111126
}
112127

128+
// Otherwise, compare the two
113129
for (let i = 0; i < previousFrames.length; i++) {
114130
const frameA = previousFrames[i];
115131
const frameB = currentFrames[i];
@@ -133,13 +149,9 @@ export class Dedupe implements Integration {
133149
}
134150

135151
/** JSDoc */
136-
private isSameException(event: SentryEvent): boolean {
137-
if (!this.previousEvent) {
138-
return false;
139-
}
140-
141-
const previousException = this.getExceptionFromEvent(this.previousEvent);
142-
const currentException = this.getExceptionFromEvent(event);
152+
private isSameException(currentEvent: SentryEvent, previousEvent: SentryEvent): boolean {
153+
const previousException = this.getExceptionFromEvent(previousEvent);
154+
const currentException = this.getExceptionFromEvent(currentEvent);
143155

144156
if (!previousException || !currentException) {
145157
return false;
@@ -149,16 +161,32 @@ export class Dedupe implements Integration {
149161
return false;
150162
}
151163

152-
return this.isSameStacktrace(event);
164+
return this.isSameStacktrace(currentEvent, previousEvent);
153165
}
154166

155167
/** JSDoc */
156-
private isSameFingerprint(event: SentryEvent): boolean {
157-
if (!this.previousEvent) {
168+
private isSameFingerprint(currentEvent: SentryEvent, previousEvent: SentryEvent): boolean {
169+
let currentFingerprint = currentEvent.fingerprint;
170+
let previousFingerprint = previousEvent.fingerprint;
171+
172+
// If no event has a fingerprint, they are assumed to be the same
173+
if (!currentFingerprint && !previousFingerprint) {
174+
return true;
175+
}
176+
177+
// If only one event has a fingerprint, but not the other one, they are not the same
178+
if ((currentFingerprint && !previousFingerprint) || (!currentFingerprint && previousFingerprint)) {
158179
return false;
159180
}
160181

161-
return Boolean(event.fingerprint && this.previousEvent.fingerprint) &&
162-
JSON.stringify(event.fingerprint) === JSON.stringify(this.previousEvent.fingerprint)
182+
currentFingerprint = currentFingerprint as string[];
183+
previousFingerprint = previousFingerprint as string[];
184+
185+
// Otherwise, compare the two
186+
try {
187+
return !!(currentFingerprint.join('') === previousFingerprint.join(''));
188+
} catch (_oO) {
189+
return false;
190+
}
163191
}
164192
}

‎packages/browser/test/integrations/dedupe.test.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { expect } from 'chai';
2+
import { Dedupe } from '../../src/integrations/dedupe';
3+
4+
function clone<T>(data: T): T {
5+
return JSON.parse(JSON.stringify(data));
6+
}
7+
8+
const dedupe = new Dedupe();
9+
const messageEvent = {
10+
fingerprint: ['MrSnuffles'],
11+
message: 'PickleRick',
12+
stacktrace: {
13+
frames: [
14+
{
15+
colno: 1,
16+
filename: 'filename.js',
17+
function: 'function',
18+
lineno: 1,
19+
},
20+
{
21+
colno: 2,
22+
filename: 'filename.js',
23+
function: 'function',
24+
lineno: 2,
25+
},
26+
],
27+
},
28+
};
29+
const exceptionEvent = {
30+
exception: {
31+
values: [
32+
{
33+
stacktrace: {
34+
frames: [
35+
{
36+
colno: 1,
37+
filename: 'filename.js',
38+
function: 'function',
39+
lineno: 1,
40+
},
41+
{
42+
colno: 2,
43+
filename: 'filename.js',
44+
function: 'function',
45+
lineno: 2,
46+
},
47+
],
48+
},
49+
type: 'SyntaxError',
50+
value: 'missing ( on line 10',
51+
},
52+
],
53+
},
54+
};
55+
56+
describe('Dedupe', () => {
57+
describe('shouldDropEvent(messageEvent)', () => {
58+
it('should not drop if there was no previous event', () => {
59+
const event = clone(messageEvent);
60+
expect(dedupe.shouldDropEvent(event)).equal(false);
61+
});
62+
63+
it('should not drop if events have different messages', () => {
64+
const eventA = clone(messageEvent);
65+
const eventB = clone(messageEvent);
66+
eventB.message = 'EvilMorty';
67+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(false);
68+
});
69+
70+
it('should not drop if events have same messages, but different stacktraces', () => {
71+
const eventA = clone(messageEvent);
72+
const eventB = clone(messageEvent);
73+
eventB.stacktrace.frames[0].colno = 1337;
74+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(false);
75+
});
76+
77+
it('should drop if there are two events with same messages and no fingerprints', () => {
78+
const eventA = clone(messageEvent);
79+
delete eventA.fingerprint;
80+
const eventB = clone(messageEvent);
81+
delete eventB.fingerprint;
82+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(true);
83+
});
84+
85+
it('should drop if there are two events with same messages and same fingerprints', () => {
86+
const eventA = clone(messageEvent);
87+
const eventB = clone(messageEvent);
88+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(true);
89+
});
90+
91+
it('should not drop if there are two events with same message but different fingerprints', () => {
92+
const eventA = clone(messageEvent);
93+
const eventB = clone(messageEvent);
94+
eventA.fingerprint = ['Birdperson'];
95+
const eventC = clone(messageEvent);
96+
delete eventC.fingerprint;
97+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(false);
98+
expect(dedupe.shouldDropEvent(eventA, eventC)).equal(false);
99+
expect(dedupe.shouldDropEvent(eventB, eventC)).equal(false);
100+
});
101+
});
102+
103+
describe('shouldDropEvent(exceptionEvent)', () => {
104+
it('should drop when events type, value and stacktrace are the same', () => {
105+
const event = clone(exceptionEvent);
106+
expect(dedupe.shouldDropEvent(event, event)).equal(true);
107+
});
108+
109+
it('should not drop if types are different', () => {
110+
const eventA = clone(exceptionEvent);
111+
const eventB = clone(exceptionEvent);
112+
eventB.exception.values[0].type = 'TypeError';
113+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(false);
114+
});
115+
116+
it('should not drop if values are different', () => {
117+
const eventA = clone(exceptionEvent);
118+
const eventB = clone(exceptionEvent);
119+
eventB.exception.values[0].value = 'Expected number, got string';
120+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(false);
121+
});
122+
123+
it('should not drop if stacktraces are different', () => {
124+
const eventA = clone(exceptionEvent);
125+
const eventB = clone(exceptionEvent);
126+
eventB.exception.values[0].stacktrace.frames[0].colno = 1337;
127+
expect(dedupe.shouldDropEvent(eventA, eventB)).equal(false);
128+
});
129+
});
130+
});

‎packages/raven-js/test/raven.test.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,15 +3859,10 @@ describe('Raven (private methods)', function() {
38593859
it('should return false for different fingerprints', function() {
38603860
var data = JSON.parse(JSON.stringify(Raven._lastData)); // copy
38613861
data.fingerprint = ['{{ default }}', 'grouping-identifier'];
3862-
38633862
assert.isFalse(Raven._isRepeatData(data));
3864-
38653863
Raven._lastData.fingerprint = ['{{ default }}', 'other-grouping-identifier'];
3866-
38673864
assert.isFalse(Raven._isRepeatData(data));
3868-
38693865
delete data.fingerprint;
3870-
38713866
assert.isFalse(Raven._isRepeatData(data));
38723867
});
38733868

@@ -3876,7 +3871,6 @@ describe('Raven (private methods)', function() {
38763871
Raven._lastData.fingerprint = ['{{ default }}', 'grouping-identifier'];
38773872
var data = JSON.parse(JSON.stringify(Raven._lastData)); // copy
38783873
data.message = 'the other thing broke';
3879-
38803874
assert.isFalse(Raven._isRepeatData(data));
38813875
});
38823876

0 commit comments

Comments
 (0)