fix(django): Do not patch resolver_match (#842)

Co-authored-by: sentry-bot <markus+ghbot@sentry.io>

Author: untitaker

sentry_sdk/integrations/django/__init__.py

@@ -39,7 +39,7 @@
 from sentry_sdk.integrations.django.transactions import LEGACY_RESOLVER
 from sentry_sdk.integrations.django.templates import get_template_frame_from_exception
 from sentry_sdk.integrations.django.middleware import patch_django_middlewares
-from sentry_sdk.integrations.django.views import patch_resolver
+from sentry_sdk.integrations.django.views import patch_views
 
 
 if MYPY:
@@ -200,7 +200,7 @@ def _django_queryset_repr(value, hint):
 
         _patch_channels()
         patch_django_middlewares()
-        patch_resolver()
+        patch_views()
 
 
 _DRF_PATCHED = False

sentry_sdk/integrations/django/views.py

@@ -5,63 +5,39 @@
 if MYPY:
     from typing import Any
 
-    from django.urls.resolvers import ResolverMatch
 
-
-def patch_resolver():
+def patch_views():
     # type: () -> None
-    try:
-        from django.urls.resolvers import URLResolver
-    except ImportError:
-        try:
-            from django.urls.resolvers import RegexURLResolver as URLResolver
-        except ImportError:
-            from django.core.urlresolvers import RegexURLResolver as URLResolver
 
+    from django.core.handlers.base import BaseHandler
     from sentry_sdk.integrations.django import DjangoIntegration
 
-    old_resolve = URLResolver.resolve
-
-    def resolve(self, path):
-        # type: (URLResolver, Any) -> ResolverMatch
-        hub = Hub.current
-        integration = hub.get_integration(DjangoIntegration)
-
-        if integration is None or not integration.middleware_spans:
-            return old_resolve(self, path)
+    old_make_view_atomic = BaseHandler.make_view_atomic
 
-        return _wrap_resolver_match(hub, old_resolve(self, path))
+    @_functools.wraps(old_make_view_atomic)
+    def sentry_patched_make_view_atomic(self, *args, **kwargs):
+        # type: (Any, *Any, **Any) -> Any
+        callback = old_make_view_atomic(self, *args, **kwargs)
 
-    URLResolver.resolve = resolve
+        # XXX: The wrapper function is created for every request. Find more
+        # efficient way to wrap views (or build a cache?)
 
+        hub = Hub.current
+        integration = hub.get_integration(DjangoIntegration)
 
-def _wrap_resolver_match(hub, resolver_match):
-    # type: (Hub, ResolverMatch) -> ResolverMatch
-
-    # XXX: The wrapper function is created for every request. Find more
-    # efficient way to wrap views (or build a cache?)
-
-    old_callback = resolver_match.func
+        if integration is not None and integration.middleware_spans:
 
-    # Explicitly forward `csrf_exempt` in case it is not an attribute in
-    # callback.__dict__, but rather a class attribute (on a class
-    # implementing __call__) such as this:
-    #
-    #     class Foo(object):
-    #         csrf_exempt = True
-    #
-    #         def __call__(self, request): ...
-    #
-    # We have had this in the Sentry codebase (for no good reason, but
-    # nevertheless we broke user code)
-    assigned = _functools.WRAPPER_ASSIGNMENTS + ("csrf_exempt",)
+            @_functools.wraps(callback)
+            def sentry_wrapped_callback(request, *args, **kwargs):
+                # type: (Any, *Any, **Any) -> Any
+                with hub.start_span(
+                    op="django.view", description=request.resolver_match.view_name
+                ):
+                    return callback(request, *args, **kwargs)
 
-    @_functools.wraps(old_callback, assigned=assigned)
-    def callback(*args, **kwargs):
-        # type: (*Any, **Any) -> Any
-        with hub.start_span(op="django.view", description=resolver_match.view_name):
-            return old_callback(*args, **kwargs)
+        else:
+            sentry_wrapped_callback = callback
 
-    resolver_match.func = callback
+        return sentry_wrapped_callback
 
-    return resolver_match
+    BaseHandler.make_view_atomic = sentry_patched_make_view_atomic

tests/integrations/django/myapp/settings.py

@@ -59,6 +59,11 @@
 
 class TestMiddleware(MiddlewareMixin):
     def process_request(self, request):
+        # https://github.com/getsentry/sentry-python/issues/837 -- We should
+        # not touch the resolver_match because apparently people rely on it.
+        if request.resolver_match:
+            assert not getattr(request.resolver_match.callback, "__wrapped__", None)
+
         if "middleware-exc" in request.path:
             1 / 0