fix: Implement repr hooks, refactor cycle breaking (getsentry#247)

untitaker · web-flow · commit 30eddcfc8102 · 2019-02-03T18:43:25.000+01:00
* fix: Implement repr hooks, refactor cycle breaking

* fix: Only run queryset_repr if integration is active

* fix: Revert unrelated changes

* fix: Do not use TLS if not a queryset

* fix: Fix import

* fix: Do not check for current hub in hook

* test: Test new repr hooks in django

* fix: Remove dead code

* fix: Return unicode string

* fix: Imports

* fix: Linting
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
@@ -12,7 +12,6 @@
     get_type_name,
     capture_internal_exceptions,
     current_stacktrace,
-    break_cycles,
     logger,
 )
 from sentry_sdk.transport import make_transport
@@ -122,7 +121,6 @@ def _prepare_event(self, event, hint, scope):
         # Postprocess the event here so that annotated types do
         # generally not surface in before_send
         if event is not None:
-            event = break_cycles(event)
             strip_event_mut(event)
             event = flatten_metadata(event)
             event = convert_types(event)
diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py
@@ -5,6 +5,7 @@
 import weakref
 
 from django import VERSION as DJANGO_VERSION
+from django.db.models.query import QuerySet
 from django.core import signals
 
 try:
@@ -31,6 +32,7 @@ def sql_to_string(sql):
 from sentry_sdk.hub import _should_send_default_pii
 from sentry_sdk.scope import add_global_event_processor
 from sentry_sdk.utils import (
+    add_global_repr_processor,
     capture_internal_exceptions,
     event_from_exception,
     safe_repr,
@@ -140,6 +142,22 @@ def process_django_templates(event, hint):
 
             return event
 
+        @add_global_repr_processor
+        def _django_queryset_repr(value, hint):
+            if not isinstance(value, QuerySet) or value._result_cache:
+                return NotImplemented
+
+            # Do not call Hub.get_integration here. It is intentional that
+            # running under a new hub does not suddenly start executing
+            # querysets. This might be surprising to the user but it's likely
+            # less annoying.
+
+            return u"<%s from %s at 0x%x>" % (
+                value.__class__.__name__,
+                value.__module__,
+                id(value),
+            )
+
 
 def _make_event_processor(weak_request, integration):
     def event_processor(event, hint):
diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py
@@ -3,7 +3,7 @@
 from functools import wraps
 from itertools import chain
 
-from sentry_sdk.utils import logger, capture_internal_exceptions
+from sentry_sdk.utils import logger, capture_internal_exceptions, object_to_json
 
 
 global_event_processors = []
@@ -165,7 +165,7 @@ def _drop(event, cause, ty):
             event["fingerprint"] = self._fingerprint
 
         if self._extras:
-            event.setdefault("extra", {}).update(self._extras)
+            event.setdefault("extra", {}).update(object_to_json(self._extras))
 
         if self._tags:
             event.setdefault("tags", {}).update(self._tags)
diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py
@@ -34,6 +34,13 @@
 CYCLE_MARKER = object()
 
 
+global_repr_processors = []
+
+
+def add_global_repr_processor(processor):
+    global_repr_processors.append(processor)
+
+
 def _get_debug_hub():
     # This function is replaced by debug.py
     pass
@@ -307,22 +314,40 @@ def safe_repr(value):
         return u"<broken repr>"
 
 
-def object_to_json(obj):
-    def _walk(obj, depth):
-        if depth < 4:
+def object_to_json(obj, remaining_depth=4, memo=None):
+    if memo is None:
+        memo = Memo()
+    if memo.memoize(obj):
+        return CYCLE_MARKER
+
+    try:
+        if remaining_depth > 0:
+            hints = {"memo": memo, "remaining_depth": remaining_depth}
+            for processor in global_repr_processors:
+                with capture_internal_exceptions():
+                    result = processor(obj, hints)
+                    if result is not NotImplemented:
+                        return result
+
             if isinstance(obj, (list, tuple)):
                 # It is not safe to iterate over another sequence types as this may raise errors or
                 # bring undesired side-effects (e.g. Django querysets are executed during iteration)
-                return [_walk(x, depth + 1) for x in obj]
-            if isinstance(obj, Mapping):
-                return {safe_str(k): _walk(v, depth + 1) for k, v in obj.items()}
+                return [
+                    object_to_json(x, remaining_depth=remaining_depth - 1, memo=memo)
+                    for x in obj
+                ]
 
-        if obj is CYCLE_MARKER:
-            return obj
+            if isinstance(obj, Mapping):
+                return {
+                    safe_str(k): object_to_json(
+                        v, remaining_depth=remaining_depth - 1, memo=memo
+                    )
+                    for k, v in obj.items()
+                }
 
         return safe_repr(obj)
-
-    return _walk(break_cycles(obj), 0)
+    finally:
+        memo.unmemoize(obj)
 
 
 def extract_locals(frame):
@@ -645,23 +670,18 @@ def strip_frame_mut(frame):
         frame["vars"] = strip_databag(frame["vars"])
 
 
-def break_cycles(obj, memo=None):
-    if memo is None:
-        memo = {}
-    if id(obj) in memo:
-        return CYCLE_MARKER
-    memo[id(obj)] = obj
+class Memo(object):
+    def __init__(self):
+        self._inner = {}
 
-    try:
-        if isinstance(obj, Mapping):
-            return {k: break_cycles(v, memo) for k, v in obj.items()}
-        if isinstance(obj, (list, tuple)):
-            # It is not safe to iterate over another sequence types as this may raise errors or
-            # bring undesired side-effects (e.g. Django querysets are executed during iteration)
-            return [break_cycles(v, memo) for v in obj]
-        return obj
-    finally:
-        del memo[id(obj)]
+    def memoize(self, obj):
+        if id(obj) in self._inner:
+            return True
+        self._inner[id(obj)] = obj
+        return False
+
+    def unmemoize(self, obj):
+        self._inner.pop(id(obj), None)
 
 
 def convert_types(obj):
diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py
@@ -3,6 +3,7 @@
 import pytest
 
 from werkzeug.test import Client
+from django.contrib.auth.models import User
 from django.core.management import execute_from_command_line
 from django.db.utils import OperationalError
 
@@ -12,7 +13,7 @@
 except ImportError:
     from django.core.urlresolvers import reverse
 
-from sentry_sdk import capture_message
+from sentry_sdk import capture_message, capture_exception
 from sentry_sdk.integrations.django import DjangoIntegration
 
 from tests.integrations.django.myapp.wsgi import application
@@ -101,6 +102,28 @@ def test_user_captured(sentry_init, client, capture_events):
     }
 
 
+@pytest.mark.django_db
+def test_queryset_repr(sentry_init, capture_events):
+    sentry_init(integrations=[DjangoIntegration()])
+    events = capture_events()
+    User.objects.create_user("john", "lennon@thebeatles.com", "johnpassword")
+
+    try:
+        my_queryset = User.objects.all()  # noqa
+        1 / 0
+    except Exception:
+        capture_exception()
+
+    event, = events
+
+    exception, = event["exception"]["values"]
+    assert exception["type"] == "ZeroDivisionError"
+    frame, = exception["stacktrace"]["frames"]
+    assert frame["vars"]["my_queryset"].startswith(
+        "<QuerySet from django.db.models.query at"
+    )
+
+
 def test_custom_error_handler_request_context(sentry_init, client, capture_events):
     sentry_init(integrations=[DjangoIntegration()])
     events = capture_events()
diff --git a/tests/test_client.py b/tests/test_client.py
@@ -350,7 +350,7 @@ def test_cyclic_data(sentry_init, capture_events):
     event, = events
 
     data = event["extra"]["foo"]
-    assert data == {"not_cyclic2": "", "not_cyclic": "", "is_cyclic": "<cyclic>"}
+    assert data == {"not_cyclic2": "''", "not_cyclic": "''", "is_cyclic": "<cyclic>"}
 
 
 def test_databag_stripping(sentry_init, capture_events):
