From 4a00ba15fb0190b96f4ace7ed1173bcb2bc6bd15 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 25 Jun 2024 19:21:43 +0200 Subject: [PATCH] adapts http error logic --- .../servicelib/aiohttp/rest_middlewares.py | 16 +++---- .../src/servicelib/aiohttp/rest_responses.py | 44 ++++++++++++------- .../director_v2/_handlers.py | 30 ++++++++----- 3 files changed, 57 insertions(+), 33 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py index 8c12f7b34918..e68db2cbb46b 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py @@ -18,7 +18,7 @@ from .rest_models import ErrorItemType, ErrorType, LogMessageType from .rest_responses import ( create_data_response, - create_error_response, + create_http_error, is_enveloped_from_map, is_enveloped_from_text, wrap_as_envelope, @@ -44,7 +44,7 @@ def error_middleware_factory( _is_prod: bool = is_production_environ() def _process_and_raise_unexpected_error(request: web.BaseRequest, err: Exception): - resp = create_error_response( + http_error = create_http_error( err, "Unexpected Server error", web.HTTPInternalServerError, @@ -58,11 +58,11 @@ def _process_and_raise_unexpected_error(request: web.BaseRequest, err: Exception request.remote, request.method, request.path, - resp.status, + http_error.status, exc_info=err, stack_info=True, ) - raise resp + raise http_error @web.middleware async def _middleware_handler(request: web.Request, handler: Handler): @@ -115,22 +115,22 @@ async def _middleware_handler(request: web.Request, handler: Handler): raise except NotImplementedError as err: - error_response = create_error_response( + http_error = create_http_error( err, f"{err}", web.HTTPNotImplemented, skip_internal_error_details=_is_prod, ) - raise error_response from err + raise http_error from err except asyncio.TimeoutError as err: - error_response = create_error_response( + http_error = create_http_error( err, f"{err}", web.HTTPGatewayTimeout, skip_internal_error_details=_is_prod, ) - raise error_response from err + raise http_error from err except Exception as err: # pylint: disable=broad-except _process_and_raise_unexpected_error(request, err) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_responses.py b/packages/service-library/src/servicelib/aiohttp/rest_responses.py index 71f5a24d8220..100146fe92d5 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_responses.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_responses.py @@ -12,6 +12,7 @@ from models_library.utils.json_serialization import json_dumps from servicelib.aiohttp.status import HTTP_200_OK +from ..mimetype_constants import MIMETYPE_APPLICATION_JSON from .rest_models import ErrorItemType, ErrorType _ENVELOPE_KEYS = ("data", "error") @@ -64,24 +65,26 @@ def create_data_response( response = web.json_response(payload, dumps=json_dumps, status=status) except (TypeError, ValueError) as err: - response = create_error_response( - [ - err, - ], - str(err), - web.HTTPInternalServerError, - skip_internal_error_details=skip_internal_error_details, + response = exception_to_response( + create_http_error( + [ + err, + ], + str(err), + web.HTTPInternalServerError, + skip_internal_error_details=skip_internal_error_details, + ) ) return response -def create_error_response( +def create_http_error( errors: list[Exception] | Exception, reason: str | None = None, http_error_cls: type[HTTPError] = web.HTTPInternalServerError, *, skip_internal_error_details: bool = False, -) -> web.Response: +) -> HTTPError: """ - Response body conforms OAS schema model - Can skip internal details when 500 status e.g. to avoid transmitting server @@ -108,13 +111,24 @@ def create_error_response( assert not http_error_cls.empty_body # nosec payload = wrap_as_envelope(error=asdict(error)) - # Returning web.HTTPException is deprecated, returning instead a response object - # SEE https://github.com/aio-libs/aiohttp/issues/2415 - return web.json_response( - payload, + return http_error_cls( reason=reason, - status=http_error_cls.status_code, - dumps=json_dumps, + text=json_dumps(payload), + content_type=MIMETYPE_APPLICATION_JSON, + ) + + +def exception_to_response(exc: 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 + return web.Response( + status=exc.status, + headers=exc.headers, + reason=exc.reason, + body=exc.body, + text=exc.text, + content_type=exc.content_type, ) diff --git a/services/web/server/src/simcore_service_webserver/director_v2/_handlers.py b/services/web/server/src/simcore_service_webserver/director_v2/_handlers.py index 6886c2408c2b..ccd523fc7505 100644 --- a/services/web/server/src/simcore_service_webserver/director_v2/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/director_v2/_handlers.py @@ -10,7 +10,11 @@ from models_library.utils.json_serialization import json_dumps from pydantic import BaseModel, Field, ValidationError, parse_obj_as from pydantic.types import NonNegativeInt -from servicelib.aiohttp.rest_responses import create_error_response, get_http_error +from servicelib.aiohttp.rest_responses import ( + create_http_error, + exception_to_response, + get_http_error, +) from servicelib.common_headers import ( UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE, X_SIMCORE_USER_AGENT, @@ -162,15 +166,21 @@ async def start_computation(request: web.Request) -> web.Response: return envelope_json_response(data, status_cls=web.HTTPCreated) except DirectorServiceError as exc: - return create_error_response( - exc, - reason=exc.reason, - http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable, + return exception_to_response( + create_http_error( + exc, + reason=exc.reason, + http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable, + ) ) except UserDefaultWalletNotFoundError as exc: - return create_error_response(exc, http_error_cls=web.HTTPNotFound) + return exception_to_response( + create_http_error(exc, http_error_cls=web.HTTPNotFound) + ) except WalletNotEnoughCreditsError as exc: - return create_error_response(exc, http_error_cls=web.HTTPPaymentRequired) + return exception_to_response( + create_http_error(exc, http_error_cls=web.HTTPPaymentRequired) + ) @routes.post(f"/{VTAG}/computations/{{project_id}}:stop", name="stop_computation") @@ -203,7 +213,7 @@ async def stop_computation(request: web.Request) -> web.Response: raise web.HTTPNoContent(content_type=MIMETYPE_APPLICATION_JSON) except DirectorServiceError as exc: - return create_error_response( + return create_http_error( exc, reason=exc.reason, http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable, @@ -252,10 +262,10 @@ async def get_computation(request: web.Request) -> web.Response: dumps=json_dumps, ) except DirectorServiceError as exc: - return create_error_response( + return create_http_error( exc, reason=exc.reason, http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable, ) except ValidationError as exc: - return create_error_response(exc, http_error_cls=web.HTTPInternalServerError) + return create_http_error(exc, http_error_cls=web.HTTPInternalServerError)