Improve exception handling (#182)

* Remove auto_handle arg from problem annotation, behave like auto_handle=True
* Rename module, function and annotation throws to raises
* Add validate_missing_raises_annotations step to openapi setup

CHANGELOG.md

@@ -4,6 +4,16 @@ 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).
 
+## [7.0.0] - 2020-12-08
+
+### New features
+- winter_openapi add annotation @winter_openapi.global_exception
+- winter_openapi add validation check for missing exceptions 
+### Changed
+- winter.web rename annotation @winter.throws → @winter.raises
+- winter.web remove argument for problem annotation `auto_handle` and define default as True 
+
+
 ## [5.0.0] - 2020-10-25
 
 ### New features

README.md

@@ -70,7 +70,7 @@ class TodoDTO:
     todo: str
 
 
-@winter.web.problem(status=HTTPStatus.NOT_FOUND, auto_handle=True)
+@winter.web.problem(status=HTTPStatus.NOT_FOUND)
 class NotFoundException(Exception):
     def __init__(self, todo_index: int):
         self.index = todo_index
@@ -173,7 +173,7 @@ import winter.web
 
 # Minimalist approach. Pointed status and that this exception will be handling automatically. Expected output below:
 # {'status': 404, 'type': 'urn:problem-type:todo-not-found', 'title': 'Todo not found', 'detail': 'Incorrect index: 1'}
-@winter.web.problem(status=HTTPStatus.NOT_FOUND, auto_handle=True)
+@winter.web.problem(status=HTTPStatus.NOT_FOUND)
 class TodoNotFoundException(Exception):
     def __init__(self, invalid_index: int):
         self.invalid_index = invalid_index
@@ -183,7 +183,7 @@ class TodoNotFoundException(Exception):
 
 # Extending output using dataclass. Dataclass fields will be added to response body. Expected output below:
 # {'status': 404, 'type': 'urn:problem-type:todo-not-found', 'title': 'Todo not found', 'detail': '', 'invalid_index': 1}
-@winter.web.problem(status=HTTPStatus.NOT_FOUND, auto_handle=True)
+@winter.web.problem(status=HTTPStatus.NOT_FOUND)
 @dataclass
 class TodoNotFoundException(Exception):
     invalid_index: int
@@ -212,7 +212,7 @@ class TodoProblemExistsController:
         raise TodoNotFoundException(invalid_index=todo_index)
 
     @winter.route_get('custom/{todo_index}/')
-    @winter.throws(TodoNotFoundException, handler_cls=TodoNotFoundExceptionCustomHandler)
+    @winter.raises(TodoNotFoundException, handler_cls=TodoNotFoundExceptionCustomHandler)
     def get_todo_with_custom_handling(self, todo_index: int):
         raise TodoNotFoundException(invalid_index=todo_index)
 

tests/controllers/controller_with_exceptions.py

@@ -53,17 +53,17 @@ class ChildCustomException(CustomException):
 class ControllerWithExceptions:
 
     @winter.route_get('declared_but_not_thrown/')
-    @winter.throws(CustomException)
+    @winter.raises(CustomException)
     def declared_but_not_thrown(self) -> str:
         return 'Hello, sir!'
 
     @winter.route_get('declared_and_thrown/')
-    @winter.throws(CustomException)
+    @winter.raises(CustomException)
     def declared_and_thrown(self) -> str:
         raise CustomException('declared_and_thrown')
 
     @winter.route_get('declared_and_thrown_child/')
-    @winter.throws(CustomException)
+    @winter.raises(CustomException)
     def declared_and_thrown_child(self) -> str:
         raise ChildCustomException('declared_and_thrown_child')
 
@@ -72,16 +72,16 @@ def not_declared_but_thrown(self) -> str:
         raise CustomException('not_declared_but_thrown')
 
     @winter.route_get('declared_but_no_handler/')
-    @winter.throws(ExceptionWithoutHandler)
+    @winter.raises(ExceptionWithoutHandler)
     def declared_but_no_handler(self) -> str:
         raise ExceptionWithoutHandler()
 
-    @winter.throws(CustomException, AnotherExceptionHandler)
+    @winter.raises(CustomException, AnotherExceptionHandler)
     @winter.route_get('exception_with_custom_handler/')
     def with_custom_handler(self) -> str:
         raise CustomException('message')
 
-    @winter.throws(WithUnknownArgumentException)
+    @winter.raises(WithUnknownArgumentException)
     @winter.route_get('with_unknown_argument_exception/')
     def with_unknown_argument_handler(self) -> str:
         raise WithUnknownArgumentException()

tests/controllers/controller_with_problem_exceptions.py

@@ -13,12 +13,11 @@ def __str__(self):
         return 'Implicit string of detail'
 
 
-@winter.web.problem(status=HTTPStatus.FORBIDDEN, auto_handle=True)
-class ProblemExistsAutoHandleException(Exception):
+class InheritorOfProblemExistsException(ProblemExistsException):
     pass
 
 
-@winter.web.problem(status=HTTPStatus.FORBIDDEN, auto_handle=True)
+@winter.web.problem(status=HTTPStatus.FORBIDDEN)
 @dataclasses.dataclass
 class ProblemExistsDataclassException(Exception):
     custom_field: str = 'custom value'
@@ -28,12 +27,12 @@ def __str__(self):
 
 
 @winter.web.problem(
-    status=HTTPStatus.FORBIDDEN,
-    title='Problem exists',
-    type='urn:problem-type:problem-exists',
+    status=HTTPStatus.BAD_REQUEST,
+    title='All fields problem exists',
+    type='urn:problem-type:all-field-problem-exists',
     detail='A lot of interesting things happens with this problem',
 )
-class ProblemExistsStaticException(Exception):
+class AllFieldConstProblemExistsException(Exception):
     pass
 
 
@@ -62,25 +61,26 @@ def handle(self, exception: ProblemExistsException) -> CustomExceptionDTO:
 @winter.route('controller_with_problem_exceptions/')
 class ControllerWithProblemExceptions:
 
-    @winter.route_get('problem_exists_not_handled_exception/')
-    def problem_exists_not_handled_exception(self) -> str:
-        raise ProblemExistsException()
-
-    @winter.throws(ProblemExistsException)
+    @winter.raises(ProblemExistsException)
     @winter.route_get('problem_exists_exception/')
     def problem_exists_exception(self) -> str:
         raise ProblemExistsException()
 
-    @winter.throws(ProblemExistsDataclassException)
+    @winter.raises(ProblemExistsDataclassException)
     @winter.route_get('problem_exists_dataclass_exception/')
     def problem_exists_dataclass_exception(self) -> str:
         raise ProblemExistsDataclassException()
 
-    @winter.route_get('problem_exists_auto_handle_exception/')
+    @winter.raises(AllFieldConstProblemExistsException)
+    @winter.route_get('all_field_const_problem_exists_exception/')
+    def all_field_const_problem_exists_exception(self) -> str:
+        raise AllFieldConstProblemExistsException()
+
+    @winter.route_get('inherited_problem_exists_exception/')
     def problem_exists_auto_handle_exception(self) -> str:
-        raise ProblemExistsAutoHandleException()
+        raise InheritorOfProblemExistsException()
 
-    @winter.throws(ProblemExistsException, ProblemExistsExceptionCustomHandler)
+    @winter.raises(ProblemExistsException, ProblemExistsExceptionCustomHandler)
     @winter.route_get('custom_handler_problem_exists_exception/')
     def custom_handler_problem_exists_exception(self) -> str:
         raise ProblemExistsException()

tests/controllers/controller_with_throttling.py

@@ -35,6 +35,6 @@ def method_without_throttling(self):
 
     @winter.route_get('custom-handler/')
     @winter.web.throttling('5/s')
-    @winter.throws(ThrottleException, CustomThrottleExceptionHandler)
+    @winter.raises(ThrottleException, CustomThrottleExceptionHandler)
     def simple_method_with_custom_handler(self) -> int:
         return 1

tests/test_exception_handling.py

@@ -6,7 +6,6 @@
 from winter.web.argument_resolver import ArgumentNotSupported
 from .controllers.controller_with_exceptions import CustomException
 from .controllers.controller_with_exceptions import ExceptionWithoutHandler
-from .controllers.controller_with_problem_exceptions import ProblemExistsException
 from .entities import AuthorizedUser
 
 
@@ -86,14 +85,25 @@ def test_exception_handler_with_unknown_argument():
             },
         ),
         (
-            'problem_exists_auto_handle_exception',
+            'all_field_const_problem_exists_exception',
+            HTTPStatus.BAD_REQUEST,
+            'application/json+problem',
+            {
+                'status': 400,
+                'type': 'urn:problem-type:all-field-problem-exists',
+                'title': 'All fields problem exists',
+                'detail': 'A lot of interesting things happens with this problem',
+            },
+        ),
+        (
+            'inherited_problem_exists_exception',
             HTTPStatus.FORBIDDEN,
             'application/json+problem',
             {
                 'status': 403,
-                'type': 'urn:problem-type:problem-exists-auto-handle',
-                'title': 'Problem exists auto handle',
-                'detail': '',
+                'type': 'urn:problem-type:inheritor-of-problem-exists',
+                'title': 'Inheritor of problem exists',
+                'detail': 'Implicit string of detail',
             },
         ),
         (
@@ -128,19 +138,3 @@ def test_controller_with_problem_exceptions(url_path, expected_status, expected_
     assert response.status_code == expected_status
     assert response.get('Content-Type') == expected_content_type
     assert response.json() == expected_body
-
-
-@pytest.mark.parametrize(
-    ['url_path', 'expected_exception_cls'], (
-        ('problem_exists_not_handled_exception', ProblemExistsException),
-    ),
-)
-def test_controller_with_problem_exceptions_raise_error(url_path, expected_exception_cls):
-    client = APIClient()
-    user = AuthorizedUser()
-    client.force_authenticate(user)
-    url = f'/controller_with_problem_exceptions/{url_path}/'
-
-    # Act
-    with pytest.raises(expected_exception_cls):
-        client.get(url)

tests/winter_openapi/test_global_exception_annotation.py

@@ -0,0 +1,25 @@
+from winter.core import Component
+from winter_openapi.annotations import GlobalExceptionAnnotation
+from winter_openapi.annotations import global_exception
+from winter_openapi.annotations import register_global_exception
+
+
+def test_global_exception_decorator_declarative_way():
+    @global_exception
+    class GlobalException(Exception):
+        pass
+
+    component = Component.get_by_cls(GlobalException)
+    assert component is not None
+    assert component.annotations.get(GlobalExceptionAnnotation) == [GlobalExceptionAnnotation(GlobalException)]
+
+
+def test_global_exception_decorator_imperative_way():
+    class GlobalException(Exception):
+        pass
+
+    register_global_exception(GlobalException)
+
+    component = Component.get_by_cls(GlobalException)
+    assert component is not None
+    assert component.annotations.get(GlobalExceptionAnnotation) == [GlobalExceptionAnnotation(GlobalException)]

tests/winter_openapi/test_validators.py

@@ -0,0 +1,16 @@
+from http import HTTPStatus
+
+import pytest
+
+from winter.web import problem
+from winter_openapi import validate_missing_raises_annotations
+
+
+def test_validate_missing_raises_annotations_with_missed_raises_and_not_global_expect_assert_exception():
+    @problem(HTTPStatus.BAD_REQUEST)
+    class MissingException(Exception):
+        pass
+    expected_error_message = f'You are missing declaration for next exceptions: {MissingException.__name__}'
+
+    with pytest.raises(AssertionError, match=expected_error_message):
+        validate_missing_raises_annotations()

winter/__init__.py

@@ -8,7 +8,7 @@
 from .web.argument_resolver import ArgumentResolver
 from .web.argument_resolver import ArgumentsResolver
 from .web.argument_resolver import GenericArgumentResolver
-from .web.exceptions.throws import throws
+from .web.exceptions.raises import raises
 from .web.output_processor import register_output_processor_resolver
 from .web.query_parameters import map_query_parameter
 from .web.response_header_serializer import response_headers_serializer

winter/__version__.py

@@ -1 +1 @@
-__version__ = '6.1.0'
+__version__ = '7.0.0'

winter/web/exceptions/handlers.py

@@ -6,7 +6,7 @@
 from typing import Union
 
 from winter.core import ComponentMethod
-from .throws import get_throws
+from .raises import get_raises
 
 NotHandled = object()
 
@@ -58,7 +58,7 @@ class MethodExceptionsManager:
     def __init__(self, method: ComponentMethod):
         super().__init__()
         self._method = method
-        self._handlers_by_exception = get_throws(self._method)
+        self._handlers_by_exception = get_raises(self._method)
 
     @property
     def declared_exception_classes(self) -> Tuple[Type[Exception], ...]:

winter/web/exceptions/problem.py

@@ -12,7 +12,6 @@ def problem(
     title: Optional[str] = None,
     detail: Optional[str] = None,
     type: Optional[str] = None,
-    auto_handle: bool = False,
 ):
     def wrapper(exception_class):
         assert issubclass(exception_class, Exception), f'Class "{exception_class}" must be a subclass of Exception'
@@ -23,7 +22,6 @@ def wrapper(exception_class):
                 title=title,
                 detail=detail,
             ),
-            auto_handle=auto_handle,
         )
         annotation_decorator = annotate(annotation, unique=True)
         class_ = annotation_decorator(exception_class)

winter/web/exceptions/problem_annotation.py

@@ -6,4 +6,3 @@
 @dataclass(frozen=True)
 class ProblemAnnotation:
     handling_info: ProblemHandlingInfo
-    auto_handle: bool = False

winter/web/exceptions/problem_handling.py

@@ -103,5 +103,5 @@ def autodiscover_problem_annotations(handler_generator: ProblemExceptionHandlerG
         exception_handlers_registry.add_handler(
             exception_class,
             handler_class,
-            auto_handle=problem_annotation.auto_handle,
+            auto_handle=True,
         )

winter/web/exceptions/throws.py → winter/web/exceptions/raises.py

@@ -18,7 +18,7 @@ class ExceptionAnnotation:
     handler: Optional['ExceptionHandler'] = None
 
 
-def throws(exception_cls: Type[Exception], handler_cls: Optional[Type['ExceptionHandler']] = None):
+def raises(exception_cls: Type[Exception], handler_cls: Optional[Type['ExceptionHandler']] = None):
     """Decorator to use on methods."""
     if handler_cls is not None:
         handler = handler_cls()
@@ -28,6 +28,6 @@ def throws(exception_cls: Type[Exception], handler_cls: Optional[Type['Exception
     return annotate(ExceptionAnnotation(exception_cls, handler), unique=True)
 
 
-def get_throws(method: ComponentMethod) -> Dict[Type[Exception], 'ExceptionHandler']:
+def get_raises(method: ComponentMethod) -> Dict[Type[Exception], 'ExceptionHandler']:
     annotations = method.annotations.get(ExceptionAnnotation)
     return {annotation.exception_cls: annotation.handler for annotation in annotations}

winter/web/exceptions/throttle_exception.py

@@ -3,6 +3,6 @@
 from .problem import problem
 
 
-@problem(status=HTTPStatus.TOO_MANY_REQUESTS, detail='Request was throttled', auto_handle=True)
+@problem(status=HTTPStatus.TOO_MANY_REQUESTS, detail='Request was throttled')
 class ThrottleException(Exception):
     pass

winter/web/pagination/limits.py

@@ -8,7 +8,7 @@
 from ..exceptions import problem
 
 
-@problem(HTTPStatus.BAD_REQUEST, auto_handle=True)
+@problem(HTTPStatus.BAD_REQUEST)
 class MaximumLimitValueExceeded(Exception):
     def __init__(self, maximum_limit: int):
         super().__init__(f'Maximum limit value is exceeded: {maximum_limit}')

winter_openapi/__init__.py

@@ -1,5 +1,9 @@
 from enum import Enum
 
+from winter.web.pagination.limits import MaximumLimitValueExceeded
+from winter.data.pagination import Page
+
+from .annotations.global_exception import register_global_exception
 from .enum_inspector import inspect_enum_class
 from .inspectors import SwaggerAutoSchema
 from .method_arguments_inspector import MethodArgumentsInspector
@@ -12,14 +16,17 @@
 from .type_inspection import TypeInfo
 from .type_inspection import inspect_type
 from .type_inspection import register_type_inspector
+from .validators import validate_missing_raises_annotations
 
 
-def setup():
+def setup(allow_missing_raises_annotation: bool = False):
     from drf_yasg.inspectors.field import hinting_type_info
-    from winter.data.pagination import Page
     from .page_inspector import inspect_page
 
+    register_global_exception(MaximumLimitValueExceeded)
     hinting_type_info.insert(0, (Enum, inspect_enum_class))
     register_type_inspector(Page, func=inspect_page)
     register_controller_method_inspector(PathParametersInspector())
     register_controller_method_inspector(QueryParametersInspector())
+    if not allow_missing_raises_annotation:
+        validate_missing_raises_annotations()

winter_openapi/annotations/__init__.py

@@ -0,0 +1,3 @@
+from .global_exception import GlobalExceptionAnnotation
+from .global_exception import global_exception
+from .global_exception import register_global_exception