diff --git a/README.md b/README.md index 96f0e51f..696e8d23 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,25 @@ app = Application(modules=[service_a], injector=injector) # several application urlpatterns = winter.django.create_urls(app) ``` -- A `@problem` exception gets its handler only when it belongs to an application: define it in a scanned - module or add it, e.g. `app.add(winter.web.problem(status=404)(NotFoundException))`. +- An exception handler is a method marked with `@winter.web.exception_handler` whose first argument is + annotated with the exceptions it handles; the class is created by the injector. `ExceptionHandler`, + `exception_handlers_registry` with `add_handler` and `set_default_handler`, and the handler argument of + `@raises` are gone: `@raises(Exc)` only declares the response for OpenAPI. + + ```python + class FileStorageExceptionHandlers: + @winter.web.exception_handler + @winter.web.response_status(HTTPStatus.BAD_GATEWAY) + def handle_download(self, exception: FileStorageDownloadException) -> ErrorDTO: + ... + ``` + +- Exception handlers belong to applications: a handler, like a `@problem` exception, handles exceptions in the + applications whose definitions include it, in every endpoint, whether declared with `@raises` or not. Define it + in a scanned module or add it, e.g. `app.add(winter.web.problem(status=404)(NotFoundException))`. + The handler of the nearest class in the exception's MRO handles it; two handlers of one exception class in an + application fail on startup. A handler of the application replaces the built-in handler of the same exception + class: a handler of `Exception` replaces the default 500 response. - `winter.django.create_urls` installs the Django throttling identity on its first call: replace it with `winter.web.set_throttling_identity_resolver` after that. - OpenAPI: `generate_openapi(app, title=..., version=...)` returns the spec and checks `@raises` annotations @@ -227,8 +244,6 @@ from dataclasses import dataclass from http import HTTPStatus from typing import List -from django.http import HttpRequest - import winter.web @@ -249,32 +264,41 @@ class TodoNotFoundException(Exception): class TodoNotFoundException(Exception): invalid_index: int -# When we want to override global handler and customize response body. Expected output below: -# {index: 1, 'message': 'Access denied'} +# When we want to build the response body ourselves. Expected output below: +# {'index': 1, 'message': 'Access denied'} +class TodoAccessDeniedException(Exception): + def __init__(self, index: int): + self.index = index + + @dataclass class ErrorDTO: index: int message: str -class TodoNotFoundExceptionCustomHandler(winter.web.ExceptionHandler): - @winter.web.response_status(HTTPStatus.NOT_FOUND) - def handle(self, request: HttpRequest, exception: TodoNotFoundException) -> ErrorDTO: - return ErrorDTO(index=exception.invalid_index, message='Access denied') +class TodoExceptionHandlers: + # Handles the exception its first argument is annotated with, and its subclasses, everywhere in the + # application. The class is created by the injector. A handler of Exception replaces the default 500 response. + @winter.web.exception_handler + @winter.web.response_status(HTTPStatus.FORBIDDEN) + def handle_access_denied(self, exception: TodoAccessDeniedException) -> ErrorDTO: + return ErrorDTO(index=exception.index, message='Access denied') todo_list: List[str] = [] class TodoProblemExistsExampleAPI: - @winter.web.route_get('global/{todo_index}/') - def get_todo_with_global_handling(self, todo_index: int): + @winter.web.route_get('problem/{todo_index}/') + @winter.web.raises(TodoNotFoundException) # Declares the response in the OpenAPI spec + def get_todo(self, todo_index: int): raise TodoNotFoundException(invalid_index=todo_index) - @winter.web.route_get('custom/{todo_index}/') - @winter.web.raises(TodoNotFoundException, handler_cls=TodoNotFoundExceptionCustomHandler) - def get_todo_with_custom_handling(self, todo_index: int): - raise TodoNotFoundException(invalid_index=todo_index) + @winter.web.route_get('denied/{todo_index}/') + @winter.web.raises(TodoAccessDeniedException) + def get_denied_todo(self, todo_index: int): + raise TodoAccessDeniedException(index=todo_index) ``` diff --git a/docs/architecture.md b/docs/architecture.md index 6ac670a9..63d97af4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -81,19 +81,18 @@ at least two of them. ## Current deviations - **Framework behaviour is still process-global.** An `Application` holds the code of an - application and its injector, but the behavioural registries of `winter.web` and `winter.openapi` - are module singletons, and the application installs its injector as the service locator - `winter.application.get_injector`. `tests/test_application_scope.py` states the intended behaviour - and is marked `xfail` until an `Application` instance owns that state. + application and its injector, and the adapters derive its exception handlers from its definitions. + The other behavioural registries of `winter.web` and `winter.openapi` (interceptors, argument + resolvers, output processors, response header serializers, native response types, throttling, + OpenAPI inspectors) are module singletons, and the application installs its injector as the + service locator `winter.application.get_injector`. - **Models are initialised once per process.** `winter.web.initialization.initialize`, the Django adapter's `_initialize_process` and `winter.openapi.initialization.initialize` fill those module - singletons with the standard resolvers, inspectors and handlers on first use, and add the handlers - of each application's `@problem` exceptions to the same global registry. In the target, each + singletons with the standard resolvers and inspectors on first use. In the target, each `Application` owns these registries in an injector of its own, a child of the injector it is given, and they start with the standard entries. Building from an application, an adapter adds its own entries (the Django request resolver, the native response type, the throttling identity) to that - application only, and the `@problem` handlers go to the application's registry. The `initialize` - functions then disappear. + application only. The `initialize` functions then disappear. - **`winter.wsgi`, `winter.asgi` and `winter.rabbitmq` do not exist yet.** HTTP is served through `winter.django` only, and `winter.messaging` has no transport adapter. - **All dependencies are mandatory.** Django, SQLAlchemy, Redis and pydantic are installed with @@ -105,9 +104,8 @@ at least two of them. - **import-linter** (`lint-imports`, configured in `pyproject.toml`) checks the layer rules. There are no `ignore_imports`, and none may be added. -- **Isolation test** `tests/test_application_scope.py` is marked `xfail(strict=True)`. It - builds two applications in one process and asserts that the handler of a `@problem` exception - of one does not handle that exception in the other. `strict` forces the marker to be removed - the moment it passes. +- **Isolation test** `tests/test_application_scope.py` builds two applications in one process + and asserts that the handler of a `@problem` exception of one does not handle that exception in + the other. Each registry that moves into `Application` adds its case there. - **This document** describes intent and is updated when the intent changes, not when the code moves. Deviations are listed above while they exist. diff --git a/tests/api/api_with_exceptions.py b/tests/api/api_with_exceptions.py index 6e751c96..57f38083 100644 --- a/tests/api/api_with_exceptions.py +++ b/tests/api/api_with_exceptions.py @@ -23,28 +23,22 @@ class CustomExceptionDTO: message: str -class CustomExceptionHandler(winter.web.ExceptionHandler): +class ExceptionHandlers: + @winter.web.exception_handler @response_status(400) - def handle(self, exception: CustomException) -> CustomExceptionDTO: + def handle_custom(self, exception: CustomException) -> CustomExceptionDTO: return CustomExceptionDTO(exception.message) - -class AnotherExceptionHandler(winter.web.ExceptionHandler): - @response_status(401) - def handle(self, exception: CustomException) -> int: - return 21 - - -class WithUnknownArgumentExceptionHandler(winter.web.ExceptionHandler): + @winter.web.exception_handler @response_status(400) - def handle(self, exception: WithUnknownArgumentException, unknown_argument: int) -> str: # pragma: no cover + def handle_with_unknown_argument( + self, + exception: WithUnknownArgumentException, + unknown_argument: int, + ) -> str: # pragma: no cover pass -winter.web.exception_handlers_registry.add_handler(CustomException, CustomExceptionHandler) -winter.web.exception_handlers_registry.add_handler(WithUnknownArgumentException, WithUnknownArgumentExceptionHandler) - - class ChildCustomException(CustomException): pass @@ -76,11 +70,6 @@ def not_declared_but_thrown(self) -> str: def declared_but_no_handler(self) -> str: raise ExceptionWithoutHandler() - @raises(CustomException, AnotherExceptionHandler) - @route_get('exception_with_custom_handler/') - def with_custom_handler(self) -> str: - raise CustomException('message') - @raises(WithUnknownArgumentException) @raises(CustomException) # second "raises" just for 100% test coverage @route_get('with_unknown_argument_exception/') diff --git a/tests/api/api_with_problem_exceptions.py b/tests/api/api_with_problem_exceptions.py index 10fda394..864a7dcf 100644 --- a/tests/api/api_with_problem_exceptions.py +++ b/tests/api/api_with_problem_exceptions.py @@ -1,12 +1,11 @@ from http import HTTPStatus -from typing import Dict import dataclasses import winter.web from winter.ddd.exceptions import NotFoundException from winter.web.exceptions import RequestDataDecodeException -from winter.web import route_get, route, raises, response_status +from winter.web import route_get, route, raises @winter.web.problem(status=HTTPStatus.FORBIDDEN) @@ -38,27 +37,10 @@ class AllFieldConstProblemExistsException(Exception): pass -class ProblemExistsExceptionHandler(winter.web.ExceptionHandler): - @response_status(HTTPStatus.FORBIDDEN) - def handle(self, exception: ProblemExistsException, **kwargs) -> Dict: # pragma: no cover - pass - - -@dataclasses.dataclass -class CustomExceptionDTO: - message: str - - class MyEntity: pass -class ProblemExistsExceptionCustomHandler(winter.web.ExceptionHandler): - @response_status(HTTPStatus.BAD_REQUEST) - def handle(self, exception: ProblemExistsException) -> CustomExceptionDTO: - return CustomExceptionDTO(message=str(exception)) - - @route('with-problem-exceptions/') class APIWithProblemExceptions: @@ -81,11 +63,6 @@ def all_field_const_problem_exists_exception(self) -> str: def problem_exists_auto_handle_exception(self) -> str: raise InheritorOfProblemExistsException() - @raises(ProblemExistsException, ProblemExistsExceptionCustomHandler) - @route_get('custom_handler_problem_exists_exception/') - def custom_handler_problem_exists_exception(self) -> str: - raise ProblemExistsException() - @route_get('not_found_exception/') def not_found_exception(self) -> str: raise NotFoundException(entity_id=1, entity_cls=MyEntity) diff --git a/tests/api/api_with_throttling.py b/tests/api/api_with_throttling.py index 5635ff56..84750e3d 100644 --- a/tests/api/api_with_throttling.py +++ b/tests/api/api_with_throttling.py @@ -1,18 +1,10 @@ -from http import HTTPStatus import winter.web -from winter.web import route_post, route_get, raises, response_status, ExceptionHandler +from winter.web import route_post, route_get from winter.web import Request -from winter.web.exceptions import ThrottleException from winter.web.throttling import reset -class CustomThrottleExceptionHandler(ExceptionHandler): - @response_status(HTTPStatus.TOO_MANY_REQUESTS) - def handle(self, exception: ThrottleException) -> str: - return 'custom throttle exception' - - @route_get('with-throttling/') class APIWithThrottling: @@ -34,10 +26,9 @@ def same_simple_method(self) -> int: def method_without_throttling(self): pass - @route_get('custom-handler/') + @route_get('without-user/') @winter.web.throttling('5/s') - @raises(ThrottleException, CustomThrottleExceptionHandler) - def simple_method_with_custom_handler(self) -> int: + def method_for_request_without_user(self) -> int: return 1 @route_get('with-reset/{?is_reset}') diff --git a/tests/apps.py b/tests/apps.py index 5eb27ddc..6d95693f 100644 --- a/tests/apps.py +++ b/tests/apps.py @@ -3,9 +3,7 @@ from tests.web.interceptors import HelloWorldInterceptor import winter.web from winter.web import InMemoryThrottlingStorage -from winter.web import exception_handlers_registry from winter.web import interceptor_registry -from winter.web.exceptions.handlers import DefaultExceptionHandler class TestAppConfig(AppConfig): @@ -19,5 +17,3 @@ def ready(self): # The Redis storage is covered in tests/redis; the sample application throttles in memory. winter.web.set_throttling_storage(InMemoryThrottlingStorage()) - - exception_handlers_registry.set_default_handler(DefaultExceptionHandler) # for 100% test coverage diff --git a/tests/openapi/test_exception_spec.py b/tests/openapi/test_exception_spec.py index 9103429c..0068e53e 100644 --- a/tests/openapi/test_exception_spec.py +++ b/tests/openapi/test_exception_spec.py @@ -3,7 +3,10 @@ import pytest +import winter.openapi import winter.web +from winter.application import Application +from winter.application import get_injector from winter.web.routing import get_route from winter.openapi.generator import generate_openapi_for_routes from winter.openapi.annotations import global_exception @@ -38,7 +41,7 @@ def get_resource(self): # pragma: no cover 'deprecated': False, 'operationId': '_TestAPI.get_resource', 'parameters': [], - 'responses': {'200': {'description': ''}}, + 'responses': {'200': {'description': ''}, '500': {'description': ''}}, 'tags': ['resource'], }, }, @@ -83,33 +86,32 @@ class ProblemExistsExceptionDTO: (None, {'description': ''}, {'parameters': {}, 'responses': {}, 'schemas': {}}), ], ) -def test_exception_annotated_on_method_with_custom_handler_with_dto( +def test_response_of_raised_exception_is_that_of_its_handler_in_application( handler_return_type, handler_spec, expected_components, ): - @winter.web.problem(status=HTTPStatus.FORBIDDEN) class ProblemExistsException(Exception): pass - class ProblemExistsExceptionHandler(winter.web.ExceptionHandler): + class _Handlers: + @winter.web.exception_handler @winter.web.response_status(HTTPStatus.FORBIDDEN) - def handle(self, exception: ProblemExistsException, **kwargs) -> handler_return_type: # pragma: no cover + def handle(self, exception: ProblemExistsException) -> handler_return_type: # pragma: no cover pass class _TestAPI: - @winter.web.raises(ProblemExistsException, ProblemExistsExceptionHandler) + @winter.web.raises(ProblemExistsException) @winter.web.route_get('resource') def get_resource(self): # pragma: no cover pass - route = get_route(_TestAPI.get_resource) + app = Application(modules=[], injector=get_injector()) + app.add(_Handlers) + app.add(_TestAPI) + # Act - result = generate_openapi_for_routes( - title='title', - version='1.0.0', - routes=[route], - ) + result = winter.openapi.generate_openapi(app, title='title', version='1.0.0') # Assert assert result == { 'components': expected_components, diff --git a/tests/test_application_scope.py b/tests/test_application_scope.py index 2f697a59..5ebabf7b 100644 --- a/tests/test_application_scope.py +++ b/tests/test_application_scope.py @@ -1,12 +1,10 @@ """ Isolation test for docs/architecture.md, principle 2: framework behaviour is never process-global. Two applications in one process must not share behaviour: the handler of a @problem exception of one application -does not handle that exception in another. Marked xfail(strict=True) while the exception handlers are -process-global; strict forces the marker to be removed the moment the test passes. +does not handle that exception in another. """ from http import HTTPStatus -import pytest from django.test import RequestFactory import winter.django @@ -26,11 +24,6 @@ def raise_a(self): raise _ErrorA() -@pytest.mark.xfail( - strict=True, - raises=AssertionError, - reason='Isolation: exception handlers are scoped to an Application instance', -) def test_problem_handler_of_one_application_does_not_handle_exceptions_in_another(): app_1 = Application(modules=[], injector=get_injector()) app_1.add(_ErrorA) diff --git a/tests/web/test_exception_handlers.py b/tests/web/test_exception_handlers.py new file mode 100644 index 00000000..ceca9bd2 --- /dev/null +++ b/tests/web/test_exception_handlers.py @@ -0,0 +1,150 @@ +from http import HTTPStatus + +import pytest +from django.test import RequestFactory + +import winter.django +import winter.web +from winter.application import Application +from winter.application import get_injector +from winter.web.exceptions import RedirectException + + +class _BaseError(Exception): + pass + + +class _ChildError(_BaseError): + pass + + +class _GrandChildError(_ChildError): + pass + + +class _OtherError(Exception): + pass + + +def _raising(exception: Exception): + class _API: + @winter.web.route_get('raise/') + def raise_(self): + raise exception + + return _API + + +def _get(*definitions): + app = Application(modules=[], injector=get_injector()) + for definition in definitions: + app.add(definition) + [url] = winter.django.create_urls(app) + return url.callback(RequestFactory().get('/raise/')) + + +class _ExceptionHandler: + @winter.web.exception_handler + @winter.web.response_status(HTTPStatus.SERVICE_UNAVAILABLE) + def handle(self, exception: Exception) -> str: + return 'handled by the application' + + +def test_handler_of_application_replaces_built_in_handler_of_same_exception(): + # Act + response = _get(_raising(_OtherError()), _ExceptionHandler) + + # Assert + assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE + assert response.content == b'"handled by the application"' + + +def test_built_in_handler_of_nearer_exception_is_not_replaced(): + # Act + response = _get(_raising(RedirectException('/elsewhere/')), _ExceptionHandler) + + # Assert + assert response.status_code == HTTPStatus.FOUND + assert response['Location'] == '/elsewhere/' + + +class _Handlers: + @winter.web.exception_handler + def handle_base(self, exception: _BaseError) -> str: + return 'base' + + @winter.web.exception_handler + def handle_child(self, exception: _ChildError) -> str: + return 'child' + + +@pytest.mark.parametrize('exception, expected_content', [ + (_BaseError(), b'"base"'), + (_ChildError(), b'"child"'), + (_GrandChildError(), b'"child"'), +]) +def test_handler_of_nearest_class_in_mro_handles_exception(exception, expected_content): + # Act + response = _get(_raising(exception), _Handlers) + + # Assert + assert response.content == expected_content + + +class _UnionHandler: + @winter.web.exception_handler + def handle(self, error: _ChildError | _OtherError) -> str: + return type(error).__name__ + + +@pytest.mark.parametrize('exception, expected_content', [ + (_ChildError(), b'"_ChildError"'), + (_OtherError(), b'"_OtherError"'), +]) +def test_handler_annotated_with_union_handles_each_exception(exception, expected_content): + # Act + response = _get(_raising(exception), _UnionHandler) + + # Assert + assert response.content == expected_content + + +def test_two_handlers_of_same_exception_in_application_fail_startup(): + class _OtherHandlers: + @winter.web.exception_handler + def handle(self, exception: _BaseError): # pragma: no cover + pass + + # Act + with pytest.raises(ValueError) as exception_info: + _get(_raising(_BaseError()), _Handlers, _OtherHandlers) + + # Assert + assert str(exception_info.value) == '_BaseError has two handlers: _Handlers.handle_base and _OtherHandlers.handle' + + +def _not_annotated(self, exception): # pragma: no cover + pass + + +def _not_an_exception(self, exception: str): # pragma: no cover + pass + + +def _without_arguments(self): # pragma: no cover + pass + + +@pytest.mark.parametrize('handle', [_not_annotated, _not_an_exception, _without_arguments]) +def test_handler_without_exception_as_first_argument_fails_startup(handle): + handlers = type('_InvalidHandlers', (), {'handle': winter.web.exception_handler(handle)}) + + # Act + with pytest.raises(TypeError) as exception_info: + _get(_raising(_BaseError()), handlers) + + # Assert + assert str(exception_info.value) == ( + '_InvalidHandlers.handle: the first argument of an exception handler must be annotated ' + 'with the exceptions it handles' + ) diff --git a/tests/web/test_exception_handling.py b/tests/web/test_exception_handling.py index c0b9ca49..e97df853 100644 --- a/tests/web/test_exception_handling.py +++ b/tests/web/test_exception_handling.py @@ -12,7 +12,7 @@ ('declared_but_not_thrown', HTTPStatus.OK, 'Hello, sir!'), ('declared_and_thrown', HTTPStatus.BAD_REQUEST, {'message': 'declared_and_thrown'}), ('declared_and_thrown_child', HTTPStatus.BAD_REQUEST, {'message': 'declared_and_thrown_child'}), - ('exception_with_custom_handler', HTTPStatus.UNAUTHORIZED, 21), + ('not_declared_but_thrown', HTTPStatus.BAD_REQUEST, {'message': 'not_declared_but_thrown'}), ), ) def test_api_with_exceptions(api_client, url_path, expected_status, expected_body): @@ -28,7 +28,6 @@ def test_api_with_exceptions(api_client, url_path, expected_status, expected_bod @pytest.mark.parametrize( 'url_path', ( - '/with_exceptions/not_declared_but_thrown/', '/with_exceptions/declared_but_no_handler/', '/with_exceptions/with_unknown_argument_exception/', ), @@ -87,12 +86,6 @@ def test_api_with_exceptions_throws_500(api_client, url_path): 'detail': 'Implicit string of detail', }, ), - ( - 'custom_handler_problem_exists_exception', - HTTPStatus.BAD_REQUEST, - 'application/json', - {'message': 'Implicit string of detail'}, - ), ( 'not_found_exception', HTTPStatus.NOT_FOUND, diff --git a/tests/web/test_throttling.py b/tests/web/test_throttling.py index 74b5ba26..d384132e 100644 --- a/tests/web/test_throttling.py +++ b/tests/web/test_throttling.py @@ -21,10 +21,9 @@ @pytest.mark.parametrize('endpoint_url, auth, expected_response', [ ('/with-throttling/', '', expected_error_response), ('/with-throttling/same/', '', expected_error_response), - ('/with-throttling/custom-handler/', 'user_none', 'custom throttle exception'), + ('/with-throttling/without-user/', 'user_none', expected_error_response), ('/with-throttling/', 'user', expected_error_response), ('/with-throttling/same/', 'user', expected_error_response), - ('/with-throttling/custom-handler/', 'user', 'custom throttle exception'), ]) def test_get_throttling(api_client, endpoint_url, auth, expected_response): now = datetime.datetime.now() diff --git a/winter/django/view.py b/winter/django/view.py index 0baa8e13..d6df0ce5 100644 --- a/winter/django/view.py +++ b/winter/django/view.py @@ -16,6 +16,8 @@ from winter.web import register_native_response_type from winter.web import set_throttling_identity_resolver from winter.web.autodiscovery import find_routes +from winter.web.exception_handlers import find_exception_handlers +from winter.web.exceptions import ExceptionHandlers from winter.web.routing import Route from winter.web.urls import rewrite_uritemplate_with_regexps from .http_request_argument_resolver import HttpRequestArgumentResolver @@ -28,9 +30,9 @@ def create_urls(app: Application) -> List[URLPattern]: """URL patterns of the routes of an application, relative to where they are included in the urlpatterns.""" - winter.web.initialization.initialize(app) + winter.web.initialization.initialize() _initialize_process() - return _create_urls_from_routes(find_routes(app)) + return _create_urls_from_routes(find_routes(app), find_exception_handlers(app)) _process_initialized = False @@ -46,7 +48,7 @@ def _initialize_process(): _process_initialized = True -def _create_urls_from_routes(routes: List[Route]) -> List[URLPattern]: +def _create_urls_from_routes(routes: List[Route], exception_handlers: ExceptionHandlers) -> List[URLPattern]: django_urls = [] grouped_routes = defaultdict(list) @@ -54,7 +56,7 @@ def _create_urls_from_routes(routes: List[Route]) -> List[URLPattern]: grouped_routes[route.url_path].append(route) for url_path, routes in grouped_routes.items(): - django_view = _create_django_view_from_routes(routes).as_view() + django_view = _create_django_view_from_routes(routes, exception_handlers).as_view() winter_url_path = f'^{url_path}$' methods = [route.method for route in routes] django_url_path = rewrite_uritemplate_with_regexps(winter_url_path, methods) @@ -66,7 +68,7 @@ def _create_urls_from_routes(routes: List[Route]) -> List[URLPattern]: return django_urls -def _create_django_view_from_routes(routes: List[Route]) -> 'View': +def _create_django_view_from_routes(routes: List[Route], exception_handlers: ExceptionHandlers) -> 'View': from django.views.generic import View class WinterView(View): @@ -75,7 +77,7 @@ def as_view(cls, **initkwargs): return super().as_view(**initkwargs) for route in routes: - dispatch = _create_dispatch_function(route) + dispatch = _create_dispatch_function(route, exception_handlers) dispatch.route = route dispatch_method_name = route.http_method.lower() setattr(WinterView, dispatch_method_name, dispatch) @@ -83,10 +85,10 @@ def as_view(cls, **initkwargs): return WinterView() -def _create_dispatch_function(route: Route): +def _create_dispatch_function(route: Route, exception_handlers: ExceptionHandlers): @wraps(route.method.func) def dispatch(winter_view, request: django.http.HttpRequest, **path_parameters): - response = handle_request(route, DjangoRequest(request, path_parameters)) + response = handle_request(route, DjangoRequest(request, path_parameters), exception_handlers) return to_django_response(response) return dispatch diff --git a/winter/openapi/generator.py b/winter/openapi/generator.py index 525c8ca6..dde31fb6 100644 --- a/winter/openapi/generator.py +++ b/winter/openapi/generator.py @@ -33,7 +33,10 @@ from winter.components import ComponentMethod from winter.web import MediaType from winter.web.default_response_status import get_response_status -from winter.web.exceptions import MethodExceptionsManager +from winter.web.exception_handlers import built_in_exception_handlers +from winter.web.exception_handlers import find_exception_handlers +from winter.web.exceptions import ExceptionHandlers +from winter.web.exceptions.raises import get_raises from winter.web.request_body_annotation import RequestBodyAnnotation from winter.web.routing import Route from winter.web.routing import RouteAnnotation @@ -156,6 +159,7 @@ def generate_openapi( title=title, version=version, routes=find_routes(app), + exception_handlers=find_exception_handlers(app), description=description, tags=tags, validate=validate, @@ -167,12 +171,16 @@ def generate_openapi_for_routes( title: str, version: str, routes: Sequence[Route], + exception_handlers: Optional[ExceptionHandlers] = None, description: Optional[str] = None, tags: Optional[List[Dict[str, Any]]] = None, validate: bool = True, add_url_segment_as_tag: bool = True, ) -> Dict[str, Any]: + """The responses of the raised exceptions come from the exception handlers, the built-in ones by default.""" initialize() + if exception_handlers is None: + exception_handlers = built_in_exception_handlers() routes = list(routes) routes.sort(key=lambda r: r.url_path) schema_registry = SchemaRegistry() @@ -199,6 +207,7 @@ def generate_openapi_for_routes( operation_ids=operation_ids, tag_names=path_tag_names, schema_registry=schema_registry, + exception_handlers=exception_handlers, ) paths[url_path_without_prefix] = path_item @@ -222,6 +231,7 @@ def _get_openapi_path( operation_ids: Set[str], tag_names: Iterable[str], schema_registry: SchemaRegistry, + exception_handlers: ExceptionHandlers, ) -> PathItem: path = {} for route in routes: @@ -236,6 +246,7 @@ def _get_openapi_path( operation_id=operation_id, tag_names=tag_names, schema_registry=schema_registry, + exception_handlers=exception_handlers, ) except InspectorNotFound as e: raise CanNotInspectType(route.method, str(e)) @@ -250,12 +261,13 @@ def _get_openapi_operation( operation_id: str, tag_names: Iterable[str], schema_registry: SchemaRegistry, + exception_handlers: ExceptionHandlers, ) -> Operation: summary = route.method.docstring.short_description description = route.method.docstring.long_description operation_parameters = get_route_parameters(route, schema_registry) operation_request_body = get_request_body_parameters(route, schema_registry) - operation_responses = get_responses_schemas(route, schema_registry) + operation_responses = get_responses_schemas(route, schema_registry, exception_handlers) return Operation( tags=tag_names, summary=summary, @@ -343,17 +355,19 @@ def get_request_body_parameters(route: Route, schema_registry: SchemaRegistry) - return RequestBody(description=description, content=content) -def get_responses_schemas(route: Route, schema_registry: SchemaRegistry) -> Responses: +def get_responses_schemas( + route: Route, + schema_registry: SchemaRegistry, + exception_handlers: ExceptionHandlers, +) -> Responses: responses: Responses = {} http_method = route.http_method response_status = str(get_response_status(http_method, route.method)) responses[response_status] = _build_response_schema(route.method, schema_registry) - method_exceptions_manager = MethodExceptionsManager(route.method) - for exception_cls in method_exceptions_manager.declared_exception_classes: - handler = method_exceptions_manager.get_handler(exception_cls) - handle_method = ComponentMethod.get_or_create(handler.__class__.handle) + for exception_cls in get_raises(route.method): + handle_method = exception_handlers.get(exception_cls) response_status = str(get_response_status(http_method, handle_method)) responses[response_status] = _build_response_exception_handler_schema(handle_method, schema_registry) return responses diff --git a/winter/web/__init__.py b/winter/web/__init__.py index 409df52e..9f7ab1d1 100644 --- a/winter/web/__init__.py +++ b/winter/web/__init__.py @@ -3,8 +3,7 @@ from .argument_resolver import ArgumentResolver from .argument_resolver import ArgumentsResolver from .argument_resolver import arguments_resolver -from .exceptions import ExceptionHandler -from .exceptions import exception_handlers_registry +from .exceptions import exception_handler from .exceptions import problem from .exceptions.raises import raises from .interceptor import Interceptor diff --git a/winter/web/exception_handlers.py b/winter/web/exception_handlers.py index e9c13ab3..ee0f2268 100644 --- a/winter/web/exception_handlers.py +++ b/winter/web/exception_handlers.py @@ -1,14 +1,75 @@ +from functools import cache from http import HTTPStatus +from typing import Iterable +from typing import Type -from .exceptions import ExceptionHandler +from winter.application import Application +from winter.components import Component from .exceptions import RedirectException +from .exceptions import RequestDataDecodeException +from .exceptions import ThrottleException +from .exceptions import UnsupportedMediaTypeException +from .exceptions.handlers import ExceptionHandlers +from .exceptions.handlers import exception_handler +from .exceptions.handlers import find_handlers +from .exceptions.problem_annotation import ProblemAnnotation +from .exceptions.problem_handling import ProblemExceptionHandlerGenerator +from .exceptions.problem_handling import ProblemExceptionMapper +from .exceptions.problem_handling_info import ProblemHandlingInfo +from .pagination.limits import MaximumLimitValueExceeded +from .response import Response from .response_header_annotation import ResponseHeader from .response_header_annotation import response_header from .response_status_annotation import response_status -class RedirectExceptionHandler(ExceptionHandler): +class BuiltInExceptionHandlers: + @exception_handler + @response_status(HTTPStatus.INTERNAL_SERVER_ERROR) + def handle(self, exception: Exception) -> Response: + return Response(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, body=b'Server Error (500)') + + @exception_handler @response_status(HTTPStatus.FOUND) @response_header('Location', 'location_header') - def handle(self, exception: RedirectException, location_header: ResponseHeader[str]): + def handle_redirect(self, exception: RedirectException, location_header: ResponseHeader[str]): location_header.set(exception.redirect_to) + + +# The exceptions that winter.web itself raises, and the last resort for any other one +_BUILT_IN = ( + BuiltInExceptionHandlers, + ThrottleException, + RequestDataDecodeException, + UnsupportedMediaTypeException, + MaximumLimitValueExceeded, +) +_problem_handler_generator = ProblemExceptionHandlerGenerator(ProblemExceptionMapper()) + + +def find_exception_handlers(app: Application) -> ExceptionHandlers: + """ + The exception handlers of an application: the @exception_handler methods and the handlers of the @problem + exceptions among its definitions, over the built-in ones. A handler of the application replaces the built-in + handler of the same exception class. + """ + return ExceptionHandlers({**_handlers_in(_BUILT_IN), **_handlers_in(app.definitions)}) + + +def built_in_exception_handlers() -> ExceptionHandlers: + return ExceptionHandlers(_handlers_in(_BUILT_IN)) + + +def _handlers_in(definitions: Iterable) -> dict: + return find_handlers([_problem_handler(definition) or definition for definition in definitions]) + + +def _problem_handler(definition) -> Type | None: + component = Component.get_all().get(definition) + problem_annotation = component and component.annotations.get_one_or_none(ProblemAnnotation) + return problem_annotation and _generate_problem_handler(definition, problem_annotation.handling_info) + + +@cache +def _generate_problem_handler(exception_class: Type[Exception], handling_info: ProblemHandlingInfo) -> Type: + return _problem_handler_generator.generate(exception_class, handling_info) diff --git a/winter/web/exceptions/__init__.py b/winter/web/exceptions/__init__.py index f760660f..9b16a20c 100644 --- a/winter/web/exceptions/__init__.py +++ b/winter/web/exceptions/__init__.py @@ -4,8 +4,7 @@ from .exceptions import ThrottleException from .exceptions import UnsupportedMediaTypeException from .exceptions import RequestDataDecodeException -from .handlers import ExceptionHandler -from .handlers import MethodExceptionsManager -from .handlers import exception_handlers_registry +from .handlers import ExceptionHandlers +from .handlers import exception_handler from .problem import problem from .problem_handling_info import ProblemHandlingInfo diff --git a/winter/web/exceptions/exception_handler_generator.py b/winter/web/exceptions/exception_handler_generator.py index 07ab1f91..75de8e56 100644 --- a/winter/web/exceptions/exception_handler_generator.py +++ b/winter/web/exceptions/exception_handler_generator.py @@ -2,11 +2,10 @@ from abc import abstractmethod from typing import Type -from .handlers import ExceptionHandler from .problem_handling_info import ProblemHandlingInfo class ExceptionHandlerGenerator(ABC): @abstractmethod - def generate(self, exception_class: Type[Exception], handling_info: ProblemHandlingInfo) -> Type[ExceptionHandler]: - pass + def generate(self, exception_class: Type[Exception], handling_info: ProblemHandlingInfo) -> Type: + """A class with an @exception_handler method that handles the exception class.""" diff --git a/winter/web/exceptions/handlers.py b/winter/web/exceptions/handlers.py index cb103cfa..95eebafc 100644 --- a/winter/web/exceptions/handlers.py +++ b/winter/web/exceptions/handlers.py @@ -1,93 +1,74 @@ -import abc -from http import HTTPStatus +import inspect +from dataclasses import dataclass from typing import Dict +from typing import Iterable from typing import Tuple from typing import Type -from typing import Union - +from winter.components import Component from winter.components import ComponentMethod -from .raises import get_raises -from ..response import Response - -NotHandled = object() - - -class ExceptionHandler(abc.ABC): - @abc.abstractmethod - def handle(self, exception: Exception, **kwargs): # pragma: no cover - pass - - -class DefaultExceptionHandler(ExceptionHandler): - def handle(self, exception: Exception, **kwargs) -> Response: - return Response(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, body=b'Server Error (500)') - - -class ExceptionHandlersRegistry: - HandlersMap = Dict[Type[Exception], ExceptionHandler] - - def __init__(self): - self._handlers: ExceptionHandlersRegistry.HandlersMap = {} - self._auto_handle_exceptions = set() - self._default_handler = DefaultExceptionHandler() - super().__init__() - - def add_handler( - self, - exception_cls: Type[Exception], - handler_cls: Type[ExceptionHandler], - *, - auto_handle: bool = False, - ): - assert exception_cls not in self._handlers, exception_cls - self._handlers[exception_cls] = handler_cls() - - if auto_handle: - self._auto_handle_exceptions.add(exception_cls) - - def get_handler( - self, - exception: Union[Type[Exception], Exception], - auto_handled_only: bool = False, - ) -> ExceptionHandler: - exception_type = type(exception) if isinstance(exception, Exception) else exception - - for exception_cls, handler in self._handlers.items(): - if issubclass(exception_type, exception_cls) and ( - not auto_handled_only or exception_cls in self._auto_handle_exceptions - ): - return handler - - return self._default_handler - - def get_default_handler(self) -> ExceptionHandler: - return self._default_handler - - def set_default_handler(self, handler_cls: Type[ExceptionHandler]): - self._default_handler = handler_cls() - - -class MethodExceptionsManager: - def __init__(self, method: ComponentMethod): - super().__init__() - self._method = method - self._handlers_by_exception = get_raises(self._method) - - @property - def declared_exception_classes(self) -> Tuple[Type[Exception], ...]: - return tuple(self._handlers_by_exception.keys()) - - def get_handler(self, exception: Union[Type[Exception], Exception]) -> ExceptionHandler: - exception_type = type(exception) if isinstance(exception, Exception) else exception - - for exception_cls, handler in self._handlers_by_exception.items(): - if issubclass(exception_type, exception_cls): - if handler is not None: - return handler - return exception_handlers_registry.get_handler(exception) - - return exception_handlers_registry.get_handler(exception, auto_handled_only=True) - - -exception_handlers_registry = ExceptionHandlersRegistry() +from winter.components import annotate_method +from winter.typing import get_union_args +from winter.typing import is_union + + +@dataclass(frozen=True) +class ExceptionHandlerAnnotation: + pass + + +def exception_handler(method): + """ + Marks a method as an exception handler. It handles the exceptions its first argument is annotated with, + `UploadError | DownloadError` for several, in the applications whose definitions include its class. + """ + return annotate_method(ExceptionHandlerAnnotation(), single=True)(method) + + +class ExceptionHandlers: + """The exception handlers of an application. The handler of the nearest class in its MRO handles an exception.""" + + def __init__(self, handlers: Dict[Type[Exception], ComponentMethod]): + assert Exception in handlers, 'Every exception must have a handler' + self._handlers = dict(handlers) + + def get(self, exception_cls: Type[Exception]) -> ComponentMethod: + return next(self._handlers[cls] for cls in exception_cls.__mro__ if cls in self._handlers) + + +def find_handlers(classes: Iterable[type]) -> Dict[Type[Exception], ComponentMethod]: + """The handlers marked with @exception_handler in the classes, by the exception they handle.""" + handlers: Dict[Type[Exception], ComponentMethod] = {} + components = Component.get_all() + for cls in classes: + component = components.get(cls) + methods = component.methods if component is not None else () + for method in methods: + if method.annotations.get_one_or_none(ExceptionHandlerAnnotation) is None: + continue + for exception_cls in handled_exceptions(method): + if exception_cls in handlers: + raise ValueError( + f'{exception_cls.__name__} has two handlers: {handlers[exception_cls].full_name} ' + f'and {method.full_name}', + ) + handlers[exception_cls] = method + return handlers + + +def handled_exceptions(method: ComponentMethod) -> Tuple[Type[Exception], ...]: + argument = method.get_argument(exception_argument_name(method)) + type_ = argument.type_ if argument is not None else None + exception_classes = tuple(get_union_args(type_)) if is_union(type_) else (type_,) + if not all(inspect.isclass(cls) and issubclass(cls, Exception) for cls in exception_classes): + raise TypeError( + f'{method.full_name}: the first argument of an exception handler must be annotated ' + f'with the exceptions it handles', + ) + return exception_classes + + +def exception_argument_name(method: ComponentMethod) -> str: + """The name of the argument that receives the exception: the first one after self.""" + parameters = list(method.signature.parameters) + return parameters[1] if len(parameters) > 1 else '' diff --git a/winter/web/exceptions/problem_handling.py b/winter/web/exceptions/problem_handling.py index 1b92d34b..06a72107 100644 --- a/winter/web/exceptions/problem_handling.py +++ b/winter/web/exceptions/problem_handling.py @@ -4,19 +4,13 @@ from dataclasses import dataclass from dataclasses import is_dataclass from typing import Dict -from typing import Iterable -from typing import Set from typing import Type - -from winter.components import Component from .beautify_string import camel_to_human from winter.web import ResponseHeader from .exception_handler_generator import ExceptionHandlerGenerator from .exception_mapper import ExceptionMapper -from .handlers import ExceptionHandler -from .handlers import exception_handlers_registry -from .problem_annotation import ProblemAnnotation +from .handlers import exception_handler from .problem_handling_info import ProblemHandlingInfo from .. import MediaType from ..request import Request @@ -27,18 +21,19 @@ class ProblemExceptionHandlerGenerator(ExceptionHandlerGenerator): def __init__(self, exception_mapper: ExceptionMapper): self._exception_mapper = exception_mapper - def generate(self, exception_class: Type[Exception], handling_info: ProblemHandlingInfo) -> Type[ExceptionHandler]: + def generate(self, exception_class: Type[Exception], handling_info: ProblemHandlingInfo) -> Type: from ..response_status_annotation import response_status return_type_class = self._build_exception_dataclass(exception_class) exception_mapper = self._exception_mapper + @exception_handler @response_status(handling_info.status) @response_header('Content-Type', 'content_type_header') - def handle_method( + def handle( self, - request: Request, exception: exception_class, + request: Request, content_type_header: ResponseHeader[str], **kwargs, ) -> return_type_class: @@ -46,8 +41,7 @@ def handle_method( return exception_mapper.to_response_body(request, exception, handling_info) handler_class_name = exception_class.__name__ + 'Handler' - handler_class = type(handler_class_name, (ExceptionHandler,), {'handle': handle_method}) - return handler_class + return type(handler_class_name, (), {'handle': handle}) def _build_exception_dataclass(self, exception_class: Type[Exception]) -> Type: class_name = exception_class.__name__ @@ -93,19 +87,3 @@ def _generate_type_value(cls, exception_cls: Type[Exception]) -> str: @classmethod def _try_cut_exception_name_postfix(cls, exception_cls: Type[Exception]) -> str: return re.sub('Exception$', '', exception_cls.__name__) - - -_problems_with_handlers: Set[Type[Exception]] = set() - - -def add_problem_handlers(definitions: Iterable, handler_generator: ProblemExceptionHandlerGenerator): - """Registers a handler for each @problem exception among the definitions that has none yet.""" - components = Component.get_all() - for definition in definitions: - component = components.get(definition) - problem_annotation = component and component.annotations.get_one_or_none(ProblemAnnotation) - if problem_annotation is None or definition in _problems_with_handlers: - continue - handler_class = handler_generator.generate(definition, problem_annotation.handling_info) - exception_handlers_registry.add_handler(definition, handler_class, auto_handle=True) - _problems_with_handlers.add(definition) diff --git a/winter/web/exceptions/raises.py b/winter/web/exceptions/raises.py index ddf7e7a1..32ef626c 100644 --- a/winter/web/exceptions/raises.py +++ b/winter/web/exceptions/raises.py @@ -1,6 +1,4 @@ -from typing import Dict -from typing import Optional -from typing import TYPE_CHECKING +from typing import Tuple from typing import Type import dataclasses @@ -8,26 +6,16 @@ from winter.components import ComponentMethod from winter.components import annotate -if TYPE_CHECKING: - from .handlers import ExceptionHandler # noqa: F401 - @dataclasses.dataclass class ExceptionAnnotation: exception_cls: Type[Exception] - handler: Optional['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() - else: - handler = None - return annotate(ExceptionAnnotation(exception_cls, handler), unique=True) +def raises(exception_cls: Type[Exception]): + """Declares an exception that the method raises, for its OpenAPI responses. Its handler is the application's.""" + return annotate(ExceptionAnnotation(exception_cls), unique=True) -def get_raises(method: ComponentMethod) -> Dict[Type[Exception], 'ExceptionHandler']: - annotations = method.annotations.get(ExceptionAnnotation) - return {annotation.exception_cls: annotation.handler for annotation in annotations} +def get_raises(method: ComponentMethod) -> Tuple[Type[Exception], ...]: + return tuple(annotation.exception_cls for annotation in method.annotations.get(ExceptionAnnotation)) diff --git a/winter/web/initialization.py b/winter/web/initialization.py index a0a57209..cc14f377 100644 --- a/winter/web/initialization.py +++ b/winter/web/initialization.py @@ -1,16 +1,5 @@ -from winter.application import Application from .argument_resolver import arguments_resolver -from .exception_handlers import RedirectExceptionHandler -from .exceptions import RedirectException -from .exceptions import RequestDataDecodeException -from .exceptions import ThrottleException -from .exceptions import UnsupportedMediaTypeException -from .exceptions import exception_handlers_registry -from .exceptions.problem_handling import ProblemExceptionHandlerGenerator -from .exceptions.problem_handling import ProblemExceptionMapper -from .exceptions.problem_handling import add_problem_handlers from .output_processor import register_output_processor_resolver -from .pagination.limits import MaximumLimitValueExceeded from .pagination.page_position_argument_resolver import PagePositionArgumentResolver from .pagination.page_processor_resolver import PageOutputProcessorResolver from .path_parameters_argument_resolver import PathParametersArgumentResolver @@ -24,23 +13,18 @@ from .response_header_serializers import DateTimeResponseHeaderSerializer from .response_header_serializers import LastModifiedResponseHeaderSerializer -_problem_handler_generator = ProblemExceptionHandlerGenerator(ProblemExceptionMapper()) _process_initialized = False -def initialize(app: Application): +def initialize(): """ - Prepares winter.web to handle the requests of an application. Adapters call it before building their routes. - - The standard resolvers are registered on the first call, the handlers of the application's @problem - exceptions on every call. + Registers the standard resolvers of winter.web on the first call. Adapters call it before building their routes. Process-global until the registries move into Application: docs/architecture.md, Current deviations. """ global _process_initialized if not _process_initialized: _initialize_process() _process_initialized = True - add_problem_handlers(app.definitions, _problem_handler_generator) def _initialize_process(): @@ -55,11 +39,3 @@ def _initialize_process(): arguments_resolver.add_argument_resolver(PagePositionArgumentResolver()) arguments_resolver.add_argument_resolver(RequestHeaderArgumentResolver()) arguments_resolver.add_argument_resolver(RequestArgumentResolver()) - framework_problems = [ - ThrottleException, - RequestDataDecodeException, - UnsupportedMediaTypeException, - MaximumLimitValueExceeded, - ] - add_problem_handlers(framework_problems, _problem_handler_generator) - exception_handlers_registry.add_handler(RedirectException, RedirectExceptionHandler, auto_handle=True) diff --git a/winter/web/request_handler.py b/winter/web/request_handler.py index 9e087d50..b8b8a52f 100644 --- a/winter/web/request_handler.py +++ b/winter/web/request_handler.py @@ -6,9 +6,9 @@ from winter.json import JSONEncoder from .argument_resolver import arguments_resolver from .default_response_status import get_response_status -from .exceptions import MethodExceptionsManager +from .exceptions import ExceptionHandlers from .exceptions import ThrottleException -from .exceptions import exception_handlers_registry +from .exceptions.handlers import exception_argument_name from .interceptor import interceptor_registry from .output_processor import get_output_processor from .request import Request @@ -19,14 +19,13 @@ from .throttling import create_throttle_class -def handle_request(route: Route, request: Request) -> Response: +def handle_request(route: Route, request: Request, exception_handlers: ExceptionHandlers) -> Response: """ Run the whole handling pipeline for a matched route: throttling, interceptors, argument resolution, - the API method, exception handlers, output processing and JSON encoding. Framework-agnostic: adapters - build the Request from their own request object and send the returned Response. + the API method, the application's exception handlers, output processing and JSON encoding. + Framework-agnostic: adapters build the Request from their own request object and send the returned Response. """ method = route.method - method_exceptions_manager = MethodExceptionsManager(method) response_headers: dict[str, Any] = {} throttle = create_throttle_class(route) @@ -43,24 +42,29 @@ def handle_request(route: Route, request: Request) -> Response: api_instance = get_injector().get(method.component.component_cls) result = method(api_instance, **arguments) except Exception as exception: - handler = method_exceptions_manager.get_handler(exception) - method = ComponentMethod.get_or_create(handler.__class__.handle) + method = exception_handlers.get(type(exception)) try: - arguments = arguments_resolver.resolve_arguments( - method, request, response_headers, {'exception': exception, 'response_headers': response_headers}, - ) - result = method(handler, **arguments) - except Exception as inner_exception: - handler = exception_handlers_registry.get_default_handler() - method = ComponentMethod.get_or_create(handler.__class__.handle) - arguments = arguments_resolver.resolve_arguments( - method, request, response_headers, {'exception': inner_exception, 'response_headers': response_headers}, - ) - result = method(handler, **arguments) + result = _handle_exception(method, exception, request, response_headers) + except Exception as handler_exception: + # A failing handler is not retried: its exception goes to the handler of Exception + method = exception_handlers.get(Exception) + result = _handle_exception(method, handler_exception, request, response_headers) return _to_response(request, result, method, response_headers) +def _handle_exception( + method: ComponentMethod, + exception: Exception, + request: Request, + response_headers: dict[str, Any], +) -> Any: + context = {exception_argument_name(method): exception, 'response_headers': response_headers} + arguments = arguments_resolver.resolve_arguments(method, request, response_headers, context) + handler = get_injector().get(method.component.component_cls) + return method(handler, **arguments) + + def _to_response(request: Request, result: Any, method: ComponentMethod, response_headers: dict[str, Any]) -> Response: headers = { name: response_headers_serializer.serialize(value, name)