From 0d109c90a74bc575efa6d497a6501aef2b837983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sat, 13 Dec 2014 18:18:00 -0400 Subject: [PATCH 1/8] Add context to exception handler #2236 Same context as renderers which include: the view, args, kwargs, and request. This provides enough contextual information to the exception handlers to handle errors better. In a use case like #1671, a custom handler would allow Sentry to log the request properly. --- rest_framework/views.py | 5 +++-- tests/test_views.py | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index bc870417f..07e713939 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -46,7 +46,7 @@ def get_view_description(view_cls, html=False): return description -def exception_handler(exc): +def exception_handler(exc, context=None): """ Returns the response that should be used for any given exception. @@ -369,7 +369,8 @@ class APIView(View): else: exc.status_code = status.HTTP_403_FORBIDDEN - response = self.settings.EXCEPTION_HANDLER(exc) + context = self.get_renderer_context() + response = self.settings.EXCEPTION_HANDLER(exc, context) if response is None: raise diff --git a/tests/test_views.py b/tests/test_views.py index 77b113ee5..e9b75f065 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -121,7 +121,12 @@ class TestCustomExceptionHandler(TestCase): def setUp(self): self.DEFAULT_HANDLER = api_settings.EXCEPTION_HANDLER - def exception_handler(exc): + def exception_handler(exc, context=None): + self.assertTrue('args' in context) + self.assertTrue('kwargs' in context) + self.assertTrue('request' in context) + self.assertTrue('view' in context) + return Response('Error!', status=status.HTTP_400_BAD_REQUEST) api_settings.EXCEPTION_HANDLER = exception_handler From e8c0766568cb20a5357c5e6823283f0c187b35b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sat, 13 Dec 2014 20:54:35 -0400 Subject: [PATCH 2/8] Support handlers with and without context --- rest_framework/views.py | 10 ++++++++-- tests/test_views.py | 27 +++++++++++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 07e713939..3ece66e68 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -2,6 +2,7 @@ Provides an APIView class that is the base of all views in REST framework. """ from __future__ import unicode_literals +import inspect from django.core.exceptions import PermissionDenied from django.http import Http404 @@ -369,8 +370,13 @@ class APIView(View): else: exc.status_code = status.HTTP_403_FORBIDDEN - context = self.get_renderer_context() - response = self.settings.EXCEPTION_HANDLER(exc, context) + exception_handler = self.settings.EXCEPTION_HANDLER + + if 'context' in inspect.getargspec(exception_handler).args: + context = self.get_renderer_context() + response = exception_handler(exc, context) + else: + response = exception_handler(exc) if response is None: raise diff --git a/tests/test_views.py b/tests/test_views.py index e9b75f065..9952248fc 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -6,6 +6,7 @@ from django.test import TestCase from rest_framework import status from rest_framework.decorators import api_view from rest_framework.response import Response +from rest_framework.request import Request from rest_framework.settings import api_settings from rest_framework.test import APIRequestFactory from rest_framework.views import APIView @@ -121,12 +122,7 @@ class TestCustomExceptionHandler(TestCase): def setUp(self): self.DEFAULT_HANDLER = api_settings.EXCEPTION_HANDLER - def exception_handler(exc, context=None): - self.assertTrue('args' in context) - self.assertTrue('kwargs' in context) - self.assertTrue('request' in context) - self.assertTrue('view' in context) - + def exception_handler(exc): return Response('Error!', status=status.HTTP_400_BAD_REQUEST) api_settings.EXCEPTION_HANDLER = exception_handler @@ -151,3 +147,22 @@ class TestCustomExceptionHandler(TestCase): expected = 'Error!' self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.data, expected) + + def test_context_exception_handler(self): + def exception_handler(exc, context=None): + self.assertEqual(context['args'], ()) + self.assertEqual(context['kwargs'], {}) + self.assertTrue(isinstance(context['request'], Request)) + self.assertTrue(isinstance(context['view'], ErrorView)) + + return Response('Error!', status=status.HTTP_400_BAD_REQUEST) + + api_settings.EXCEPTION_HANDLER = exception_handler + + view = ErrorView.as_view() + + request = factory.get('/', content_type='application/json') + response = view(request) + expected = 'Error!' + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data, expected) From 3f85b476fa2ddb8a205c03cea6684fca257dbd02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sun, 14 Dec 2014 10:15:13 -0400 Subject: [PATCH 3/8] Remove test --- tests/test_views.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index 9952248fc..77b113ee5 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -6,7 +6,6 @@ from django.test import TestCase from rest_framework import status from rest_framework.decorators import api_view from rest_framework.response import Response -from rest_framework.request import Request from rest_framework.settings import api_settings from rest_framework.test import APIRequestFactory from rest_framework.views import APIView @@ -147,22 +146,3 @@ class TestCustomExceptionHandler(TestCase): expected = 'Error!' self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.data, expected) - - def test_context_exception_handler(self): - def exception_handler(exc, context=None): - self.assertEqual(context['args'], ()) - self.assertEqual(context['kwargs'], {}) - self.assertTrue(isinstance(context['request'], Request)) - self.assertTrue(isinstance(context['view'], ErrorView)) - - return Response('Error!', status=status.HTTP_400_BAD_REQUEST) - - api_settings.EXCEPTION_HANDLER = exception_handler - - view = ErrorView.as_view() - - request = factory.get('/', content_type='application/json') - response = view(request) - expected = 'Error!' - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.data, expected) From 478c8d724b846b370c897548f8ee89f1128e12c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sun, 14 Dec 2014 10:16:52 -0400 Subject: [PATCH 4/8] Update docs --- docs/api-guide/exceptions.md | 4 ++-- docs/api-guide/settings.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/api-guide/exceptions.md b/docs/api-guide/exceptions.md index 467ad9709..31a8431bc 100644 --- a/docs/api-guide/exceptions.md +++ b/docs/api-guide/exceptions.md @@ -51,10 +51,10 @@ In order to alter the style of the response, you could write the following custo from rest_framework.views import exception_handler - def custom_exception_handler(exc): + def custom_exception_handler(exc, context): # Call REST framework's default exception handler first, # to get the standard error response. - response = exception_handler(exc) + response = exception_handler(exc, context) # Now add the HTTP status code to the response. if response is not None: diff --git a/docs/api-guide/settings.md b/docs/api-guide/settings.md index 9005511b7..2c4f84237 100644 --- a/docs/api-guide/settings.md +++ b/docs/api-guide/settings.md @@ -393,7 +393,7 @@ This setting can be changed to support error responses other than the default `{ This should be a function with the following signature: - exception_handler(exc) + exception_handler(exc, context) * `exc`: The exception. From fd003fcefaee964e744ded0aec1ae76715889378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sun, 14 Dec 2014 15:03:20 -0400 Subject: [PATCH 5/8] Add pending deprecation warning message --- rest_framework/views.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 3ece66e68..37889d1b4 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -3,6 +3,7 @@ Provides an APIView class that is the base of all views in REST framework. """ from __future__ import unicode_literals import inspect +import warnings from django.core.exceptions import PermissionDenied from django.http import Http404 @@ -370,13 +371,16 @@ class APIView(View): else: exc.status_code = status.HTTP_403_FORBIDDEN - exception_handler = self.settings.EXCEPTION_HANDLER - - if 'context' in inspect.getargspec(exception_handler).args: - context = self.get_renderer_context() - response = exception_handler(exc, context) + if len(inspect.getargspec(self.settings.EXCEPTION_HANDLER).args) == 1: + warnings.warn( + 'The `exception_handler(exc)` call signature is deprecated. ' + 'Use `exception_handler(exc, context) instead.', + PendingDeprecationWarning + ) + response = self.settings.EXCEPTION_HANDLER(exc) else: - response = exception_handler(exc) + context = self.get_renderer_context() + response = self.settings.EXCEPTION_HANDLER(exc, context) if response is None: raise From 89e9fc98d6e7407e6f7715fa2680df7c94221105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sun, 14 Dec 2014 15:20:44 -0400 Subject: [PATCH 6/8] Reuse exception_handler variable throughout --- rest_framework/views.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 37889d1b4..c2e19bf42 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -371,16 +371,18 @@ class APIView(View): else: exc.status_code = status.HTTP_403_FORBIDDEN - if len(inspect.getargspec(self.settings.EXCEPTION_HANDLER).args) == 1: + exception_handler = self.settings.EXCEPTION_HANDLER + + if len(inspect.getargspec(exception_handler).args) == 1: warnings.warn( 'The `exception_handler(exc)` call signature is deprecated. ' 'Use `exception_handler(exc, context) instead.', PendingDeprecationWarning ) - response = self.settings.EXCEPTION_HANDLER(exc) + response = exception_handler(exc) else: context = self.get_renderer_context() - response = self.settings.EXCEPTION_HANDLER(exc, context) + response = exception_handler(exc, context) if response is None: raise From 26c223a34f7e0cc21d37c6302e53d547dae252dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sun, 14 Dec 2014 16:43:58 -0400 Subject: [PATCH 7/8] Add get_exception_handler_context() --- rest_framework/views.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index c2e19bf42..80a13a1a9 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -186,6 +186,18 @@ class APIView(View): 'request': getattr(self, 'request', None) } + def get_exception_handler_context(self): + """ + Returns a dict that is passed through to EXCEPTION_HANDLER, + as the `context` argument. + """ + return { + 'view': self, + 'args': getattr(self, 'args', ()), + 'kwargs': getattr(self, 'kwargs', {}), + 'request': getattr(self, 'request', None) + } + def get_view_name(self): """ Return the view name, as used in OPTIONS responses and in the @@ -381,7 +393,7 @@ class APIView(View): ) response = exception_handler(exc) else: - context = self.get_renderer_context() + context = self.get_exception_handler_context() response = exception_handler(exc, context) if response is None: From 4ebd8770b94ecb8fe8fb41fe8daa4309b33b9952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Sun, 14 Dec 2014 20:47:33 -0400 Subject: [PATCH 8/8] Update excepteion_handler signature --- rest_framework/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 80a13a1a9..b39724c2f 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -48,7 +48,7 @@ def get_view_description(view_cls, html=False): return description -def exception_handler(exc, context=None): +def exception_handler(exc, context): """ Returns the response that should be used for any given exception.