fix(browser): Ensure dedupe integration ignores non-errors (#7172)

getsentry · Feb 14, 2023 · 2518492 · 2518492
1 parent 7fad62f
commit 2518492
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 2 deletions.
diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts
@@ -23,6 +23,12 @@ export class Dedupe implements Integration {
    */
   public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
     const eventProcessor: EventProcessor = currentEvent => {
+      // We want to ignore any non-error type events, e.g. transactions or replays
+      // These should never be deduped, and also not be compared against as _previousEvent.
+      if (currentEvent.type) {
+        return currentEvent;
+      }
+
       const self = getCurrentHub().getIntegration(Dedupe);
       if (self) {
         // Juuust in case something goes wrong

diff --git a/packages/browser/test/integration/suites/api.js b/packages/browser/test/integration/suites/api.js
@@ -112,12 +112,26 @@ describe('API', function () {
 
       // Same exceptions, different stacktrace (different line number), don't dedupe
       throwSameConsecutiveErrors('bar');
+
+      // Same exception, with transaction in between, dedupe
+      throwError();
+      Sentry.captureEvent({
+        event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
+        message: 'someMessage',
+        transaction: 'wat',
+        type: 'transaction',
+      });
+      throwError();
     }).then(function (summary) {
+      // We have a length of one here since transactions don't go through beforeSend
+      // and we add events to summary in beforeSend
+      assert.equal(summary.events.length, 6);
       assert.match(summary.events[0].exception.values[0].value, /Exception no \d+/);
       assert.match(summary.events[1].exception.values[0].value, /Exception no \d+/);
       assert.equal(summary.events[2].exception.values[0].value, 'foo');
       assert.equal(summary.events[3].exception.values[0].value, 'bar');
       assert.equal(summary.events[4].exception.values[0].value, 'bar');
+      assert.equal(summary.events[5].exception.values[0].value, 'foo');
     });
   });
 

diff --git a/packages/integrations/src/dedupe.ts b/packages/integrations/src/dedupe.ts
@@ -23,6 +23,12 @@ export class Dedupe implements Integration {
    */
   public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
     const eventProcessor: EventProcessor = currentEvent => {
+      // We want to ignore any non-error type events, e.g. transactions or replays
+      // These should never be deduped, and also not be compared against as _previousEvent.
+      if (currentEvent.type) {
+        return currentEvent;
+      }
+
       const self = getCurrentHub().getIntegration(Dedupe);
       if (self) {
         // Juuust in case something goes wrong

diff --git a/packages/integrations/test/dedupe.test.ts b/packages/integrations/test/dedupe.test.ts
@@ -1,6 +1,6 @@
-import type { Event as SentryEvent, Exception, StackFrame, Stacktrace } from '@sentry/types';
+import type { Event as SentryEvent, EventProcessor, Exception, Hub, StackFrame, Stacktrace } from '@sentry/types';
 
-import { _shouldDropEvent } from '../src/dedupe';
+import { _shouldDropEvent, Dedupe } from '../src/dedupe';
 
 type EventWithException = SentryEvent & {
   exception: {
@@ -175,4 +175,48 @@ describe('Dedupe', () => {
       expect(_shouldDropEvent(eventB, eventC)).toBe(false);
     });
   });
+
+  describe('setupOnce', () => {
+    let dedupeFunc: EventProcessor;
+
+    beforeEach(function () {
+      const integration = new Dedupe();
+      const addGlobalEventProcessor = (callback: EventProcessor) => {
+        dedupeFunc = callback;
+      };
+
+      const getCurrentHub = () => {
+        return {
+          getIntegration() {
+            return integration;
+          },
+        } as unknown as Hub;
+      };
+
+      integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
+    });
+
+    it('ignores consecutive errors', () => {
+      expect(dedupeFunc(clone(exceptionEvent), {})).not.toBeNull();
+      expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull();
+      expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull();
+    });
+
+    it('ignores transactions between errors', () => {
+      expect(dedupeFunc(clone(exceptionEvent), {})).not.toBeNull();
+      expect(
+        dedupeFunc(
+          {
+            event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
+            message: 'someMessage',
+            transaction: 'wat',
+            type: 'transaction',
+          },
+          {},
+        ),
+      ).not.toBeNull();
+      expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull();
+      expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull();
+    });
+  });
 });