Follow RFC 7807 when responding with JSONDecodeException (#234)

- JSONDecodeExceptions thrown while decoding request data (body json, get attributes) are now caught and re-thrown as @web.problem annotated RequestDataDecodingException to comply with RFC7807

- DecodeExceptionHandler is removed - uncaught JSONDecodeExceptions will be eventually responded as http 500

Author: iisaev-qs

Diff for: CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [12.0.0]
+### Changed
+- Adjust response body to comply with RFC7807 when request data decoding fails
+
 ## [11.0.0]
 ### Added
 - get_all_subclasses function in module_discovery module

Diff for: pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "winter"
-version = "11.0.0"
+version = "12.0.0"
 homepage = "https://github.com/WinterFramework/winter"
 description = "Web Framework inspired by Spring Framework"
 authors = ["Alexander Egorov <[email protected]>"]

Diff for: tests/api/api_with_problem_exceptions.py

@@ -5,6 +5,7 @@
 
 import winter.web
 from winter.data.exceptions import NotFoundException
+from winter.web.exceptions import RequestDataDecodeException
 
 
 @winter.web.problem(status=HTTPStatus.FORBIDDEN)
@@ -87,3 +88,18 @@ def custom_handler_problem_exists_exception(self) -> str:
     @winter.route_get('not_found_exception/')
     def not_found_exception(self) -> str:
         raise NotFoundException(entity_id=1, entity_cls=MyEntity)
+
+    @winter.route_get('request_data_decoding_exception_with_dict_errors/')
+    def json_decoder_errors_as_dict_exception(self) -> None:
+        errors = {
+            'non_field_error': 'Missing fields: "id", "status", "int_status", "birthday"',
+            'contact': {
+                'phones': 'Cannot decode "123" to set',
+            },
+        }
+        raise RequestDataDecodeException(errors)
+
+    @winter.route_get('request_data_decoding_exception_with_str_errors/')
+    def json_decoder_errors_as_str_exception(self) -> None:
+        errors = 'Cannot decode "data1" to integer'
+        raise RequestDataDecodeException(errors)

Diff for: tests/pagination/test_page_position_argument_resolver.py

@@ -9,6 +9,7 @@
 from winter.core.json import decoder
 from winter.data.pagination import PagePosition
 from winter.data.pagination import Sort
+from winter.web.exceptions import RequestDataDecodeException
 from winter.web.pagination import PagePositionArgumentResolver
 
 
@@ -111,19 +112,30 @@ def method(page_position: PagePosition):  # pragma: no cover
 
 
 @pytest.mark.parametrize(
-    ('query_string', 'exception_type', 'message'), (
-        ('limit=none', decoder.JSONDecodeException, 'Cannot decode "none" to PositiveInteger'),
-        ('offset=-20', decoder.JSONDecodeException, 'Cannot decode "-20" to PositiveInteger'),
-        ('order_by=id,', ParseError, 'Invalid field for order: ""'),
-        ('order_by=-', ParseError, 'Invalid field for order: "-"'),
+    ('query_string', 'exception_type', 'message', 'errors_dict'), (
+        (
+            'limit=none',
+            RequestDataDecodeException,
+            'Failed to decode request data',
+            {'error': 'Cannot decode "none" to PositiveInteger'}
+        ),
+        (
+            'offset=-20',
+            RequestDataDecodeException,
+            'Failed to decode request data',
+            {'error': 'Cannot decode "-20" to PositiveInteger'}
+        ),
+        ('order_by=id,', ParseError, 'Invalid field for order: ""', None),
+        ('order_by=-', ParseError, 'Invalid field for order: "-"', None),
         (
             'order_by=not_allowed_order_by_field',
             ParseError,
             'Fields do not allowed as order by fields: "not_allowed_order_by_field"',
+            None,
         ),
     ),
 )
-def test_resolve_argument_fails_in_page_position_argument_resolver(query_string, exception_type, message):
+def test_resolve_argument_fails_in_page_position_argument_resolver(query_string, exception_type, message, errors_dict):
     @winter.web.pagination.order_by(['id'])
     def method(arg1: int):  # pragma: no cover
         return arg1
@@ -140,3 +152,4 @@ def method(arg1: int):  # pragma: no cover
         resolver.resolve_argument(argument, request, {})
 
     assert exception_info.value.args[0] == message
+    assert not hasattr(exception_info.value, 'errors') or exception_info.value.errors == errors_dict

Diff for: tests/routing/test_query_parameters.py

@@ -14,8 +14,8 @@
 from tests.entities import AuthorizedUser
 from tests.utils import get_request
 from winter.core.annotations import AlreadyAnnotated
-from winter.core.json import decoder
 from winter.web.argument_resolver import ArgumentNotSupported
+from winter.web.exceptions import RequestDataDecodeException
 from winter.web.query_parameters.query_parameters_argument_resolver import QueryParameterArgumentResolver
 
 
@@ -47,12 +47,16 @@ def method(
 
 
 @pytest.mark.parametrize(
-    ('query_string', 'expected_exception_message'), (
-        ('query_param=invalid_int', 'Cannot decode "invalid_int" to integer'),
-        ('', 'Missing required query parameter "query_param"'),
+    ('query_string', 'expected_exception_message', 'expected_errors'), (
+        (
+            'query_param=invalid_int',
+            'Failed to decode request data',
+            {'error': 'Cannot decode "invalid_int" to integer'}
+        ),
+        ('', 'Failed to decode request data', {'error': 'Missing required query parameter "query_param"'}),
     ),
 )
-def test_query_parameter_resolver_with_raises_parse_error(query_string, expected_exception_message):
+def test_query_parameter_resolver_with_raises_parse_error(query_string, expected_exception_message, expected_errors):
     @winter.route_get('{?query_param}')
     def method(query_param: int):  # pragma: no cover
         return query_param
@@ -62,10 +66,11 @@ def method(query_param: int):  # pragma: no cover
     argument = method.get_argument('query_param')
     request = get_request(query_string)
 
-    with pytest.raises(decoder.JSONDecodeException) as exception:
+    with pytest.raises(RequestDataDecodeException) as exception_info:
         resolver.resolve_argument(argument, request, {})
 
-    assert str(exception.value) == expected_exception_message
+    assert str(exception_info.value) == expected_exception_message
+    assert exception_info.value.errors == expected_errors
 
 
 def test_query_parameter_resolver_with_raises_parse_uuid_error():
@@ -78,10 +83,11 @@ def method(query_param: UUID):  # pragma: no cover
     argument = method.get_argument('query_param')
     request = get_request('query_param=invalid_uuid')
 
-    with pytest.raises(decoder.JSONDecodeException) as exception:
+    with pytest.raises(RequestDataDecodeException) as exception_info:
         resolver.resolve_argument(argument, request, {})
 
-    assert str(exception.value) == 'Cannot decode "invalid_uuid" to uuid'
+    assert str(exception_info.value) == 'Failed to decode request data'
+    assert exception_info.value.errors == {'error': 'Cannot decode "invalid_uuid" to uuid'}
 
 
 @pytest.mark.parametrize(
@@ -160,10 +166,11 @@ def method(x_param: int, y_param: int = 1):  # pragma: no cover
     argument = method.get_argument('x_param')
     request = get_request('?x=1')
 
-    with pytest.raises(decoder.JSONDecodeException) as exception:
+    with pytest.raises(RequestDataDecodeException) as exception_info:
         resolver.resolve_argument(argument, request, {})
 
-    assert str(exception.value) == 'Missing required query parameter "x"'
+    assert str(exception_info.value) == 'Failed to decode request data'
+    assert exception_info.value.errors == {'error': 'Missing required query parameter "x"'}
 
 
 @pytest.mark.parametrize(

Diff for: tests/test_exception_handling.py

@@ -123,6 +123,35 @@ def test_exception_handler_with_unknown_argument():
                 'detail': 'MyEntity with ID=1 not found',
             },
         ),
+        (
+            'request_data_decoding_exception_with_str_errors',
+            HTTPStatus.BAD_REQUEST,
+            'application/json+problem',
+            {
+                'status': 400,
+                'type': 'urn:problem-type:request-data-decode',
+                'title': 'Request data decode',
+                'detail': 'Failed to decode request data',
+                'errors': {'error': 'Cannot decode "data1" to integer'},
+            },
+        ),
+        (
+            'request_data_decoding_exception_with_dict_errors',
+            HTTPStatus.BAD_REQUEST,
+            'application/json+problem',
+            {
+                'status': 400,
+                'type': 'urn:problem-type:request-data-decode',
+                'title': 'Request data decode',
+                'detail': 'Failed to decode request data',
+                'errors': {
+                    'non_field_error': 'Missing fields: "id", "status", "int_status", "birthday"',
+                    'contact': {
+                        'phones': 'Cannot decode "123" to set',
+                    },
+                }
+            },
+        ),
     ),
 )
 def test_api_with_problem_exceptions(url_path, expected_status, expected_content_type, expected_body):

Diff for: tests/web/test_request_body.py

@@ -84,10 +84,16 @@ def test_request_body_with_errors():
     }
 
     expected_data = {
-        'id': 'Cannot decode "invalid integer" to integer',
-        'status': 'Value not in allowed values("active", "super_active"): "invalid status"',
-        'items': 'Cannot decode "invalid integer" to integer',
-        'non_field_error': 'Missing fields: "name"',
+        'status': 400,
+        'type': 'urn:problem-type:request-data-decode',
+        'title': 'Request data decode',
+        'detail': 'Failed to decode request data',
+        'errors': {
+            'id': 'Cannot decode "invalid integer" to integer',
+            'status': 'Value not in allowed values("active", "super_active"): "invalid status"',
+            'items': 'Cannot decode "invalid integer" to integer',
+            'non_field_error': 'Missing fields: "name"',
+        }
     }
     data = json.dumps(data)
 

Diff for: winter/web/__init__.py

@@ -23,7 +23,6 @@
 
 
 def setup():
-    from winter.core.json.decoder import JSONDecodeException
     from winter.data.exceptions import NotFoundException
     from .configurer import run_configurers
     from .exceptions import RedirectException
@@ -32,7 +31,6 @@ def setup():
     from .exceptions.problem_handling import ProblemExceptionMapper
     from .exceptions.problem_handling_info import ProblemHandlingInfo
     from .exception_handlers import RedirectExceptionHandler
-    from .exception_handlers import DecodeExceptionHandler
     from .pagination.page_processor_resolver import PageOutputProcessorResolver
     from .pagination.page_position_argument_resolver import PagePositionArgumentResolver
     from .path_parameters_argument_resolver import PathParametersArgumentResolver
@@ -53,7 +51,6 @@ def setup():
     exception_handler_generator = ProblemExceptionHandlerGenerator(exception_mapper)
     autodiscover_problem_annotations(exception_handler_generator)
     auto_handle_exceptions = {
-        JSONDecodeException: DecodeExceptionHandler,
         RedirectException: RedirectExceptionHandler,
         NotFoundException: exception_handler_generator.generate(NotFoundException, ProblemHandlingInfo(status=404)),
     }

Diff for: winter/web/exception_handlers.py

@@ -1,20 +1,12 @@
 from http import HTTPStatus
-from typing import Dict
 
-from winter.core.json.decoder import JSONDecodeException
 from .exceptions import ExceptionHandler
 from .exceptions import RedirectException
 from .response_header_annotation import ResponseHeader
 from .response_header_annotation import response_header
 from .response_status_annotation import response_status
 
 
-class DecodeExceptionHandler(ExceptionHandler):
-    @response_status(HTTPStatus.BAD_REQUEST)
-    def handle(self, exception: JSONDecodeException) -> Dict:
-        return exception.errors
-
-
 class RedirectExceptionHandler(ExceptionHandler):
     @response_status(HTTPStatus.FOUND)
     @response_header('Location', 'location_header')

Diff for: winter/web/exceptions/__init__.py

@@ -2,6 +2,7 @@
 from .exception_mapper import ExceptionMapper
 from .exceptions import RedirectException
 from .exceptions import ThrottleException
+from .exceptions import RequestDataDecodeException
 from .handlers import ExceptionHandler
 from .handlers import MethodExceptionsManager
 from .handlers import exception_handlers_registry

Diff for: winter/web/exceptions/exceptions.py

@@ -1,4 +1,9 @@
 from http import HTTPStatus
+from typing import Dict
+from typing import Optional
+from typing import Union
+
+import dataclasses
 
 from .problem import problem
 
@@ -12,3 +17,16 @@ class RedirectException(Exception):
     def __init__(self, redirect_to: str):
         super().__init__()
         self.redirect_to = redirect_to
+
+
+@problem(status=HTTPStatus.BAD_REQUEST)
+@dataclasses.dataclass
+class RequestDataDecodeException(Exception):
+    errors: dict = dataclasses.field(default_factory=dict)
+
+    def __init__(self, errors: Optional[Union[str, Dict]]):
+        super().__init__('Failed to decode request data')
+        if type(errors) == dict:
+            self.errors = errors
+        elif type(errors) == str:
+            self.errors = {'error': errors}

Diff for: winter/web/exceptions/problem_handling.py

@@ -1,11 +1,11 @@
-import re
-from typing import Dict
-from typing import Type
-
 import dataclasses
+import re
 from dataclasses import asdict
 from dataclasses import dataclass
 from dataclasses import is_dataclass
+from typing import Dict
+from typing import Type
+
 from rest_framework.request import Request
 
 from winter.core import Component

Diff for: winter/web/pagination/page_position_argument_resolver.py

@@ -6,12 +6,14 @@
 
 from winter.core import ComponentMethod
 from winter.core import ComponentMethodArgument
+from winter.core.json import JSONDecodeException
 from winter.core.json import json_decode
 from winter.core.utils import PositiveInteger
 from winter.data.pagination import PagePosition
 from winter.data.pagination import Sort
 from winter.web.argument_resolver import ArgumentResolver
 from winter.web.exceptions import RedirectException
+from winter.web.exceptions import RequestDataDecodeException
 from winter.web.pagination.check_sort import check_sort
 from winter.web.pagination.limits import Limits
 from winter.web.pagination.limits import LimitsAnnotation
@@ -74,8 +76,11 @@ def _parse_page_position(self, argument: ComponentMethodArgument, http_request:
         raw_limit = http_request.query_params.get(self.limit_name) or None
         raw_offset = http_request.query_params.get(self.offset_name) or None
         raw_order_by = http_request.query_params.get(self.order_by_name, '')
-        limit = json_decode(raw_limit, Optional[PositiveInteger])
-        offset = json_decode(raw_offset, Optional[PositiveInteger])
+        try:
+            limit = json_decode(raw_limit, Optional[PositiveInteger])
+            offset = json_decode(raw_offset, Optional[PositiveInteger])
+        except JSONDecodeException as e:
+            raise RequestDataDecodeException(e.errors)
         sort = self._parse_sort_properties(raw_order_by, argument)
         return PagePosition(limit=limit, offset=offset, sort=sort)
 

Diff for: winter/web/query_parameters/query_parameters_argument_resolver.py

@@ -11,6 +11,7 @@
 from .query_parameter import QueryParameter
 from ..argument_resolver import ArgumentNotSupported
 from ..argument_resolver import ArgumentResolver
+from ..exceptions import RequestDataDecodeException
 from ..routing import get_route
 
 
@@ -44,10 +45,14 @@ def resolve_argument(
             try:
                 return argument.get_default()
             except ArgumentDoesNotHaveDefault:
-                raise JSONDecodeException(f'Missing required query parameter "{parameter_name}"')
+                raise RequestDataDecodeException(f'Missing required query parameter "{parameter_name}"')
 
         value = self._get_value(query_parameters, parameter_name, is_iterable, explode)
-        return json_decode(value, argument.type_)
+
+        try:
+            return json_decode(value, argument.type_)
+        except JSONDecodeException as e:
+            raise RequestDataDecodeException(e.errors)
 
     def _get_query_parameter(self, argument: ComponentMethodArgument) -> Optional[QueryParameter]:
         if argument in self._query_parameters:

Diff for: winter/web/request_body_resolver.py

@@ -3,8 +3,10 @@
 from rest_framework.request import Request
 
 from .argument_resolver import ArgumentResolver
+from .exceptions.exceptions import RequestDataDecodeException
 from .request_body_annotation import RequestBodyAnnotation
 from ..core import ComponentMethodArgument
+from ..core.json import JSONDecodeException
 from ..core.json import json_decode
 
 
@@ -22,4 +24,7 @@ def resolve_argument(
         request: Request,
         response_headers: MutableMapping[str, str],
     ):
-        return json_decode(request.data, argument.type_)
+        try:
+            return json_decode(request.data, argument.type_)
+        except JSONDecodeException as e:
+            raise RequestDataDecodeException(e.errors)