✨ Enhance response handling: streamline data response creation and add safe status message truncation

Author: pcrespov

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

@@ -186,10 +186,7 @@ async def _middleware_handler(
             return resp
 
         if not isinstance(resp, StreamResponse):
-            resp = create_data_response(
-                data=resp,
-                skip_internal_error_details=_is_prod,
-            )
+            resp = create_data_response(data=resp)
 
         assert isinstance(resp, web.StreamResponse)  # nosec
         return resp

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

@@ -1,4 +1,4 @@
-from typing import Any
+from typing import Any, Final, TypedDict
 
 from aiohttp import web
 from aiohttp.web_exceptions import HTTPError
@@ -10,21 +10,65 @@
 from ..mimetype_constants import MIMETYPE_APPLICATION_JSON
 from ..rest_constants import RESPONSE_MODEL_POLICY
 from ..rest_responses import is_enveloped
-from ..status_codes_utils import get_code_description
+from ..status_codes_utils import get_code_description, is_error
+
+
+class EnvelopeDict(TypedDict):
+    data: Any
+    error: Any
 
 
 def wrap_as_envelope(
-    data: Any = None,
-    error: Any = None,
-) -> dict[str, Any]:
+    data: Any | None = None,
+    error: Any | None = None,
+) -> EnvelopeDict:
     return {"data": data, "error": error}
 
 
 def create_data_response(data: Any, *, status: int = HTTP_200_OK) -> web.Response:
+    """Creates a JSON response with the given data and ensures it is wrapped in an envelope."""
+
+    assert (  # nosec
+        is_error(status) is False
+    ), f"Expected a non-error status code, got {status=}"
+
     enveloped_payload = wrap_as_envelope(data) if not is_enveloped(data) else data
     return web.json_response(enveloped_payload, dumps=json_dumps, status=status)
 
 
+MAX_STATUS_MESSAGE_LENGTH: Final[int] = 50
+
+
+def safe_status_message(
+    message: str | None, max_length: int = MAX_STATUS_MESSAGE_LENGTH
+) -> str | None:
+    """
+    Truncates a status-message (i.e. `reason` in HTTP errors) to a maximum length, replacing newlines with spaces.
+
+    If the message is longer than max_length, it will be truncated and "..." will be appended.
+
+    This prevents issues such as:
+        - `aiohttp.http_exceptions.LineTooLong`: 400, message: Got more than 8190 bytes when reading Status line is too long.
+        - Multiline not allowed in HTTP reason attribute (aiohttp now raises ValueError).
+
+    See:
+        - When to use http status and/or text messages https://github.com/ITISFoundation/osparc-simcore/pull/7760
+        - [RFC 9112, Section 4.1: HTTP/1.1 Message Syntax and Routing](https://datatracker.ietf.org/doc/html/rfc9112#section-4.1) (status line length limits)
+        - [RFC 9110, Section 15.5: Reason Phrase](https://datatracker.ietf.org/doc/html/rfc9110#section-15.5) (reason phrase definition)
+    """
+    assert max_length > 0  # nosec
+
+    if not message:
+        return None
+
+    flat_message = message.replace("\n", " ")
+    if len(flat_message) <= max_length:
+        return flat_message
+
+    # Truncate and add ellipsis
+    return flat_message[: max_length - 3] + "..."
+
+
 def create_http_error(
     errors: list[Exception] | Exception,
     error_message: str | None = None,
@@ -45,7 +89,9 @@ def create_http_error(
     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
+    error_message = error_message or get_code_description(http_error_cls.status_code)
+
+    assert len(status_reason) < MAX_STATUS_MESSAGE_LENGTH  # nosec
 
     if is_internal_error and skip_internal_error_details:
         error = ErrorGet.model_validate(
@@ -90,29 +136,3 @@ def exception_to_response(exc: HTTPError) -> web.Response:
         reason=exc.reason,
         text=exc.text,
     )
-
-
-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.
-
-    If the message is longer than max_length, it will be truncated and "..." will be appended.
-
-    This prevents issues such as:
-        - `aiohttp.http_exceptions.LineTooLong`: 400, message: Got more than 8190 bytes when reading Status line is too long.
-        - Multiline not allowed in HTTP reason attribute (aiohttp now raises ValueError).
-
-    See:
-        - When to use http status and/or text messages https://github.com/ITISFoundation/osparc-simcore/pull/7760
-        - [RFC 9112, Section 4.1: HTTP/1.1 Message Syntax and Routing](https://datatracker.ietf.org/doc/html/rfc9112#section-4.1) (status line length limits)
-        - [RFC 9110, Section 15.5: Reason Phrase](https://datatracker.ietf.org/doc/html/rfc9110#section-15.5) (reason phrase definition)
-    """
-    if not message:
-        return None
-
-    flat_message = message.replace("\n", " ")
-    if len(flat_message) <= max_length:
-        return flat_message
-
-    # Truncate and add ellipsis
-    return flat_message[: max_length - 3] + "..."

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

@@ -17,7 +17,6 @@
     HTTPOk,
 )
 from common_library.error_codes import ErrorCodeStr, create_error_code
-from servicelib.aiohttp import status
 from servicelib.aiohttp.rest_responses import create_http_error, exception_to_response
 from servicelib.aiohttp.web_exceptions_extension import (
     _STATUS_CODE_TO_HTTP_ERRORS,
@@ -61,18 +60,43 @@ def test_collected_http_errors_map(status_code: int, http_error_cls: type[HTTPEr
 
 @pytest.mark.parametrize("skip_details", [True, False])
 @pytest.mark.parametrize("error_code", [None, create_error_code(Exception("fake"))])
-def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | None):
-
+@pytest.mark.parametrize(
+    "http_error_cls",
+    [
+        web.HTTPBadRequest,  # 400
+        web.HTTPUnauthorized,  # 401
+        web.HTTPForbidden,  # 403
+        web.HTTPNotFound,  # 404
+        web.HTTPGone,  # 410
+        web.HTTPInternalServerError,  # 500
+        web.HTTPBadGateway,  # 502
+        web.HTTPServiceUnavailable,  # 503
+    ],
+    ids=[
+        "400",
+        "401",
+        "403",
+        "404",
+        "410",
+        "500",
+        "502",
+        "503",
+    ],
+)
+def tests_exception_to_response(
+    skip_details: bool, error_code: ErrorCodeStr | None, http_error_cls: type[HTTPError]
+):
     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,
         error_message=expected_error_message,
-        status_reason="SHORT REASON",
-        http_error_cls=web.HTTPInternalServerError,
-        skip_internal_error_details=skip_details,
+        status_reason=expected_status_reason,
+        http_error_cls=http_error_cls,
+        skip_internal_error_details=skip_details
+        and (http_error_cls == web.HTTPInternalServerError),
         error_code=error_code,
     )
 
@@ -89,7 +113,7 @@ def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | N
 
     # checks response components
     assert response.content_type == MIMETYPE_APPLICATION_JSON
-    assert response.status == status.HTTP_500_INTERNAL_SERVER_ERROR
+    assert response.status == http_error_cls.status_code
     assert response.text
     assert response.body
 
@@ -99,6 +123,7 @@ def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | N
     assert response_json["error"]["message"] == expected_error_message
     assert response_json["error"]["supportId"] == error_code
     assert response_json["error"]["status"] == response.status
+    assert response.reason == expected_status_reason
 
 
 @pytest.mark.parametrize(