🐛 Fix: assert request in successful HTTP response handlers and update type hint for create_http_error

Author: pcrespov

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

@@ -51,7 +51,10 @@ def _handle_unexpected_exception_as_500(
     *,
     skip_internal_error_details: bool,
 ) -> web.HTTPInternalServerError:
-    """Process unexpected exceptions and return them as HTTP errors with proper formatting."""
+    """Process unexpected exceptions and return them as HTTP errors with proper formatting.
+
+    IMPORTANT: this function cannot throw exceptions, as it is called
+    """
     error_code = create_error_code(exception)
     error_context: dict[str, Any] = {
         "request.remote": f"{request.remote}",
@@ -60,13 +63,17 @@ def _handle_unexpected_exception_as_500(
     }
 
     user_error_msg = _FMSG_INTERNAL_ERROR_USER_FRIENDLY
+
     http_error = create_http_error(
         exception,
         user_error_msg,
         web.HTTPInternalServerError,
         skip_internal_error_details=skip_internal_error_details,
         error_code=error_code,
     )
+
+    error_context["http_error"] = http_error
+
     _logger.exception(
         **create_troubleshotting_log_kwargs(
             user_error_msg,
@@ -177,7 +184,6 @@ async def _middleware_handler(request: web.Request, handler: Handler):
                 result = _handle_http_successful(request, exc)
 
             except web.HTTPRedirection as exc:  # 3XX raised as exceptions
-                _logger.debug("Redirected to %s", exc)
                 result = exc
 
             except NotImplementedError as exc:

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

@@ -14,6 +14,7 @@
 from aiohttp import web
 from aiohttp.test_utils import TestClient
 from common_library.json_serialization import json_dumps
+from pytest_mock import MockerFixture
 from servicelib.aiohttp import status
 from servicelib.aiohttp.rest_middlewares import (
     envelope_middleware_factory,
@@ -339,3 +340,41 @@ async def test_http_ok_with_text_is_enveloped(client: TestClient):
     assert not error
     assert data
     assert data.get("ok") is True
+
+
+async def test_exception_in_handler_returns_500(
+    client: TestClient, mocker: MockerFixture
+):
+    """Test that exceptions in the handler functions are caught and return 500."""
+
+    # Mock _handle_http_successful to raise an exception
+    def mocked_handler(*args, **kwargs):
+        msg = "Simulated error in handler"
+        raise ValueError(msg)
+
+    mocker.patch(
+        "servicelib.aiohttp.rest_middlewares._handle_http_successful",
+        side_effect=mocked_handler,
+    )
+
+    # Trigger a successful HTTP response that will be processed by our mocked handler
+    response = await client.get(
+        "/v1/raise_exception", params={"exc": web.HTTPOk.__name__}
+    )
+
+    # Should return 500 since our handler raised an exception
+    assert response.status == status.HTTP_500_INTERNAL_SERVER_ERROR
+
+    # Check that the response is properly enveloped
+    payload = await response.json()
+    assert is_enveloped(payload)
+
+    # Verify error details
+    data, error = unwrap_envelope(payload)
+    assert not data
+    assert error
+    assert error.get("status") == status.HTTP_500_INTERNAL_SERVER_ERROR
+
+    # Make sure there are no detailed error logs in production mode
+    assert not error.get("errors")
+    assert not error.get("logs")