fix(django): Delay patching of DRF (getsentry#398)

untitaker · web-flow · commit cd4585a3837c · 2019-06-23T16:42:46.000+02:00
* fix(django): Delay patching of DRF Fix getsentry#397 * fix: Update doc comments * fix: Workaround for Django 1.6
diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py
@@ -2,10 +2,10 @@
 from __future__ import absolute_import
 
 import sys
+import threading
 import weakref
 
 from django import VERSION as DJANGO_VERSION  # type: ignore
-from django.db.models.query import QuerySet  # type: ignore
 from django.core import signals  # type: ignore
 
 if False:
@@ -116,6 +116,8 @@ def sentry_patched_get_response(self, request):
             hub = Hub.current
             integration = hub.get_integration(DjangoIntegration)
             if integration is not None:
+                _patch_drf()
+
                 with hub.configure_scope() as scope:
                     scope.add_event_processor(
                         _make_event_processor(weakref.ref(request), integration)
@@ -124,25 +126,6 @@ def sentry_patched_get_response(self, request):
 
         BaseHandler.get_response = sentry_patched_get_response
 
-        try:
-            from rest_framework.views import APIView  # type: ignore
-        except ImportError:
-            pass
-        else:
-            # DRF's request type (which wraps the Django request and proxies
-            # all attrs) has some attributes such as `data` which buffer
-            # request data.  We want to use those in the RequestExtractor to
-            # get body data more reliably.
-            old_drf_initial = APIView.initial
-
-            def sentry_patched_drf_initial(self, request, *args, **kwargs):
-                with capture_internal_exceptions():
-                    request._request._sentry_drf_request_backref = weakref.ref(request)
-                    pass
-                return old_drf_initial(self, request, *args, **kwargs)
-
-            APIView.initial = sentry_patched_drf_initial
-
         signals.got_request_exception.connect(_got_request_exception)
 
         @add_global_event_processor
@@ -190,6 +173,17 @@ def process_django_templates(event, hint):
 
         @add_global_repr_processor
         def _django_queryset_repr(value, hint):
+            try:
+                # Django 1.6 can fail to import `QuerySet` when Django settings
+                # have not yet been initialized.
+                #
+                # If we fail to import, return `NotImplemented`. It's at least
+                # unlikely that we have a query set in `value` when importing
+                # `QuerySet` fails.
+                from django.db.models.query import QuerySet  # type: ignore
+            except Exception:
+                return NotImplemented
+
             if not isinstance(value, QuerySet) or value._result_cache:
                 return NotImplemented
 
@@ -205,6 +199,63 @@ def _django_queryset_repr(value, hint):
             )
 
 
+_DRF_PATCHED = False
+_DRF_PATCH_LOCK = threading.Lock()
+
+
+def _patch_drf():
+    """
+    Patch Django Rest Framework for more/better request data. DRF's request
+    type is a wrapper around Django's request type. The attribute we're
+    interested in is `request.data`, which is a cached property containing a
+    parsed request body. Reading a request body from that property is more
+    reliable than reading from any of Django's own properties, as those don't
+    hold payloads in memory and therefore can only be accessed once.
+
+    We patch the Django request object to include a weak backreference to the
+    DRF request object, such that we can later use either in
+    `DjangoRequestExtractor`.
+
+    This function is not called directly on SDK setup, because importing almost
+    any part of Django Rest Framework will try to access Django settings (where
+    `sentry_sdk.init()` might be called from in the first place). Instead we
+    run this function on every request and do the patching on the first
+    request.
+    """
+
+    global _DRF_PATCHED
+
+    if _DRF_PATCHED:
+        # Double-checked locking
+        return
+
+    with _DRF_PATCH_LOCK:
+        if _DRF_PATCHED:
+            return
+
+        # We set this regardless of whether the code below succeeds or fails.
+        # There is no point in trying to patch again on the next request.
+        _DRF_PATCHED = True
+
+        with capture_internal_exceptions():
+            try:
+                from rest_framework.views import APIView  # type: ignore
+            except ImportError:
+                pass
+            else:
+                old_drf_initial = APIView.initial
+
+                def sentry_patched_drf_initial(self, request, *args, **kwargs):
+                    with capture_internal_exceptions():
+                        request._request._sentry_drf_request_backref = weakref.ref(
+                            request
+                        )
+                        pass
+                    return old_drf_initial(self, request, *args, **kwargs)
+
+                APIView.initial = sentry_patched_drf_initial
+
+
 def _make_event_processor(weak_request, integration):
     # type: (Callable[[], WSGIRequest], DjangoIntegration) -> Callable
     def event_processor(event, hint):
diff --git a/tests/integrations/django/myapp/settings.py b/tests/integrations/django/myapp/settings.py
@@ -10,6 +10,15 @@
 https://docs.djangoproject.com/en/2.0/ref/settings/
 """
 
+
+# We shouldn't access settings while setting up integrations. Initialize SDK
+# here to provoke any errors that might occur.
+import sentry_sdk
+from sentry_sdk.integrations.django import DjangoIntegration
+
+sentry_sdk.init(integrations=[DjangoIntegration()])
+
+
 import os
 
 try:
diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py
@@ -179,7 +179,7 @@ def test_sql_queries(sentry_init, capture_events):
 
     event, = events
 
-    crumb, = event["breadcrumbs"]
+    crumb = event["breadcrumbs"][-1]
 
     assert crumb["message"] == """SELECT count(*) FROM people_person WHERE foo = 123"""
 
@@ -297,7 +297,7 @@ def test_sql_queries_large_params(sentry_init, capture_events):
 
     event, = events
 
-    crumb, = event["breadcrumbs"]
+    crumb = event["breadcrumbs"][-1]
     assert crumb["message"] == (
         "SELECT count(*) FROM people_person WHERE foo = '%s... and bar IS NULL"
         % ("x" * 124,)
