diff --git a/.gitignore b/.gitignore index ac41fdd8..98794c77 100644 --- a/.gitignore +++ b/.gitignore @@ -31,11 +31,15 @@ pip-delete-this-directory.txt # PyTest cache .cache/ +.pytest_cache/ + # Tox .tox/ .cache/ .python-version +.coverage +htmlcov/ # VirtualEnv .venv/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 21513fda..fc8f8354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,12 @@ v2.5.0 - [unreleased] + * Add new pagination classes based on JSON:API query parameter *recommendations*: * JsonApiPageNumberPagination and JsonApiLimitOffsetPagination. See [usage docs](docs/usage.md#pagination). * Deprecates PageNumberPagination and LimitOffsetPagination. +* Add exception handling improvements: + * Document JSON_API_UNIFORM_EXCEPTIONS setting. + * Catch all exceptions not caught by DRF and format as JSON API error objects instead of returning HTML error pages. + * handle missing fields exception v2.4.0 - Released January 25, 2018 diff --git a/docs/usage.md b/docs/usage.md index c0d8d21e..96789c4c 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -82,6 +82,14 @@ class MyLimitPagination(JsonApiLimitOffsetPagination): max_limit = None ``` +### Exception handling + +For the `exception_handler` class, if the optional `JSON_API_UNIFORM_EXCEPTIONS` is set to True, +all exceptions will respond with the JSON API [error format](http://jsonapi.org/format/#error-objects). + +When `JSON_API_UNIFORM_EXCEPTIONS` is False (the default), non-JSON API views will respond +with the normal DRF error format. + ### Performance Testing If you are trying to see if your viewsets are configured properly to optimize performance, diff --git a/example/tests/test_views.py b/example/tests/test_views.py index db3a3407..2ce7f8ef 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -1,15 +1,24 @@ import json +import sys +import pytest from django.test import RequestFactory from django.utils import timezone from rest_framework.reverse import reverse from rest_framework.test import APITestCase, force_authenticate from rest_framework_json_api.utils import format_resource_type +from rest_framework_json_api.views import ModelViewSet from . import TestBase from .. import views from example.models import Author, Blog, Comment, Entry +from example.serializers import BlogSerializer + +try: + from unittest import mock +except ImportError: + import mock class TestRelationshipView(APITestCase): @@ -280,3 +289,32 @@ def test_no_content_response(self): response = self.client.delete(url) assert response.status_code == 204, response.rendered_content.decode() assert len(response.rendered_content) == 0, response.rendered_content.decode() + + +class BadBlogViewSet(ModelViewSet): + queryset = Blog.objects.all() + serializer_class = BlogSerializer + + def get_queryset(self, *args, **kwargs): + return self.queryset.filter(foo=1) + + +@pytest.mark.xfail((sys.version_info.major, sys.version_info.minor) == (2, 7), + reason="python2.7 mock raises " + "'unbound method get_queryset() must be called with BadBlogViewSet " + "instance as first argument (got nothing instead)' " + "rather than working properly as does python3.x mock") +class TestViewExceptions(TestBase): + def setUp(self): + self.blog = Blog.objects.create(name='Some Blog', tagline="It's a blog") + + @mock.patch( + 'example.views.BlogViewSet.get_queryset', + new=BadBlogViewSet.get_queryset) + def test_field_error_exception(self): + url = '/blogs' + response = self.client.get(url) + self.assertEqual(response.data[0]['code'], "field_error", + msg=response.data[0]['detail']) + self.assertEqual(response.data[0]['source']['parameter'], "foo", + msg=response.data[0]['detail']) diff --git a/rest_framework_json_api/exceptions.py b/rest_framework_json_api/exceptions.py index 38ff527b..b7b75d9d 100644 --- a/rest_framework_json_api/exceptions.py +++ b/rest_framework_json_api/exceptions.py @@ -1,5 +1,9 @@ +import re + +from django.core.exceptions import FieldDoesNotExist, FieldError from django.utils.translation import ugettext_lazy as _ from rest_framework import exceptions, status +from rest_framework.response import Response from rest_framework_json_api import utils @@ -14,6 +18,52 @@ def rendered_with_json_api(view): return False +def unhandled_drf_exception_handler(exc, context): + """ + Deal with exceptions that DRF doesn't catch and return a JSON:API errors object. + + For model query parameters, attempt to identify the parameter that caused the exception: + "Cannot resolve keyword 'xname' into field. Choices are: name, title, ...." + Unfortunately there's no "clean" way to identify which field caused the exception other than + parsing the error message. If the parse fails, a more generic error is reported. + + Even a 500 error for a jsonapi view should return a jsonapi error object, not an HTML response. + """ + if not rendered_with_json_api(context['view']): + return None + keymatch = re.compile(r"^Cannot resolve keyword '(?P\w+)' into field.") + if isinstance(exc, (FieldError, FieldDoesNotExist,)): + matched = keymatch.match(str(exc)) + bad_kw = matched.group("keyword") if matched else "?" + status_code = 400 + errors = [ + { + "status": status_code, + "code": "field_error", + "title": "Field error", + "detail": str(exc), + "source": { + "pointer": context["request"].path, + "parameter": bad_kw + } + } + ] + else: + status_code = 500 + errors = [ + { + "status": status_code, + "code": "server_error", + "title": "Internal Server Error", + "detail": str(exc), + "source": { + "pointer": context["request"].path, + } + } + ] + return Response(errors, status=status_code) + + def exception_handler(exc, context): # Import this here to avoid potential edge-case circular imports, which # crashes with: @@ -26,7 +76,7 @@ def exception_handler(exc, context): # Render exception with DRF response = drf_exception_handler(exc, context) if not response: - return response + return unhandled_drf_exception_handler(exc, context) # Use regular DRF format if not rendered by DRF JSON API and not uniform is_json_api_view = rendered_with_json_api(context['view']) diff --git a/tox.ini b/tox.ini index dc7470e0..a333e219 100644 --- a/tox.ini +++ b/tox.ini @@ -7,13 +7,16 @@ deps = django111: Django>=1.11,<1.12 drf36: djangorestframework>=3.6.3,<3.7 drf37: djangorestframework>=3.7.0,<3.8 + coverage setenv = PYTHONPATH = {toxinidir} DJANGO_SETTINGS_MODULE=example.settings.test commands = - python setup.py test {posargs} + coverage erase + coverage run setup.py test {posargs} + coverage html --include=rest_framework_json_api/* [testenv:isort] deps =