Fix unavailable request data while using with REST framework #591 (#1061)

* Add DjangoRestFrameworkCompatMiddleware

* Update DjangoRestFrameworkCompatMiddleware to not require get_response in constructor

This is for compatibility with Django 1.10 and earlier

* Add unit tests for DjangoRestFrameworkCompatMiddleware

* Update docstring in install_middleware function

* Convert DjangoRestFrameworkCompatMiddleware to be backwards-compatible

* Make sure that middlewares are not set in test_request_data_unavailable_if_request_is_read

5 files changed, 67 insertions, 10 deletions

diff --git a/raven/contrib/django/middleware/__init__.py b/raven/contrib/django/middleware/__init__.py
index fd0486d..809cd82 100644
--- a/raven/contrib/django/middleware/__init__.py
+++ b/raven/contrib/django/middleware/__init__.py
@@ -118,3 +118,18 @@ class SentryMiddleware(MiddlewareMixin):
 
 
 SentryLogMiddleware = SentryMiddleware
+
+
+class DjangoRestFrameworkCompatMiddleware(MiddlewareMixin):
+    def process_request(self, request):
+        """
+        Access request.body, otherwise it might not be accessible later
+        after request has been read/streamed
+        """
+        content_type = request.META.get('CONTENT_TYPE', '')
+        if 'application/x-www-form-urlencoded' in content_type:
+            pass
+        elif 'multipart/form-data' in content_type:
+            pass
+        else:
+            request.body  # forces stream to be read into memory
diff --git a/raven/contrib/django/models.py b/raven/contrib/django/models.py
index 08caa67..149ef9f 100644
--- a/raven/contrib/django/models.py
+++ b/raven/contrib/django/models.py
@@ -212,15 +212,12 @@ def register_serializers():
     import raven.contrib.django.serializers  # NOQA
 
 
-def install_middleware():
+def install_middleware(middleware_name, lookup_names=None):
     """
-    Force installation of SentryMiddlware if it's not explicitly present.
-
-    This ensures things like request context and transaction names are made
-    available.
+    Install specified middleware
     """
-    name = 'raven.contrib.django.middleware.SentryMiddleware'
-    all_names = (name, 'raven.contrib.django.middleware.SentryLogMiddleware')
+    if lookup_names is None:
+        lookup_names = (middleware_name,)
     # default settings.MIDDLEWARE is None
     middleware_attr = 'MIDDLEWARE' if getattr(settings,
                                               'MIDDLEWARE',
@@ -228,10 +225,10 @@ def install_middleware():
         else 'MIDDLEWARE_CLASSES'
     # make sure to get an empty tuple when attr is None
     middleware = getattr(settings, middleware_attr, ()) or ()
-    if set(all_names).isdisjoint(set(middleware)):
+    if set(lookup_names).isdisjoint(set(middleware)):
         setattr(settings,
                 middleware_attr,
-                type(middleware)((name,)) + middleware)
+                type(middleware)((middleware_name,)) + middleware)
 
 
 _setup_lock = Lock()
@@ -250,7 +247,13 @@ def initialize():
 
         try:
             register_serializers()
-            install_middleware()
+            install_middleware(
+                'raven.contrib.django.middleware.SentryMiddleware',
+                (
+                    'raven.contrib.django.middleware.SentryMiddleware',
+                    'raven.contrib.django.middleware.SentryLogMiddleware'))
+            install_middleware(
+                'raven.contrib.django.middleware.DjangoRestFrameworkCompatMiddleware')
 
             # XXX(dcramer): maybe this setting should disable ALL of this?
             if not getattr(settings, 'DISABLE_SENTRY_INSTRUMENTATION', False):
diff --git a/tests/contrib/django/tests.py b/tests/contrib/django/tests.py
index b59c839..14b68a1 100644
--- a/tests/contrib/django/tests.py
+++ b/tests/contrib/django/tests.py
@@ -41,6 +41,8 @@ from raven.contrib.django.views import is_valid_origin
 from raven.transport import HTTPTransport
 from raven.utils.serializer import transform
 
+from .views import AppError
+
 #from .models import MyTestModel
 
 DJANGO_15 = django.VERSION >= (1, 5, 0)
@@ -182,6 +184,33 @@ class DjangoClientTest(TestCase):
         assert 'request' in event
         assert event['request']['url'] == 'http://testserver{}'.format(path)
 
+    def test_request_data_unavailable_if_request_is_read(self):
+        with Settings(**{MIDDLEWARE_ATTR: []}):
+            path = reverse('sentry-readrequest-raise-exc')
+            self.assertRaises(
+                AppError,
+                self.client.post,
+                path,
+                '{"a":"b"}',
+                content_type='application/json')
+        assert len(self.raven.events) == 1
+        event = self.raven.events.pop(0)
+        assert event['request']['data'] == '<unavailable>'
+
+    def test_djangorestframeworkcompatmiddleware_fills_request_data(self):
+        with Settings(**{MIDDLEWARE_ATTR: [
+            'raven.contrib.django.middleware.DjangoRestFrameworkCompatMiddleware']}):
+            path = reverse('sentry-readrequest-raise-exc')
+            self.assertRaises(
+                AppError,
+                self.client.post,
+                path,
+                '{"a":"b"}',
+                content_type='application/json')
+        assert len(self.raven.events) == 1
+        event = self.raven.events.pop(0)
+        assert event['request']['data'] == b'{"a":"b"}'
+
     def test_capture_event_with_request_middleware(self):
         path = reverse('sentry-trigger-event')
         resp = self.client.get(path)
diff --git a/tests/contrib/django/urls.py b/tests/contrib/django/urls.py
index 9560b4d..900e824 100644
--- a/tests/contrib/django/urls.py
+++ b/tests/contrib/django/urls.py
@@ -26,6 +26,7 @@ urlpatterns = (
     url(r'^no-error$', views.no_error, name='sentry-no-error'),
     url(r'^fake-login$', views.fake_login, name='sentry-fake-login'),
     url(r'^trigger-500$', views.raise_exc, name='sentry-raise-exc'),
+    url(r'^trigger-500-readrequest$', views.read_request_and_raise_exc, name='sentry-readrequest-raise-exc'),
     url(r'^trigger-500-ioerror$', views.raise_ioerror, name='sentry-raise-ioerror'),
     url(r'^trigger-500-decorated$', views.decorated_raise_exc, name='sentry-raise-exc-decor'),
     url(r'^trigger-500-django$', views.django_exc, name='sentry-django-exc'),
diff --git a/tests/contrib/django/views.py b/tests/contrib/django/views.py
index b288e85..28d0f89 100644
--- a/tests/contrib/django/views.py
+++ b/tests/contrib/django/views.py
@@ -7,6 +7,10 @@ from raven.contrib.django.models import client
 import logging
 
 
+class AppError(Exception):
+    pass
+
+
 def no_error(request):
     return HttpResponse('')
 
@@ -23,6 +27,11 @@ def raise_exc(request):
     raise Exception(request.GET.get('message', 'view exception'))
 
 
+def read_request_and_raise_exc(request):
+    request.read()
+    raise AppError()
+
+
 def raise_ioerror(request):
     raise IOError(request.GET.get('message', 'view exception'))