Skip to content

Commit b2885ea

Browse files
committed
simplified this version of the exception handling
1 parent 81d8d68 commit b2885ea

File tree

6 files changed

+86
-195
lines changed

6 files changed

+86
-195
lines changed
Lines changed: 43 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import functools
22
import logging
3-
from collections.abc import Callable
4-
from contextlib import contextmanager
5-
from typing import NamedTuple, Protocol, TypeAlias
3+
from typing import NamedTuple, TypeAlias
64

75
from aiohttp import web
86
from servicelib.aiohttp.typing_extension import Handler
@@ -13,74 +11,6 @@
1311
_logger = logging.getLogger(__name__)
1412

1513

16-
class WebApiExceptionHandler(Protocol):
17-
def __call__(
18-
self, exception: BaseException, request: web.Request
19-
) -> web.HTTPError | BaseException | None:
20-
"""
21-
Callback to process an exception raised during a web request, allowing custom handling.
22-
23-
This function can be implemented to suppress, reraise, or transform the exception
24-
into an `web.HTTPError` (i.e.the errors specified in the web-api)
25-
26-
Arguments:
27-
exception -- exception raised in web handler during this request
28-
request -- current request
29-
30-
Returns:
31-
- None: to suppress `exception`
32-
- `exception`: to reraise it
33-
- an instance of `web.HTTPError` to transform to HTTP Api exceptions
34-
"""
35-
36-
37-
@contextmanager
38-
def _handled_exception_context(
39-
exception_catch: type[BaseException] | tuple[type[BaseException], ...],
40-
exception_handler: Callable,
41-
**forward_ctx,
42-
):
43-
try:
44-
yield
45-
except exception_catch as e:
46-
if exc := exception_handler(e, **forward_ctx):
47-
assert isinstance(exc, BaseException)
48-
raise exc from e
49-
50-
_logger.debug(
51-
"%s suppressed %s: %s", exception_handler.__name__, type(e).__name__, f"{e}"
52-
)
53-
54-
55-
def create_exception_handlers_decorator(
56-
exception_handler: WebApiExceptionHandler,
57-
exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception,
58-
):
59-
"""
60-
Creates a function to decorate routes handlers functions
61-
that can catch and handle exceptions raised in the decorated functions
62-
"""
63-
64-
def _decorator(handler: Handler):
65-
@functools.wraps(handler)
66-
async def _wrapper(request: web.Request) -> web.StreamResponse:
67-
with _handled_exception_context(
68-
exception_types,
69-
exception_handler,
70-
request=request,
71-
):
72-
return await handler(request)
73-
74-
return _wrapper
75-
76-
return _decorator
77-
78-
79-
#
80-
# Http Error Map Handler Customization
81-
#
82-
83-
8414
class HttpErrorInfo(NamedTuple):
8515
status_code: int
8616
msg_template: str
@@ -104,57 +34,51 @@ def _sort_exceptions_by_specificity(
10434
)
10535

10636

107-
def create__http_error_map_handler(
108-
to_http_error_map: ExceptionToHttpErrorMap,
109-
) -> WebApiExceptionHandler:
110-
"""
111-
Creates a custom `WebApiExceptionHandler` that maps specific exceptions to HTTP errors.
112-
113-
Given an `ExceptionToHttpErrorMap`, this function returns a handler that checks if an exception
114-
matches one in the map, returning an HTTP error with the mapped status code and message.
115-
Server errors (5xx) include additional logging with request context. Unmapped exceptions are
116-
returned as-is for re-raising.
117-
118-
Arguments:
119-
to_http_error_map -- Maps exceptions to HTTP status codes and messages.
37+
def create_exception_handlers_decorator(
38+
exception_catch: type[BaseException] | tuple[type[BaseException], ...],
39+
exc_to_status_map: ExceptionToHttpErrorMap,
40+
):
12041

121-
Returns:
122-
A web api exception handler
123-
"""
12442
included: list[type[BaseException]] = _sort_exceptions_by_specificity(
125-
list(to_http_error_map.keys())
43+
list(exc_to_status_map.keys())
12644
)
12745

128-
def _handler(
129-
exception: BaseException,
130-
request: web.Request,
131-
) -> web.HTTPError | BaseException | None:
132-
if exc_cls := next((_ for _ in included if isinstance(exception, _)), None):
133-
http_error_info = to_http_error_map[exc_cls]
134-
135-
# safe formatting, i.e. does not raise
136-
user_msg = http_error_info.msg_template.format_map(
137-
_DefaultDict(getattr(exception, "__dict__", {}))
138-
)
139-
140-
http_error_cls = get_http_error_class_or_none(http_error_info.status_code)
141-
assert http_error_cls # nosec
142-
143-
if is_5xx_server_error(http_error_info.status_code):
144-
_logger.exception(
145-
**create_troubleshotting_log_kwargs(
146-
user_msg,
147-
error=exception,
148-
error_context={
149-
"request": request,
150-
"request.remote": f"{request.remote}",
151-
"request.method": f"{request.method}",
152-
"request.path": f"{request.path}",
153-
},
46+
def _decorator(handler: Handler):
47+
@functools.wraps(handler)
48+
async def _wrapper(request: web.Request) -> web.StreamResponse:
49+
try:
50+
return await handler(request)
51+
52+
except exception_catch as exc:
53+
if exc_cls := next((_ for _ in included if isinstance(exc, _)), None):
54+
http_error_info = exc_to_status_map[exc_cls]
55+
56+
# safe formatting, i.e. does not raise
57+
user_msg = http_error_info.msg_template.format_map(
58+
_DefaultDict(getattr(exc, "__dict__", {}))
15459
)
155-
)
156-
return http_error_cls(reason=user_msg)
157-
# NOTE: not in my list, return so it gets reraised
158-
return exception
15960

160-
return _handler
61+
http_error_cls = get_http_error_class_or_none(
62+
http_error_info.status_code
63+
)
64+
assert http_error_cls # nosec
65+
66+
if is_5xx_server_error(http_error_info.status_code):
67+
_logger.exception(
68+
**create_troubleshotting_log_kwargs(
69+
user_msg,
70+
error=exc,
71+
error_context={
72+
"request": request,
73+
"request.remote": f"{request.remote}",
74+
"request.method": f"{request.method}",
75+
"request.path": f"{request.path}",
76+
},
77+
)
78+
)
79+
raise http_error_cls(reason=user_msg) from exc
80+
raise
81+
82+
return _wrapper
83+
84+
return _decorator

services/web/server/src/simcore_service_webserver/folders/_exceptions_handling.py renamed to services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from ..exceptions_handlers import (
66
ExceptionToHttpErrorMap,
77
HttpErrorInfo,
8-
create__http_error_map_handler,
98
create_exception_handlers_decorator,
109
)
1110
from ..projects.exceptions import (
@@ -69,6 +68,6 @@
6968

7069

7170
handle_plugin_requests_exceptions = create_exception_handlers_decorator(
72-
create__http_error_map_handler(_TO_HTTP_ERROR_MAP),
73-
(ProjectTrashError, FoldersValueError),
71+
exception_catch=(ProjectTrashError, FoldersValueError),
72+
exc_to_status_map=_TO_HTTP_ERROR_MAP,
7473
)

services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from ..security.decorators import permission_required
2626
from ..utils_aiohttp import envelope_json_response
2727
from . import _folders_api
28-
from ._exceptions_handling import handle_plugin_requests_exceptions
28+
from ._exceptions_handlers import handle_plugin_requests_exceptions
2929
from ._models import (
3030
FolderFilters,
3131
FolderListWithJsonStrQueryParams,

services/web/server/src/simcore_service_webserver/folders/_trash_handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from ..products.api import get_product_name
1414
from ..security.decorators import permission_required
1515
from . import _trash_api
16-
from ._exceptions_handling import handle_plugin_requests_exceptions
16+
from ._exceptions_handlers import handle_plugin_requests_exceptions
1717
from ._models import FoldersPathParams, RemoveQueryParams
1818

1919
_logger = logging.getLogger(__name__)

services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from ..exceptions_handlers import (
1313
ExceptionToHttpErrorMap,
1414
HttpErrorInfo,
15-
create__http_error_map_handler,
1615
create_exception_handlers_decorator,
1716
)
1817
from ..login.decorators import get_user_id, login_required
@@ -47,7 +46,7 @@
4746

4847

4948
_handle_exceptions = create_exception_handlers_decorator(
50-
create__http_error_map_handler(_TO_HTTP_ERROR_MAP), ProjectTrashError
49+
exception_catch=ProjectTrashError, exc_to_status_map=_TO_HTTP_ERROR_MAP
5150
)
5251

5352
#

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

Lines changed: 38 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,16 @@
66
# pylint: disable=unused-variable
77

88

9+
import logging
10+
911
import pytest
1012
from aiohttp import web
1113
from aiohttp.test_utils import make_mocked_request
1214
from servicelib.aiohttp import status
1315
from simcore_service_webserver.errors import WebServerBaseError
1416
from simcore_service_webserver.exceptions_handlers import (
1517
HttpErrorInfo,
16-
_handled_exception_context,
1718
_sort_exceptions_by_specificity,
18-
create__http_error_map_handler,
1919
create_exception_handlers_decorator,
2020
)
2121

@@ -65,84 +65,53 @@ def test_sort_exceptions_by_specificity():
6565
assert not issubclass(exc_after, exc), f"{got_exceptions_cls=}"
6666

6767

68-
@pytest.fixture
69-
def fake_request() -> web.Request:
70-
return make_mocked_request("GET", "/foo")
71-
72-
73-
def test_http_error_map_handler_factory(fake_request: web.Request):
68+
async def test_exception_handlers_decorator(
69+
caplog: pytest.LogCaptureFixture,
70+
):
7471

75-
exc_handler = create__http_error_map_handler(
76-
{
72+
_handle_exceptions = create_exception_handlers_decorator(
73+
exception_catch=BasePluginError,
74+
exc_to_status_map={
7775
OneError: HttpErrorInfo(
78-
status.HTTP_400_BAD_REQUEST, "Error One mapped to 400"
76+
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
77+
msg_template="This is one error for front-end",
7978
)
80-
}
79+
},
8180
)
8281

83-
# Converts exception in map
84-
got_exc = exc_handler(OneError(), fake_request)
85-
86-
assert isinstance(got_exc, web.HTTPBadRequest)
87-
assert got_exc.reason == "Error One mapped to 400"
88-
89-
# By-passes exceptions not listed
90-
err = RuntimeError()
91-
assert exc_handler(err, fake_request) is err
92-
93-
94-
def test_handled_exception_context(fake_request: web.Request):
95-
def _suppress_handler(exception, request):
96-
assert request == fake_request
97-
assert isinstance(
98-
exception, BasePluginError
99-
), "only BasePluginError exceptions should call this handler"
100-
return None # noqa: RET501, PLR1711
101-
102-
def _fun(raises):
103-
with _handled_exception_context(
104-
BasePluginError, _suppress_handler, request=fake_request
105-
):
106-
raise raises
107-
108-
# checks
109-
_fun(raises=OneError)
110-
_fun(raises=OtherError)
82+
@_handle_exceptions
83+
async def _rest_handler(request: web.Request) -> web.Response:
84+
if request.query.get("raise") == "OneError":
85+
raise OneError
86+
if request.query.get("raise") == "ArithmeticError":
87+
raise ArithmeticError
11188

112-
with pytest.raises(ArithmeticError):
113-
_fun(raises=ArithmeticError)
89+
return web.Response(reason="all good")
11490

91+
with caplog.at_level(logging.ERROR):
11592

116-
async def test_exception_handlers_decorator():
117-
def _suppress_handler(exception, request):
118-
assert isinstance(
119-
exception, BasePluginError
120-
), "only BasePluginError exceptions should call this handler"
121-
return None # noqa: RET501, PLR1711
93+
# emulates successful call
94+
resp = await _rest_handler(make_mocked_request("GET", "/foo"))
95+
assert resp.status == status.HTTP_200_OK
96+
assert resp.reason == "all good"
12297

123-
_handle_exceptons = create_exception_handlers_decorator(
124-
_suppress_handler, BasePluginError
125-
)
98+
assert not caplog.records
12699

127-
@_handle_exceptons
128-
async def _rest_handler(request: web.Request):
129-
if request.query.get("raise") == "OneError":
130-
raise OneError
131-
if request.query.get("raise") == "ArithmeticError":
132-
raise ArithmeticError
100+
# this will be passed and catched by the outermost error middleware
101+
with pytest.raises(ArithmeticError):
102+
await _rest_handler(
103+
make_mocked_request("GET", "/foo?raise=ArithmeticError")
104+
)
133105

134-
return web.Response(text="all good")
106+
assert not caplog.records
135107

136-
# emulates call
137-
resp = await _rest_handler(make_mocked_request("GET", "/foo"))
138-
assert resp.status == status.HTTP_200_OK
108+
# this is a 5XX will be converted to response but is logged as error as well
109+
with pytest.raises(web.HTTPException) as exc_info:
110+
await _rest_handler(make_mocked_request("GET", "/foo?raise=OneError"))
139111

140-
# OMG! not good!?
141-
resp = await _rest_handler(make_mocked_request("GET", "/foo?raise=OneError"))
142-
assert resp is None
112+
resp = exc_info.value
113+
assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE
114+
assert "front-end" in resp.reason
143115

144-
# typically capture by last
145-
with pytest.raises(ArithmeticError):
146-
resp = await _rest_handler(
147-
make_mocked_request("GET", "/foo?raise=ArithmeticError")
148-
)
116+
assert caplog.records
117+
assert caplog.records[0].levelno == logging.ERROR

0 commit comments

Comments
 (0)