fix(dedupe): Track seen exceptions via attribute instead of ContextVar

sentry_sdk/client.py

@@ -24,7 +24,6 @@
 )
 from sentry_sdk.envelope import Envelope, Item
 from sentry_sdk.integrations import setup_integrations
-from sentry_sdk.integrations.dedupe import DedupeIntegration
 from sentry_sdk.monitor import Monitor
 from sentry_sdk.profiler.continuous_profiler import setup_continuous_profiler
 from sentry_sdk.serializer import serialize
@@ -594,13 +593,6 @@ def _prepare_event(
                     )
                     self.transport.record_lost_event(reason, 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
 
         return event

sentry_sdk/integrations/dedupe.py

@@ -1,24 +1,19 @@
-import weakref
-from contextvars import ContextVar
 from typing import TYPE_CHECKING
 
 import sentry_sdk
 from sentry_sdk.integrations import Integration
 from sentry_sdk.scope import add_global_event_processor
-from sentry_sdk.utils import logger
+from sentry_sdk.utils import capture_internal_exceptions, logger
 
 if TYPE_CHECKING:
-    from typing import Any, Optional
+    from typing import Optional
 
     from sentry_sdk._types import Event, Hint
 
 
 class DedupeIntegration(Integration):
     identifier = "dedupe"
 
-    def __init__(self) -> None:
-        self._last_seen: "ContextVar[Any]" = ContextVar("last-seen")
-
     @staticmethod
     def setup_once() -> None:
         @add_global_event_processor
@@ -34,30 +29,12 @@ def processor(event: "Event", hint: "Optional[Hint]") -> "Optional[Event]":
             if exc_info is None:
                 return event
 
-            last_seen = integration._last_seen.get(None)
-            if last_seen is not None:
-                # last_seen is either a weakref or the original instance
-                last_seen = (
-                    last_seen() if isinstance(last_seen, weakref.ref) else last_seen
-                )
-
             exc = exc_info[1]
-            if last_seen is exc:
+
+            if getattr(exc, "_handled_by_sentry", False):
                 logger.info("DedupeIntegration dropped duplicated error event %s", exc)
                 return None
-
-            # we can only weakref non builtin types
-            try:
-                integration._last_seen.set(weakref.ref(exc))
-            except TypeError:
-                integration._last_seen.set(exc)
-
-            return event
-
-    @staticmethod
-    def reset_last_seen() -> None:
-        integration = sentry_sdk.get_client().get_integration(DedupeIntegration)
-        if integration is None:
-            return
-
-        integration._last_seen.set(None)
+            else:
+                with capture_internal_exceptions():
+                    exc._handled_by_sentry = True
+                return event

tests/test_basics.py

@@ -1,9 +1,11 @@
 import datetime
+import gc
 import importlib
 import logging
 import os
 import sys
 import time
+import weakref
 from collections import Counter
 
 import pytest
@@ -27,6 +29,7 @@
     Integration,
     setup_integrations,
 )
+from sentry_sdk.integrations.dedupe import DedupeIntegration
 from sentry_sdk.integrations.logging import LoggingIntegration
 from sentry_sdk.integrations.stdlib import StdlibIntegration
 from sentry_sdk.scope import add_global_event_processor
@@ -611,19 +614,75 @@ def before_send(event, hint):
     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
+            raise ValueError("aha!")
+        except Exception:
+            capture_exception()
+
+    assert len(events) == 1
+
+
+def test_dedupe_drops_exception_when_seen_a_second_time(sentry_init, capture_events):
+    """
+    This test is intended to emulate behavior seen in frameworks like Django,
+    where an exception is raised in a view and then is re-raised in middleware.
+
+    In cases like that we don't want to send a second event for that exception.
+    """
+    sentry_init()
+    events = capture_events()
+
+    test = None
+    for _ in range(2):
+        try:
+            if test is None:
+                test = ValueError("foo")
+            raise test
         except Exception:
             capture_exception()
 
     assert len(events) == 1
 
 
+def test_dedupe_does_not_retain_builtin_exceptions(sentry_init):
+    """
+    There was a different approach that used to be used by DedupeIntegration
+    that used a weakref to hold a reference to a seen exception, and then do a comparison
+    on an incoming exception with that weakref to determine if it was a duplicate.
+
+    Built in exceptions such as ValueError couldn't be used with weakref, so we would instead
+    hold a strong reference to that exception. However, this led to memory leaks as described in
+    https://github.com/getsentry/sentry-python/issues/6094
+    """
+    sentry_init(default_integrations=False, integrations=[DedupeIntegration()])
+
+    class Payload:
+        pass
+
+    payload_ref = None
+
+    def fail():
+        nonlocal payload_ref
+        payload = Payload()
+        payload_ref = weakref.ref(payload)
+        raise ValueError("boom")
+
+    def capture():
+        try:
+            fail()
+        except ValueError as e:
+            sentry_sdk.capture_exception(e)
+
+    capture()
+
+    gc.collect()
+    assert payload_ref() is None
+
+
 def test_event_processor_drop_records_client_report(
     sentry_init, capture_events, capture_record_lost_event_calls
 ):