✨ Refactor error handling: improve status message handling in HTTP errors

Author: pcrespov

packages/service-library/src/servicelib/aiohttp/rest_middlewares.py

@@ -18,7 +18,12 @@
 from ..mimetype_constants import MIMETYPE_APPLICATION_JSON
 from ..rest_responses import is_enveloped_from_map, is_enveloped_from_text
 from ..utils import is_production_environ
-from .rest_responses import create_data_response, create_http_error, wrap_as_envelope
+from .rest_responses import (
+    create_data_response,
+    create_http_error,
+    safe_status_message,
+    wrap_as_envelope,
+)
 from .rest_utils import EnvelopeFactory
 from .typing_extension import Handler, Middleware
 
@@ -83,6 +88,8 @@ async def _middleware_handler(request: web.Request, handler: Handler):  # noqa:
         except web.HTTPError as err:
 
             err.content_type = MIMETYPE_APPLICATION_JSON
+            if err.reason:
+                err.set_status(err.status, safe_status_message(message=err.reason))
 
             if not err.text or not is_enveloped_from_text(err.text):
                 error_message = err.text or err.reason or "Unexpected error"
@@ -102,6 +109,9 @@ async def _middleware_handler(request: web.Request, handler: Handler):  # noqa:
 
         except web.HTTPSuccessful as err:
             err.content_type = MIMETYPE_APPLICATION_JSON
+            if err.reason:
+                err.set_status(err.status, safe_status_message(message=err.reason))
+
             if err.text:
                 try:
                     payload = json_loads(err.text)

packages/service-library/src/servicelib/aiohttp/rest_responses.py

@@ -1,10 +1,7 @@
-"""Utils to check, convert and compose server responses for the RESTApi"""
-
-import inspect
 from typing import Any
 
-from aiohttp import web, web_exceptions
-from aiohttp.web_exceptions import HTTPError, HTTPException
+from aiohttp import web
+from aiohttp.web_exceptions import HTTPError
 from common_library.error_codes import ErrorCodeStr
 from common_library.json_serialization import json_dumps
 from models_library.rest_error import ErrorGet, ErrorItemType
@@ -23,36 +20,17 @@ def wrap_as_envelope(
     return {"data": data, "error": error}
 
 
-# RESPONSES FACTORIES -------------------------------
-
-
-def create_data_response(
-    data: Any, *, skip_internal_error_details=False, status=HTTP_200_OK
-) -> web.Response:
-    response = None
-    try:
-        payload = wrap_as_envelope(data) if not is_enveloped(data) else data
-
-        response = web.json_response(payload, dumps=json_dumps, status=status)
-    except (TypeError, ValueError) as err:
-        response = exception_to_response(
-            create_http_error(
-                [
-                    err,
-                ],
-                str(err),
-                web.HTTPInternalServerError,
-                skip_internal_error_details=skip_internal_error_details,
-            )
-        )
-    return response
+def create_data_response(data: Any, *, status: int = HTTP_200_OK) -> web.Response:
+    enveloped_payload = wrap_as_envelope(data) if not is_enveloped(data) else data
+    return web.json_response(enveloped_payload, dumps=json_dumps, status=status)
 
 
 def create_http_error(
     errors: list[Exception] | Exception,
-    reason: str | None = None,
+    error_message: str | None = None,
     http_error_cls: type[HTTPError] = web.HTTPInternalServerError,
     *,
+    status_reason: str | None = None,
     skip_internal_error_details: bool = False,
     error_code: ErrorCodeStr | None = None,
 ) -> HTTPError:
@@ -64,14 +42,16 @@ def create_http_error(
     if not isinstance(errors, list):
         errors = [errors]
 
-    is_internal_error: bool = http_error_cls == web.HTTPInternalServerError
-    default_message = reason or get_code_description(http_error_cls.status_code)
+    is_internal_error = bool(http_error_cls == web.HTTPInternalServerError)
+
+    status_reason = status_reason or get_code_description(http_error_cls.status_code)
+    error_message = error_message or status_reason
 
     if is_internal_error and skip_internal_error_details:
         error = ErrorGet.model_validate(
             {
                 "status": http_error_cls.status_code,
-                "message": default_message,
+                "message": error_message,
                 "support_id": error_code,
             }
         )
@@ -81,7 +61,7 @@ def create_http_error(
             {
                 "errors": items,  # NOTE: deprecated!
                 "status": http_error_cls.status_code,
-                "message": default_message,
+                "message": error_message,
                 "support_id": error_code,
             }
         )
@@ -92,7 +72,7 @@ def create_http_error(
     )
 
     return http_error_cls(
-        reason=safe_status_message(reason),
+        reason=safe_status_message(status_reason),
         text=json_dumps(
             payload,
         ),
@@ -112,24 +92,6 @@ def exception_to_response(exc: HTTPError) -> web.Response:
     )
 
 
-# Inverse map from code to HTTPException classes
-def _collect_http_exceptions(exception_cls: type[HTTPException] = HTTPException):
-    def _pred(obj) -> bool:
-        return (
-            inspect.isclass(obj)
-            and issubclass(obj, exception_cls)
-            and getattr(obj, "status_code", 0) > 0
-        )
-
-    found: list[tuple[str, Any]] = inspect.getmembers(web_exceptions, _pred)
-    assert found  # nosec
-
-    http_statuses = {cls.status_code: cls for _, cls in found}
-    assert len(http_statuses) == len(found), "No duplicates"  # nosec
-
-    return http_statuses
-
-
 def safe_status_message(message: str | None, max_length: int = 50) -> str | None:
     """
     Truncates a status-message (i.e. `reason` in HTTP errors) to a maximum length, replacing newlines with spaces.

packages/service-library/tests/aiohttp/test_rest_responses.py

@@ -63,12 +63,14 @@ def test_collected_http_errors_map(status_code: int, http_error_cls: type[HTTPEr
 @pytest.mark.parametrize("error_code", [None, create_error_code(Exception("fake"))])
 def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | None):
 
-    expected_reason = "Something whent wrong !"
+    expected_status_reason = "SHORT REASON"
+    expected_error_message = "Something whent wrong !"
     expected_exceptions: list[Exception] = [RuntimeError("foo")]
 
     http_error = create_http_error(
         errors=expected_exceptions,
-        reason=expected_reason,
+        error_message=expected_error_message,
+        status_reason="SHORT REASON",
         http_error_cls=web.HTTPInternalServerError,
         skip_internal_error_details=skip_details,
         error_code=error_code,
@@ -94,7 +96,7 @@ def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | N
     # checks response model
     response_json = json.loads(response.text)
     assert response_json["data"] is None
-    assert response_json["error"]["message"] == expected_reason
+    assert response_json["error"]["message"] == expected_error_message
     assert response_json["error"]["supportId"] == error_code
     assert response_json["error"]["status"] == response.status
 

services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py

@@ -30,6 +30,10 @@
 async def _handler_director_service_error_as_503_or_4xx(
     request: web.Request, exception: Exception
 ) -> web.Response:
+    """
+        Handles DirectorV2ServiceError exceptions by returning a 503 Service Unavailable if it's a server error (5XX),
+        or a 4XX client error if it's a client error (4XX).
+    """
     assert isinstance(exception, DirectorV2ServiceError)  # nosec
     assert status_codes_utils.is_error(
         exception.status