handler return json.response

Author: pcrespov

services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py

@@ -1,7 +1,8 @@
 import functools
 import logging
 from contextlib import asynccontextmanager
-from typing import Protocol
+from dataclasses import dataclass
+from typing import AsyncIterator, Protocol
 
 from aiohttp import web
 from servicelib.aiohttp.typing_extension import Handler as WebHandler
@@ -13,55 +14,54 @@
 #
 
 
-class WebApiExceptionHandler(Protocol):
+class AiohttpExceptionHandler(Protocol):
     __name__: str
 
     async def __call__(
         self,
         request: web.Request,
         exception: BaseException,
-    ) -> web.HTTPException | BaseException | None:
+    ) -> web.Response:
         """
         Callback to process an exception raised during a web request, allowing custom handling.
 
-        This function can be implemented to  suppress, reraise, or transform the exception
-        into an `web.HTTPException` (i.e. exceptions defined at the web-api)
+        This function can be implemented to  suppress, (transform &) reraise as web.HTTPError
+        or return a web.Response
 
         Arguments:
             request -- current request
             exception -- exception raised in web handler during this request
-
-        Returns:
-            - None: to suppress `exception`
-            - `exception`: to reraise it
-            - an instance of `web.HTTPException` to transform to HTTP Api exceptions (NOTE: that they can either be errors or success!)
         """
+        ...  # pylint: disable=unnecessary-ellipsis
+
+
+@dataclass
+class _ExceptionContext:
+    response: web.Response | None = None
 
 
 @asynccontextmanager
 async def _handled_exception_context_manager(
     exception_catch: type[BaseException] | tuple[type[BaseException], ...],
-    exception_handler: WebApiExceptionHandler,
+    exception_handler: AiohttpExceptionHandler,
     **forward_ctx,
-):
-    """Calls `exception_handler` on exceptions raised in this context and caught in `exception_catch`"""
+) -> AsyncIterator[_ExceptionContext]:
+    """Calls `exception_handler` on exceptions raised in this context
+    and caught in `exception_catch`
+    """
+    ctx = _ExceptionContext()
     try:
 
-        yield
+        yield ctx
 
     except exception_catch as e:
-        exc = await exception_handler(exception=e, **forward_ctx)
-        if exc:
-            assert isinstance(exc, BaseException)
-            raise exc from e
-
-        _logger.debug(
-            "%s suppressed %s: %s", exception_handler.__name__, type(e).__name__, f"{e}"
-        )
+        response = await exception_handler(exception=e, **forward_ctx)
+        assert isinstance(response, web.Response)  # nosec
+        ctx.response = response
 
 
 def create_decorator_from_exception_handler(
-    exception_handler: WebApiExceptionHandler,
+    exception_handler: AiohttpExceptionHandler,
     exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception,
 ):
     """Returns a decorator for aiohttp's web.Handler functions
@@ -77,9 +77,12 @@ async def _wrapper(request: web.Request) -> web.StreamResponse:
                 exception_types,
                 exception_handler,
                 request=request,
-            ):
+            ) as exc_ctx:
+
                 return await handler(request)
 
+            return exc_ctx.response
+
         return _wrapper
 
     return _decorator

services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py

@@ -7,14 +7,14 @@
 from servicelib.logging_errors import create_troubleshotting_log_kwargs
 from servicelib.status_codes_utils import is_5xx_server_error
 
-from .exceptions_handlers_base import WebApiExceptionHandler
+from .exceptions_handlers_base import AiohttpExceptionHandler
 
 _logger = logging.getLogger(__name__)
 
 
-_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[
+_STATUS_CODE_TO_HTTP_ERRORS: dict[
     int, type[web.HTTPException]
-] = get_all_aiohttp_http_exceptions(web.HTTPException)
+] = get_all_aiohttp_http_exceptions(web.HTTPError)
 
 
 class _DefaultDict(dict):
@@ -37,7 +37,7 @@ def create_exception_handler_from_http_error(
     *,
     status_code: int,
     msg_template: str,
-) -> WebApiExceptionHandler:
+) -> AiohttpExceptionHandler:
     """
     Custom Exception-Handler factory
 
@@ -60,17 +60,14 @@ def create_exception_handler_from_http_error(
     async def _exception_handler(
         request: web.Request,
         exception: BaseException,
-    ) -> web.HTTPException | BaseException | None:
+    ) -> web.Response:
         assert isinstance(exception, exception_cls)  # nosec
 
         # safe formatting, i.e. does not raise
         user_msg = msg_template.format_map(
             _DefaultDict(getattr(exception, "__dict__", {}))
         )
 
-        http_error_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code]
-        assert http_error_cls  # nosec
-
         if is_5xx_server_error(status_code):
             _logger.exception(
                 **create_troubleshotting_log_kwargs(
@@ -84,7 +81,17 @@ async def _exception_handler(
                     },
                 )
             )
-        return http_error_cls(reason=user_msg)
+
+        # TODO: make this part customizable? e.g. so we can inject e.g. oec?
+        # TODO: connect with ErrorModel
+        return web.json_response(
+            {
+                "error": {
+                    "msg": user_msg,
+                }
+            },
+            status=status_code,
+        )
 
     return _exception_handler
 
@@ -101,7 +108,7 @@ def _sort_exceptions_by_specificity(
 
 def create_exception_handler_from_http_error_map(
     exc_to_http_error_map: ExceptionToHttpErrorMap,
-) -> WebApiExceptionHandler:
+) -> AiohttpExceptionHandler:
     """
     Custom Exception-Handler factory
 
@@ -126,7 +133,7 @@ def create_exception_handler_from_http_error_map(
     async def _exception_handler(
         request: web.Request,
         exception: BaseException,
-    ) -> web.HTTPError | BaseException | None:
+    ) -> web.Response:
         if exc_cls := next(
             (_ for _ in _catch_exceptions if isinstance(exception, _)), None
         ):

services/web/server/tests/unit/isolated/test_exceptions_handlers.py

@@ -14,6 +14,7 @@
 from servicelib.aiohttp import status
 from simcore_service_webserver.errors import WebServerBaseError
 from simcore_service_webserver.exceptions_handlers import (
+    ExceptionToHttpErrorMap,
     HttpErrorInfo,
     create_decorator_from_exception_handler,
 )
@@ -29,29 +30,29 @@
 # Some custom errors in my service
 
 
-class BasePluginError(WebServerBaseError):
+class BaseError(WebServerBaseError):
     ...
 
 
-class OneError(BasePluginError):
+class OneError(BaseError):
     ...
 
 
-class OtherError(BasePluginError):
+class OtherError(BaseError):
     ...
 
 
 def test_sort_concrete_first():
-    assert _sort_exceptions_by_specificity([Exception, BasePluginError]) == [
-        BasePluginError,
+    assert _sort_exceptions_by_specificity([Exception, BaseError]) == [
+        BaseError,
         Exception,
     ]
 
     assert _sort_exceptions_by_specificity(
-        [Exception, BasePluginError], concrete_first=False
+        [Exception, BaseError], concrete_first=False
     ) == [
         Exception,
-        BasePluginError,
+        BaseError,
     ]
 
 
@@ -62,7 +63,7 @@ def test_sort_exceptions_by_specificity():
             Exception,
             OtherError,
             OneError,
-            BasePluginError,
+            BaseError,
             ValueError,
             ArithmeticError,
             ZeroDivisionError,
@@ -82,7 +83,6 @@ def fake_request() -> web.Request:
 async def test_factory__create_exception_handler_from_http_error(
     fake_request: web.Request,
 ):
-
     one_error_to_404 = create_exception_handler_from_http_error(
         OneError,
         status_code=status.HTTP_404_NOT_FOUND,
@@ -92,12 +92,14 @@ async def test_factory__create_exception_handler_from_http_error(
     # calling exception handler
     caught = OneError()
     response = await one_error_to_404(fake_request, caught)
-    assert response
     assert response.status == status.HTTP_404_NOT_FOUND
-    assert "one error" in response.text
+    assert response.text is not None
+    assert "one error message" in response.text
 
 
-async def test_create_exception_handler_from_http_error_map(fake_request: web.Request):
+async def test_factory__create_exception_handler_from_http_error_map(
+    fake_request: web.Request,
+):
 
     # call exception handler factory
     exc_handler = create_exception_handler_from_http_error_map(
@@ -109,8 +111,10 @@ async def test_create_exception_handler_from_http_error_map(fake_request: web.Re
     )
 
     # Converts exception in map
-    got_exc = await exc_handler(fake_request, OneError())
+    with pytest.raises(web.HTTPBadRequest) as exc_info:
+        await exc_handler(fake_request, OneError())
 
+    got_exc = exc_info.value
     assert isinstance(got_exc, web.HTTPBadRequest)
     assert got_exc.reason == "Error One mapped to 400"
 
@@ -120,46 +124,53 @@ async def test_create_exception_handler_from_http_error_map(fake_request: web.Re
 
 
 async def test__handled_exception_context_manager(fake_request: web.Request):
-    async def _suppress_handler(request, exception):
+
+    expected_response = web.json_response({"error": {"msg": "Foo"}})
+
+    async def _custom_handler(request, exception):
         assert request == fake_request
         assert isinstance(
-            exception, BasePluginError
+            exception, BaseError
         ), "only BasePluginError exceptions should call this handler"
-        return None  # noqa: RET501, PLR1711
+        return expected_response
 
-    async def _fun(raises):
-        async with _handled_exception_context_manager(
-            BasePluginError, _suppress_handler, request=fake_request
-        ):
-            raise raises
+    exc_handling_ctx = _handled_exception_context_manager(
+        BaseError, _custom_handler, request=fake_request
+    )
+
+    # handles any BaseError returning a response
+    async with exc_handling_ctx as ctx:
+        raise OneError
+    assert ctx.response == expected_response
 
-    # checks
-    await _fun(raises=OneError)
-    await _fun(raises=OtherError)
+    async with exc_handling_ctx as ctx:
+        raise OtherError
+    assert ctx.response == expected_response
 
+    # otherwise thru
     with pytest.raises(ArithmeticError):
-        await _fun(raises=ArithmeticError)
+        async with exc_handling_ctx:
+            raise ArithmeticError
 
 
 async def test_create_decorator_from_exception_handler(
     caplog: pytest.LogCaptureFixture,
 ):
+    # Create an SINGLE exception handler that acts as umbrella for all these exceptions
+    http_error_map: ExceptionToHttpErrorMap = {
+        OneError: HttpErrorInfo(
+            status.HTTP_503_SERVICE_UNAVAILABLE,
+            "Human readable error transmitted to the front-end",
+        )
+    }
 
-    exc_handler = create_exception_handler_from_http_error_map(
-        {
-            OneError: HttpErrorInfo(
-                status.HTTP_503_SERVICE_UNAVAILABLE,
-                "Human readable error transmitted to the front-end",
-            )
-        }
-    )
-
-    _handle_exceptions = create_decorator_from_exception_handler(
-        exception_types=BasePluginError,  # <--- FIXME" this is redundant because exception has been already passed in exc_handler!
+    exc_handler = create_exception_handler_from_http_error_map(http_error_map)
+    _exc_handling_ctx = create_decorator_from_exception_handler(
+        exception_types=BaseError,  # <--- FIXME" this is redundant because exception has been already passed in exc_handler!
         exception_handler=exc_handler,
     )
 
-    @_handle_exceptions
+    @_exc_handling_ctx
     async def _rest_handler(request: web.Request) -> web.Response:
         if request.query.get("raise") == "OneError":
             raise OneError
@@ -193,8 +204,9 @@ async def _rest_handler(request: web.Request) -> web.Response:
         assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE
         assert "front-end" in resp.reason
 
-        assert caplog.records
+        assert caplog.records, "Expected 5XX troubleshooting logged as error"
         assert caplog.records[0].levelno == logging.ERROR
+
     # typically capture by last
     with pytest.raises(ArithmeticError):
         resp = await _rest_handler(