diff --git a/packages/service-library/src/servicelib/aiohttp/requests_validation.py b/packages/service-library/src/servicelib/aiohttp/requests_validation.py index bcae45a54e21..08892a46c744 100644 --- a/packages/service-library/src/servicelib/aiohttp/requests_validation.py +++ b/packages/service-library/src/servicelib/aiohttp/requests_validation.py @@ -67,7 +67,7 @@ def handle_validation_as_http_error( } for e in details ] - error_str = json_dumps( + error_json_str = json_dumps( { "error": { "status": status.HTTP_422_UNPROCESSABLE_ENTITY, @@ -77,7 +77,7 @@ def handle_validation_as_http_error( ) else: # NEW proposed error for https://github.com/ITISFoundation/osparc-simcore/issues/443 - error_str = json_dumps( + error_json_str = json_dumps( { "error": { "msg": user_error_message, @@ -88,7 +88,7 @@ def handle_validation_as_http_error( ) raise web.HTTPUnprocessableEntity( # 422 - text=error_str, + text=error_json_str, content_type=MIMETYPE_APPLICATION_JSON, ) from err diff --git a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py index ddf10e2fcf8e..393c0500694c 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py @@ -21,7 +21,7 @@ from ..logging_errors import create_troubleshootting_log_kwargs from ..mimetype_constants import MIMETYPE_APPLICATION_JSON -from ..rest_responses import is_enveloped_from_map, is_enveloped_from_text +from ..rest_responses import is_enveloped_from_text from ..status_codes_utils import get_code_description from . import status from .rest_responses import ( @@ -167,11 +167,10 @@ def _handle_http_successful( exception.status, safe_status_message(message=exception.reason) ) - if exception.text: - payload = json_loads(exception.text) - if not is_enveloped_from_map(payload): - payload = wrap_as_envelope(data=payload) - exception.text = json_dumps(payload) + if exception.text and not is_enveloped_from_text(exception.text): + # Ensures that the response is enveloped + data = json_loads(exception.text) + exception.text = json_dumps({"data": data}) return exception diff --git a/packages/service-library/src/servicelib/aiohttp/rest_responses.py b/packages/service-library/src/servicelib/aiohttp/rest_responses.py index 821c4e42e74c..7e3214dac2f2 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_responses.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_responses.py @@ -133,13 +133,19 @@ def create_http_error( ) -def exception_to_response(exc: HTTPError) -> web.Response: +def exception_to_response(exception: HTTPError) -> web.Response: # Returning web.HTTPException is deprecated so here we have a converter to a response # so it can be used as # SEE https://github.com/aio-libs/aiohttp/issues/2415 + + if exception.reason: + reason = safe_status_message(exception.reason) + else: + reason = get_code_description(exception.status) + return web.Response( - status=exc.status, - headers=exc.headers, - reason=exc.reason, - text=exc.text, + status=exception.status, + headers=exception.headers, + reason=reason, + text=exception.text, ) diff --git a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py index 50afdfedb8e7..81ed78a23540 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py +++ b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py @@ -44,7 +44,7 @@ async def get_task_result( except Exception as exc: _logger.exception( **create_troubleshootting_log_kwargs( - user_error_msg=f"{task_id=} raised an exception", + user_error_msg=f"{task_id=} raised an exception while getting its result", error=exc, error_code=create_error_code(exc), error_context={"task_context": task_context, "task_id": task_id}, diff --git a/packages/service-library/tests/aiohttp/test_rest_middlewares.py b/packages/service-library/tests/aiohttp/test_rest_middlewares.py index 21407d099eda..d87415858083 100644 --- a/packages/service-library/tests/aiohttp/test_rest_middlewares.py +++ b/packages/service-library/tests/aiohttp/test_rest_middlewares.py @@ -128,6 +128,10 @@ async def raise_success_with_text(_request: web.Request): # NOTE: explicitly NOT enveloped! raise web.HTTPOk(reason="I'm ok", text=json.dumps({"ok": True})) + @staticmethod + async def raise_success_with_raw_text(_request: web.Request): + raise web.HTTPOk(text="I'm ok") # NOT ALLOWED! + @pytest.fixture async def client( @@ -158,6 +162,10 @@ async def client( ("/v1/raise_success", Handlers.raise_success), ("/v1/raise_success_with_reason", Handlers.raise_success_with_reason), ("/v1/raise_success_with_text", Handlers.raise_success_with_text), + ( + "/v1/raise_success_with_raw_text", + Handlers.raise_success_with_raw_text, + ), ] ] ) @@ -330,6 +338,7 @@ async def test_http_ok_with_text_is_enveloped(client: TestClient): """Test that HTTPOk with text is properly enveloped.""" response = await client.get("/v1/raise_success_with_text") assert response.status == status.HTTP_200_OK + assert response.reason == "I'm ok" # Should be enveloped payload = await response.json() @@ -342,6 +351,11 @@ async def test_http_ok_with_text_is_enveloped(client: TestClient): assert data.get("ok") is True +async def test_http_ok_with_raw_text_is_not_allowed(client: TestClient): + response = await client.get("/v1/raise_success_with_raw_text") + assert response.status == status.HTTP_500_INTERNAL_SERVER_ERROR + + async def test_exception_in_handler_returns_500( client: TestClient, mocker: MockerFixture ): diff --git a/services/web/server/src/simcore_service_webserver/exception_handling/_factory.py b/services/web/server/src/simcore_service_webserver/exception_handling/_factory.py index ffa74dc59175..df675b81dbf2 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling/_factory.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling/_factory.py @@ -8,7 +8,11 @@ from servicelib.aiohttp.rest_responses import safe_status_message from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions from servicelib.logging_errors import create_troubleshootting_log_kwargs -from servicelib.status_codes_utils import is_5xx_server_error, is_error +from servicelib.status_codes_utils import ( + get_code_display_name, + is_5xx_server_error, + is_error, +) from ._base import AiohttpExceptionHandler, ExceptionHandlersMap @@ -50,7 +54,7 @@ def create_error_response(error: ErrorGet, status_code: int) -> web.Response: return web.json_response( data={"error": error.model_dump(exclude_unset=True, mode="json")}, dumps=json_dumps, - reason=safe_status_message(error.message), + reason=safe_status_message(get_code_display_name(status_code)), status=status_code, ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index 861d635a8bbd..8ab398bcdaca 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -263,7 +263,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche simcore_user_agent: str, parent_project_uuid: ProjectID | None, parent_node_id: NodeID | None, -) -> None: +) -> web.HTTPCreated: """Implements TaskProtocol for 'create_projects' handler Arguments: @@ -278,7 +278,6 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche predefined_project -- project in request body Raises: - web.HTTPCreated: succeeded web.HTTPBadRequest: web.HTTPNotFound: web.HTTPUnauthorized: @@ -481,7 +480,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche **RESPONSE_MODEL_POLICY ) - raise web.HTTPCreated( + return web.HTTPCreated( text=json_dumps({"data": data}), content_type=MIMETYPE_APPLICATION_JSON, ) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling_factory.py b/services/web/server/tests/unit/isolated/test_exception_handling_factory.py index e87ef0b53c37..022eb12c62ae 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling_factory.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling_factory.py @@ -13,6 +13,7 @@ from aiohttp.test_utils import make_mocked_request from servicelib.aiohttp import status from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON +from servicelib.status_codes_utils import get_code_display_name from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exception_handling._base import ( ExceptionHandlingContextManager, @@ -28,16 +29,13 @@ # Some custom errors in my service -class BaseError(WebServerBaseError): - ... +class BaseError(WebServerBaseError): ... -class OneError(BaseError): - ... +class OneError(BaseError): ... -class OtherError(BaseError): - ... +class OtherError(BaseError): ... @pytest.fixture @@ -58,7 +56,7 @@ async def test_factory__create_exception_handler_from_http_error( response = await one_error_to_404(fake_request, caught) assert response.status == status.HTTP_404_NOT_FOUND assert response.text is not None - assert "one error message" in response.reason + assert response.reason == get_code_display_name(response.status) assert response.content_type == MIMETYPE_APPLICATION_JSON @@ -82,9 +80,7 @@ async def test_handling_different_exceptions_with_context( response = cm.get_response_or_none() assert response is not None assert response.status == status.HTTP_400_BAD_REQUEST - assert response.reason == exc_to_http_error_map[OneError].msg_template.format( - code="WebServerBaseError.BaseError.OneError" - ) + assert response.reason == get_code_display_name(response.status) assert not caplog.records # unhandled -> reraises @@ -103,9 +99,7 @@ async def test_handling_different_exceptions_with_context( response = cm.get_response_or_none() assert response is not None assert response.status == status.HTTP_500_INTERNAL_SERVER_ERROR - assert response.reason == exc_to_http_error_map[OtherError].msg_template.format( - code="WebServerBaseError.BaseError.OtherError" - ) + assert response.reason == get_code_display_name(response.status) assert caplog.records, "Expected 5XX troubleshooting logged as error" assert caplog.records[0].levelno == logging.ERROR