Reset DedupeIntegration's last-seen if before_send dropped the event (#4142)

Imagine an app throws an exception twice, from different places. The
first exception is dropped in the user's `before_send`. The second
exception is not. Should the second exception appear in Sentry?

The current state is that it won't, since `DedupeIntegration` will take
the first, dropped exception into account. When encountering the second
exception, it'll consider it a duplicate and will drop it, even though
the first exception never made it to Sentry.

In this PR, we reset `DedupeIntegration`'s `last-seen` if an event has
been dropped by `before_send`, ensuring that the next exception will be
reported.

Closes #371

---------

Co-authored-by: Anton Pirker <[email protected]>

Author: sentrivana

sentry_sdk/client.py

@@ -37,6 +37,7 @@
     ClientConstructor,
 )
 from sentry_sdk.integrations import _DEFAULT_INTEGRATIONS, setup_integrations
+from sentry_sdk.integrations.dedupe import DedupeIntegration
 from sentry_sdk.sessions import SessionFlusher
 from sentry_sdk.envelope import Envelope
 from sentry_sdk.profiler.continuous_profiler import setup_continuous_profiler
@@ -606,6 +607,14 @@ def _prepare_event(
                     self.transport.record_lost_event(
                         "before_send", data_category="error"
                     )
+
+                # If this is an exception, reset the DedupeIntegration. It still
+                # remembers the dropped exception as the last exception, meaning
+                # that if the same exception happens again and is not dropped
+                # in before_send, it'd get dropped by DedupeIntegration.
+                if event.get("exception"):
+                    DedupeIntegration.reset_last_seen()
+
             event = new_event
 
         before_send_transaction = self.options["before_send_transaction"]

sentry_sdk/integrations/dedupe.py

@@ -40,3 +40,12 @@ def processor(event, hint):
                 return None
             integration._last_seen.set(exc)
             return event
+
+    @staticmethod
+    def reset_last_seen():
+        # type: () -> None
+        integration = sentry_sdk.get_client().get_integration(DedupeIntegration)
+        if integration is None:
+            return
+
+        integration._last_seen.set(None)

tests/test_basics.py

@@ -710,6 +710,37 @@ def test_dedupe_event_processor_drop_records_client_report(
     assert lost_event_call == ("event_processor", "error", None, 1)
 
 
+def test_dedupe_doesnt_take_into_account_dropped_exception(sentry_init, capture_events):
+    # Two exceptions happen one after another. The first one is dropped in the
+    # user's before_send. The second one isn't.
+    # Originally, DedupeIntegration would drop the second exception. This test
+    # is making sure that that is no longer the case -- i.e., DedupeIntegration
+    # doesn't consider exceptions dropped in before_send.
+    count = 0
+
+    def before_send(event, hint):
+        nonlocal count
+        count += 1
+        if count == 1:
+            return None
+        return event
+
+    sentry_init(before_send=before_send)
+    events = capture_events()
+
+    exc = ValueError("aha!")
+    for _ in range(2):
+        # The first ValueError will be dropped by before_send. The second
+        # ValueError will be accepted by before_send, and should be sent to
+        # Sentry.
+        try:
+            raise exc
+        except Exception:
+            capture_exception()
+
+    assert len(events) == 1
+
+
 def test_event_processor_drop_records_client_report(
     sentry_init, capture_events, capture_record_lost_event_calls
 ):