From e3c6aacb581d4b58c1349a73b6fc596087bf5934 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 24 Mar 2025 13:46:29 +0100 Subject: [PATCH 1/3] adds missing supportID --- .../src/servicelib/aiohttp/rest_responses.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_responses.py b/packages/service-library/src/servicelib/aiohttp/rest_responses.py index 312e9c987db0..9b30e2e0f098 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_responses.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_responses.py @@ -1,11 +1,13 @@ """Utils to check, convert and compose server responses for the RESTApi""" import inspect -from typing import Any +from typing import Any, cast from aiohttp import web, web_exceptions from aiohttp.web_exceptions import HTTPError, HTTPException +from common_library.error_codes import ErrorCodeStr from common_library.json_serialization import json_dumps +from models_library.basic_types import IDStr from models_library.rest_error import ErrorGet, ErrorItemType from ..aiohttp.status import HTTP_200_OK @@ -52,6 +54,7 @@ def create_http_error( http_error_cls: type[HTTPError] = web.HTTPInternalServerError, *, skip_internal_error_details: bool = False, + error_code: ErrorCodeStr | None = None ) -> HTTPError: """ - Response body conforms OAS schema model @@ -61,25 +64,24 @@ def create_http_error( if not isinstance(errors, list): errors = [errors] - # TODO: guarantee no throw! + support_id: IDStr | None = cast(IDStr, error_code) if error_code else None is_internal_error: bool = http_error_cls == web.HTTPInternalServerError default_message = reason or get_code_description(http_error_cls.status_code) if is_internal_error and skip_internal_error_details: error = ErrorGet( - errors=[], - logs=[], status=http_error_cls.status_code, message=default_message, + support_id=support_id, ) else: items = [ErrorItemType.from_error(err) for err in errors] error = ErrorGet( - errors=items, - logs=[], + errors=items, # NOTE: deprecated! status=http_error_cls.status_code, message=default_message, + support_id=support_id, ) assert not http_error_cls.empty_body # nosec From b086afc4fa29f78b2d95c4f8601430c9f9700057 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 24 Mar 2025 14:00:48 +0100 Subject: [PATCH 2/3] adds model policy --- .../servicelib/aiohttp/rest_middlewares.py | 5 ++- .../src/servicelib/aiohttp/rest_responses.py | 9 ++++-- .../tests/aiohttp/test_rest_responses.py | 32 ++++++++++++++----- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py index 4847389b7195..d40abae5669d 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py @@ -51,14 +51,13 @@ def _process_and_raise_unexpected_error(request: web.BaseRequest, err: Exception "request.path": f"{request.path}", } - user_error_msg = _FMSG_INTERNAL_ERROR_USER_FRIENDLY.format( - error_code=error_code - ) + user_error_msg = _FMSG_INTERNAL_ERROR_USER_FRIENDLY http_error = create_http_error( err, user_error_msg, web.HTTPInternalServerError, skip_internal_error_details=_is_prod, + error_code=error_code, ) _logger.exception( **create_troubleshotting_log_kwargs( diff --git a/packages/service-library/src/servicelib/aiohttp/rest_responses.py b/packages/service-library/src/servicelib/aiohttp/rest_responses.py index 9b30e2e0f098..c8a3376d88f7 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_responses.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_responses.py @@ -9,6 +9,7 @@ from common_library.json_serialization import json_dumps from models_library.basic_types import IDStr from models_library.rest_error import ErrorGet, ErrorItemType +from servicelib.rest_constants import RESPONSE_MODEL_POLICY from ..aiohttp.status import HTTP_200_OK from ..mimetype_constants import MIMETYPE_APPLICATION_JSON @@ -85,11 +86,15 @@ def create_http_error( ) assert not http_error_cls.empty_body # nosec - payload = wrap_as_envelope(error=error) + payload = wrap_as_envelope( + error=error.model_dump(mode="json", **RESPONSE_MODEL_POLICY) + ) return http_error_cls( reason=reason, - text=json_dumps(payload), + text=json_dumps( + payload, + ), content_type=MIMETYPE_APPLICATION_JSON, ) diff --git a/packages/service-library/tests/aiohttp/test_rest_responses.py b/packages/service-library/tests/aiohttp/test_rest_responses.py index d1b28d1e9fe2..9865c805bfd8 100644 --- a/packages/service-library/tests/aiohttp/test_rest_responses.py +++ b/packages/service-library/tests/aiohttp/test_rest_responses.py @@ -3,6 +3,7 @@ # pylint: disable=unused-variable import itertools +import json import pytest from aiohttp import web @@ -15,6 +16,7 @@ HTTPNotModified, 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 ( @@ -58,26 +60,40 @@ def test_collected_http_errors_map(status_code: int, http_error_cls: type[HTTPEr @pytest.mark.parametrize("skip_details", [True, False]) -def tests_exception_to_response(skip_details: bool): - exception = create_http_error( - errors=[RuntimeError("foo")], - reason="Something whent wrong", +@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_exceptions: list[Exception] = [RuntimeError("foo")] + + http_error = create_http_error( + errors=expected_exceptions, + reason=expected_reason, http_error_cls=web.HTTPInternalServerError, skip_internal_error_details=skip_details, + error_code=error_code, ) # For now until deprecated SEE https://github.com/aio-libs/aiohttp/issues/2415 - assert isinstance(exception, Exception) - assert isinstance(exception, web.Response) - assert hasattr(exception, "__http_exception__") + assert isinstance(http_error, Exception) + assert isinstance(http_error, web.Response) + assert hasattr(http_error, "__http_exception__") # until they have exception.make_response(), we user - response = exception_to_response(exception) + response = exception_to_response(http_error) assert isinstance(response, web.Response) assert not isinstance(response, Exception) assert not hasattr(response, "__http_exception__") + # checks response components assert response.content_type == MIMETYPE_APPLICATION_JSON assert response.status == status.HTTP_500_INTERNAL_SERVER_ERROR assert response.text assert response.body + + # 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"]["supportId"] == error_code + assert response_json["error"]["status"] == response.status From 97d332e2131497c5639377c798cf85b199eb4fab Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 24 Mar 2025 14:08:33 +0100 Subject: [PATCH 3/3] cleanup --- .../src/servicelib/aiohttp/rest_responses.py | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_responses.py b/packages/service-library/src/servicelib/aiohttp/rest_responses.py index c8a3376d88f7..8ddf5090b5a9 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_responses.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_responses.py @@ -1,13 +1,12 @@ """Utils to check, convert and compose server responses for the RESTApi""" import inspect -from typing import Any, cast +from typing import Any from aiohttp import web, web_exceptions from aiohttp.web_exceptions import HTTPError, HTTPException from common_library.error_codes import ErrorCodeStr from common_library.json_serialization import json_dumps -from models_library.basic_types import IDStr from models_library.rest_error import ErrorGet, ErrorItemType from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -65,24 +64,26 @@ def create_http_error( if not isinstance(errors, list): errors = [errors] - support_id: IDStr | None = cast(IDStr, error_code) if error_code else None - is_internal_error: bool = http_error_cls == web.HTTPInternalServerError default_message = reason or get_code_description(http_error_cls.status_code) if is_internal_error and skip_internal_error_details: - error = ErrorGet( - status=http_error_cls.status_code, - message=default_message, - support_id=support_id, + error = ErrorGet.model_validate( + { + "status": http_error_cls.status_code, + "message": default_message, + "support_id": error_code, + } ) else: items = [ErrorItemType.from_error(err) for err in errors] - error = ErrorGet( - errors=items, # NOTE: deprecated! - status=http_error_cls.status_code, - message=default_message, - support_id=support_id, + error = ErrorGet.model_validate( + { + "errors": items, # NOTE: deprecated! + "status": http_error_cls.status_code, + "message": default_message, + "support_id": error_code, + } ) assert not http_error_cls.empty_body # nosec