From 131fb7b457300e51973735ec79f8e7d2b17db3df Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:23:40 +0100 Subject: [PATCH 01/59] new model --- .../src/models_library/rest_error.py | 103 ++++++++++++++++++ .../pytest_simcore/helpers/assert_checks.py | 10 +- 2 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 packages/models-library/src/models_library/rest_error.py diff --git a/packages/models-library/src/models_library/rest_error.py b/packages/models-library/src/models_library/rest_error.py new file mode 100644 index 000000000000..26f56cf18917 --- /dev/null +++ b/packages/models-library/src/models_library/rest_error.py @@ -0,0 +1,103 @@ +from dataclasses import dataclass +from typing import Annotated + +from pydantic import BaseModel, ConfigDict, Field + +from .basic_types import IDStr, LogLevel + + +class Log(BaseModel): + level: LogLevel | None = Field("INFO", description="log level") + message: str = Field( + ..., + description="log message. If logger is USER, then it MUST be human readable", + ) + logger: str | None = Field( + None, description="name of the logger receiving this message" + ) + + model_config = ConfigDict( + json_schema_extra={ + "example": { + "message": "Hi there, Mr user", + "level": "INFO", + "logger": "user-logger", + } + } + ) + + +class ErrorItem(BaseModel): + code: str = Field( + ..., + description="Typically the name of the exception that produced it otherwise some known error code", + ) + message: str = Field(..., description="Error message specific to this item") + resource: str | None = Field( + None, description="API resource affected by this error" + ) + field: str | None = Field(None, description="Specific field within the resource") + + +@dataclass +class LogMessageType: + # NOTE: deprecated! + message: str + level: str = "INFO" + logger: str = "user" + + +@dataclass +class ErrorItemType: + # NOTE: deprecated! + code: str + message: str + resource: str | None + field: str | None + + @classmethod + def from_error(cls, err: BaseException): + return cls( + code=err.__class__.__name__, message=str(err), resource=None, field=None + ) + + +class ErrorGet(BaseModel): + message: Annotated[ + str, + Field( + min_length=5, + description="Message displayed to the user", + ), + ] + support_id: Annotated[ + IDStr | None, + Field(description="ID to track the incident during support", alias="supportId"), + ] = None + + # NOTE: The fields blow are DEPRECATED. + # Still here to keep compatibilty with front-end until updated + status: Annotated[int, Field(deprecated=True)] = 400 + errors: Annotated[ + list[ErrorItemType], + Field(deprecated=True, default_factory=list, json_schema_extra={"default": []}), + ] + logs: Annotated[ + list[LogMessageType], + Field(deprecated=True, default_factory=list, json_schema_extra={"default": []}), + ] + + model_config = ConfigDict( + populate_by_name=True, + extra="ignore", # Used to prune extra fields from internal data + frozen=True, + json_schema_extra={ + "examples": [ + {"msg": "Sorry you do not have sufficient access rights for product"}, + { + "msg": "Opps this error was unexpected. We are working on that!", + "supportId": "OEC:12346789", + }, + ] + }, + ) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py b/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py index 3ce30f4131ac..ba92081b9e44 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/assert_checks.py @@ -82,13 +82,17 @@ def _do_assert_error( assert is_error(expected_status_code) - assert len(error["errors"]) >= 1 + # New versions of the error models might not have this attribute + details = error.get("errors", []) + if expected_msg: - messages = [detail["message"] for detail in error["errors"]] + assert details + messages = [e["message"] for e in details] assert expected_msg in messages if expected_error_code: - codes = [detail["code"] for detail in error["errors"]] + assert details + codes = [e["code"] for e in details] assert expected_error_code in codes return data, error From 835b190f5e88cbfef592ab8b16c50bdcb5fe31c3 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:24:46 +0100 Subject: [PATCH 02/59] using new model --- .../servicelib/aiohttp/rest_middlewares.py | 4 +-- .../src/servicelib/aiohttp/rest_models.py | 30 ------------------- .../src/servicelib/aiohttp/rest_responses.py | 11 +++---- .../src/servicelib/aiohttp/rest_utils.py | 8 +---- 4 files changed, 9 insertions(+), 44 deletions(-) delete mode 100644 packages/service-library/src/servicelib/aiohttp/rest_models.py diff --git a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py index d5f5a04283b4..e1d1848406e0 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py @@ -14,11 +14,11 @@ from aiohttp.web_response import StreamResponse from common_library.error_codes import create_error_code from common_library.json_serialization import json_dumps +from models_library.rest_error import ErrorGet, ErrorItemType, LogMessageType from ..logging_errors import create_troubleshotting_log_kwargs from ..mimetype_constants import MIMETYPE_APPLICATION_JSON from ..utils import is_production_environ -from .rest_models import ErrorItemType, ErrorType, LogMessageType from .rest_responses import ( create_data_response, create_http_error, @@ -98,7 +98,7 @@ async def _middleware_handler(request: web.Request, handler: Handler): err.content_type = MIMETYPE_APPLICATION_JSON if not err.text or not is_enveloped_from_text(err.text): - error = ErrorType( + error = ErrorGet( errors=[ ErrorItemType.from_error(err), ], diff --git a/packages/service-library/src/servicelib/aiohttp/rest_models.py b/packages/service-library/src/servicelib/aiohttp/rest_models.py deleted file mode 100644 index 36902f17b779..000000000000 --- a/packages/service-library/src/servicelib/aiohttp/rest_models.py +++ /dev/null @@ -1,30 +0,0 @@ -from dataclasses import dataclass, field - - -@dataclass -class LogMessageType: - message: str - level: str = "INFO" - logger: str = "user" - - -@dataclass -class ErrorItemType: - code: str - message: str - resource: str | None - field: str | None - - @classmethod - def from_error(cls, err: BaseException): - return cls( - code=err.__class__.__name__, message=str(err), resource=None, field=None - ) - - -@dataclass -class ErrorType: - logs: list[LogMessageType] = field(default_factory=list) - errors: list[ErrorItemType] = field(default_factory=list) - status: int = 400 - message: str = "Unexpected error" diff --git a/packages/service-library/src/servicelib/aiohttp/rest_responses.py b/packages/service-library/src/servicelib/aiohttp/rest_responses.py index 1cce04ae0152..d16f33b9e57b 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_responses.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_responses.py @@ -5,17 +5,16 @@ import inspect import json from collections.abc import Mapping -from dataclasses import asdict from typing import Any from aiohttp import web, web_exceptions from aiohttp.web_exceptions import HTTPError, HTTPException from common_library.json_serialization import json_dumps +from models_library.rest_error import ErrorGet, ErrorItemType from servicelib.aiohttp.status import HTTP_200_OK from ..mimetype_constants import MIMETYPE_APPLICATION_JSON from ..status_codes_utils import get_code_description -from .rest_models import ErrorItemType, ErrorType _ENVELOPE_KEYS = ("data", "error") @@ -101,21 +100,23 @@ def create_http_error( default_message = reason or get_code_description(http_error_cls.status_code) if is_internal_error and skip_internal_error_details: - error = ErrorType( + error = ErrorGet( errors=[], + logs=[], status=http_error_cls.status_code, message=default_message, ) else: items = [ErrorItemType.from_error(err) for err in errors] - error = ErrorType( + error = ErrorGet( errors=items, + logs=[], status=http_error_cls.status_code, message=default_message, ) assert not http_error_cls.empty_body # nosec - payload = wrap_as_envelope(error=asdict(error)) + payload = wrap_as_envelope(error=error) return http_error_cls( reason=reason, diff --git a/packages/service-library/src/servicelib/aiohttp/rest_utils.py b/packages/service-library/src/servicelib/aiohttp/rest_utils.py index e026e2119773..78fa58ac8ac3 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_utils.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_utils.py @@ -1,5 +1,3 @@ -from dataclasses import asdict - from aiohttp import web from aiohttp.web import RouteDef, RouteTableDef from common_library.json_serialization import json_dumps @@ -13,11 +11,7 @@ class EnvelopeFactory: """ def __init__(self, data=None, error=None): - enveloped = {"data": data, "error": error} - for key, value in enveloped.items(): - if value is not None and not isinstance(value, dict): - enveloped[key] = asdict(value) - self._envelope = enveloped + self._envelope = {"data": data, "error": error} def as_dict(self) -> dict: return self._envelope From 4bc45e8428bb0b1187aee53920e29ed0c8d6e0a6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:25:03 +0100 Subject: [PATCH 03/59] logging --- .../src/servicelib/logging_utils.py | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/service-library/src/servicelib/logging_utils.py b/packages/service-library/src/servicelib/logging_utils.py index 9ffc6b32c5b4..b6b67f8b1634 100644 --- a/packages/service-library/src/servicelib/logging_utils.py +++ b/packages/service-library/src/servicelib/logging_utils.py @@ -79,12 +79,24 @@ def format(self, record) -> str: return super().format(record).replace("\n", "\\n") +class LogExtra(TypedDict): + log_uid: NotRequired[str] + log_oec: NotRequired[str] + + # SEE https://docs.python.org/3/library/logging.html#logrecord-attributes -DEFAULT_FORMATTING = "log_level=%(levelname)s | log_timestamp=%(asctime)s | log_source=%(name)s:%(funcName)s(%(lineno)d) | log_uid=%(log_uid)s | log_msg=%(message)s" +DEFAULT_FORMATTING = ( + "log_level=%(levelname)s " + "| log_timestamp=%(asctime)s " + "| log_source=%(name)s:%(funcName)s(%(lineno)d) " + "| log_uid=%(log_uid)s " + "| log_oec=%(log_oec)s" + "| log_msg=%(message)s" +) LOCAL_FORMATTING = "%(levelname)s: [%(asctime)s/%(processName)s] [%(name)s:%(funcName)s(%(lineno)d)] - %(message)s" # Graylog Grok pattern extractor: -# log_level=%{WORD:log_level} \| log_timestamp=%{TIMESTAMP_ISO8601:log_timestamp} \| log_source=%{DATA:log_source} \| log_msg=%{GREEDYDATA:log_msg} +# log_level=%{WORD:log_level} \| log_timestamp=%{TIMESTAMP_ISO8601:log_timestamp} \| log_source=%{DATA:log_source} \| (log_uid=%{WORD:log_uid} \| )?log_msg=%{GREEDYDATA:log_msg} def config_all_loggers( @@ -336,11 +348,6 @@ def log_catch(logger: logging.Logger, *, reraise: bool = True) -> Iterator[None] raise exc from exc -class LogExtra(TypedDict): - log_uid: NotRequired[str] - log_oec: NotRequired[str] - - LogLevelInt: TypeAlias = int LogMessageStr: TypeAlias = str From 1a5dee9fa615804333d2b3d15d3a56ff029daf19 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:25:22 +0100 Subject: [PATCH 04/59] minor --- services/web/server/tests/unit/with_dbs/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index c943be8c76cd..eb24a2d1174c 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -15,11 +15,11 @@ import random import sys import textwrap -from collections.abc import AsyncIterator, Awaitable, Callable, Iterator +from collections.abc import AsyncIterable, AsyncIterator, Awaitable, Callable, Iterator from copy import deepcopy from decimal import Decimal from pathlib import Path -from typing import Any, AsyncIterable, Final +from typing import Any, Final from unittest import mock from unittest.mock import AsyncMock, MagicMock @@ -853,7 +853,7 @@ async def all_product_prices( result = {} for product_name in all_products_names: - usd_or_none = product_price.get(product_name, None) + usd_or_none = product_price.get(product_name) if usd_or_none is not None: await _pre_connection.execute( products_prices.insert().values( From a8aaea0bd503fef81d541abb90974af6cf38c399 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:26:07 +0100 Subject: [PATCH 05/59] oas --- api/specs/web-server/_auth.py | 27 +++++----- api/specs/web-server/_common.py | 54 ++++++------------- api/specs/web-server/_folders.py | 8 ++- api/specs/web-server/_workspaces.py | 8 ++- .../simcore_service_webserver/login/utils.py | 2 +- 5 files changed, 46 insertions(+), 53 deletions(-) diff --git a/api/specs/web-server/_auth.py b/api/specs/web-server/_auth.py index 4c323869e04e..ecb8c802b968 100644 --- a/api/specs/web-server/_auth.py +++ b/api/specs/web-server/_auth.py @@ -6,7 +6,7 @@ from typing import Any -from _common import Error, Log +from _common import EnvelopeE from fastapi import APIRouter, status from models_library.api_schemas_webserver.auth import ( AccountRequestInfo, @@ -15,6 +15,7 @@ UnregisterCheck, ) from models_library.generics import Envelope +from models_library.rest_error import ErrorGet, Log from pydantic import BaseModel, Field, confloat from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.login._2fa_handlers import Resend2faBody @@ -75,7 +76,7 @@ async def register(_body: RegisterBody): "/auth/unregister", response_model=Envelope[Log], status_code=status.HTTP_200_OK, - responses={status.HTTP_409_CONFLICT: {"model": Envelope[Error]}}, + responses={status.HTTP_409_CONFLICT: {"model": EnvelopeE[ErrorGet]}}, ) async def unregister_account(_body: UnregisterCheck): ... @@ -107,7 +108,7 @@ async def phone_confirmation(_body: PhoneConfirmationBody): responses={ # status.HTTP_503_SERVICE_UNAVAILABLE status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized reset due to invalid token code", } }, @@ -122,7 +123,7 @@ async def login(_body: LoginBody): operation_id="auth_login_2fa", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized reset due to invalid token code", } }, @@ -137,7 +138,7 @@ async def login_2fa(_body: LoginTwoFactorAuthBody): operation_id="auth_resend_2fa_code", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized reset due to invalid token code", } }, @@ -161,7 +162,7 @@ async def logout(_body: LogoutBody): status_code=status.HTTP_204_NO_CONTENT, responses={ status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized reset due to invalid token code", } }, @@ -174,7 +175,7 @@ async def check_auth(): "/auth/reset-password", response_model=Envelope[Log], operation_id="auth_reset_password", - responses={status.HTTP_503_SERVICE_UNAVAILABLE: {"model": Envelope[Error]}}, + responses={status.HTTP_503_SERVICE_UNAVAILABLE: {"model": EnvelopeE[ErrorGet]}}, ) async def reset_password(_body: ResetPasswordBody): """a non logged-in user requests a password reset""" @@ -186,7 +187,7 @@ async def reset_password(_body: ResetPasswordBody): operation_id="auth_reset_password_allowed", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized reset due to invalid token code", } }, @@ -201,11 +202,11 @@ async def reset_password_allowed(code: str, _body: ResetPasswordConfirmation): operation_id="auth_change_email", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized user. Login required", }, status.HTTP_503_SERVICE_UNAVAILABLE: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unable to send confirmation email", }, }, @@ -233,15 +234,15 @@ class PasswordCheckSchema(BaseModel): operation_id="auth_change_password", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "unauthorized user. Login required", }, status.HTTP_409_CONFLICT: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "mismatch between new and confirmation passwords", }, status.HTTP_422_UNPROCESSABLE_ENTITY: { - "model": Envelope[Error], + "model": EnvelopeE[ErrorGet], "description": "current password is invalid", }, }, diff --git a/api/specs/web-server/_common.py b/api/specs/web-server/_common.py index 637f41018193..5afbea3d1d2b 100644 --- a/api/specs/web-server/_common.py +++ b/api/specs/web-server/_common.py @@ -5,13 +5,22 @@ import sys from collections.abc import Callable from pathlib import Path -from typing import Annotated, NamedTuple, Optional, Union, get_args, get_origin +from typing import ( + Annotated, + Any, + Generic, + NamedTuple, + Optional, + TypeVar, + Union, + get_args, + get_origin, +) from common_library.json_serialization import json_dumps from common_library.pydantic_fields_extension import get_type from fastapi import Query -from models_library.basic_types import LogLevel -from pydantic import BaseModel, ConfigDict, Field, Json, create_model +from pydantic import BaseModel, Json, create_model from pydantic.fields import FieldInfo CURRENT_DIR = Path(sys.argv[0] if __name__ == "__main__" else __file__).resolve().parent @@ -78,43 +87,14 @@ def as_query(model_class: type[BaseModel]) -> type[BaseModel]: return create_model(new_model_name, **fields) -class Log(BaseModel): - level: LogLevel | None = Field("INFO", description="log level") - message: str = Field( - ..., - description="log message. If logger is USER, then it MUST be human readable", - ) - logger: str | None = Field( - None, description="name of the logger receiving this message" - ) - - model_config = ConfigDict( - json_schema_extra={ - "example": { - "message": "Hi there, Mr user", - "level": "INFO", - "logger": "user-logger", - } - } - ) - +ErrorT = TypeVar("ErrorT") -class ErrorItem(BaseModel): - code: str = Field( - ..., - description="Typically the name of the exception that produced it otherwise some known error code", - ) - message: str = Field(..., description="Error message specific to this item") - resource: str | None = Field( - None, description="API resource affected by this error" - ) - field: str | None = Field(None, description="Specific field within the resource") +class EnvelopeE(BaseModel, Generic[ErrorT]): + """Complementary to models_library.generics.Envelope just for the generators""" -class Error(BaseModel): - logs: list[Log] | None = Field(None, description="log messages") - errors: list[ErrorItem] | None = Field(None, description="errors metadata") - status: int | None = Field(None, description="HTTP error code") + error: ErrorT | None = None + data: Any | None = None class ParamSpec(NamedTuple): diff --git a/api/specs/web-server/_folders.py b/api/specs/web-server/_folders.py index f98b5e98308f..1ca5b74e39c1 100644 --- a/api/specs/web-server/_folders.py +++ b/api/specs/web-server/_folders.py @@ -9,7 +9,7 @@ from typing import Annotated -from _common import as_query +from _common import EnvelopeE, as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.folders_v2 import ( FolderCreateBodyParams, @@ -17,7 +17,9 @@ FolderReplaceBodyParams, ) from models_library.generics import Envelope +from models_library.rest_error import ErrorGet from simcore_service_webserver._meta import API_VTAG +from simcore_service_webserver.folders._exceptions_handlers import _TO_HTTP_ERROR_MAP from simcore_service_webserver.folders._models import ( FolderSearchQueryParams, FoldersListQueryParams, @@ -29,6 +31,10 @@ tags=[ "folders", ], + responses={ + i.status_code: {"model": EnvelopeE[ErrorGet]} + for i in _TO_HTTP_ERROR_MAP.values() + }, ) diff --git a/api/specs/web-server/_workspaces.py b/api/specs/web-server/_workspaces.py index 958b8457bca9..a2cc811bd8c1 100644 --- a/api/specs/web-server/_workspaces.py +++ b/api/specs/web-server/_workspaces.py @@ -9,7 +9,7 @@ from enum import Enum from typing import Annotated -from _common import as_query +from _common import EnvelopeE, as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.workspaces import ( WorkspaceCreateBodyParams, @@ -17,7 +17,9 @@ WorkspaceReplaceBodyParams, ) from models_library.generics import Envelope +from models_library.rest_error import ErrorGet from simcore_service_webserver._meta import API_VTAG +from simcore_service_webserver.folders._exceptions_handlers import _TO_HTTP_ERROR_MAP from simcore_service_webserver.workspaces._groups_api import WorkspaceGroupGet from simcore_service_webserver.workspaces._models import ( WorkspacesGroupsBodyParams, @@ -31,6 +33,10 @@ tags=[ "workspaces", ], + responses={ + i.status_code: {"model": EnvelopeE[ErrorGet]} + for i in _TO_HTTP_ERROR_MAP.values() + }, ) diff --git a/services/web/server/src/simcore_service_webserver/login/utils.py b/services/web/server/src/simcore_service_webserver/login/utils.py index 07cc9a4154c2..22d0a57cf3e8 100644 --- a/services/web/server/src/simcore_service_webserver/login/utils.py +++ b/services/web/server/src/simcore_service_webserver/login/utils.py @@ -4,10 +4,10 @@ from aiohttp import web from common_library.json_serialization import json_dumps from models_library.products import ProductName +from models_library.rest_error import LogMessageType from models_library.users import UserID from pydantic import PositiveInt from servicelib.aiohttp import observer -from servicelib.aiohttp.rest_models import LogMessageType from servicelib.aiohttp.status import HTTP_200_OK from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from simcore_postgres_database.models.users import UserRole From e5abec10534d5d4174056a0dba3b47f2eb9c3abe Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:31:36 +0100 Subject: [PATCH 06/59] oas --- .../api/v0/openapi.yaml | 488 ++++++++++++++++-- 1 file changed, 437 insertions(+), 51 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 688d2187cb3c..8dbf2542ee72 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -93,7 +93,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/verify-phone-number: post: tags: @@ -160,7 +160,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/validate-code-login: post: tags: @@ -186,7 +186,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/two_factor:resend: post: tags: @@ -212,7 +212,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/logout: post: tags: @@ -248,7 +248,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/reset-password: post: tags: @@ -274,7 +274,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/reset-password/{code}: post: tags: @@ -307,7 +307,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/change-password: post: tags: @@ -333,19 +333,19 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' '409': description: mismatch between new and confirmation passwords content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' '422': description: current password is invalid content: application/json: schema: - $ref: '#/components/schemas/Envelope_Error_' + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' /v0/auth/confirmation/{code}: get: tags: @@ -2613,6 +2613,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_FolderGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable get: tags: - folders @@ -2679,6 +2703,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_list_FolderGet__' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/folders:search: get: tags: @@ -2734,6 +2782,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_list_FolderGet__' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/folders/{folder_id}: get: tags: @@ -2756,6 +2828,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_FolderGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable put: tags: - folders @@ -2783,6 +2879,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_FolderGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable delete: tags: - folders @@ -2800,6 +2920,30 @@ paths: responses: '204': description: Successful Response + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/tasks: get: tags: @@ -4396,7 +4540,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: ProjectNotFoundError, UserDefaultWalletNotFoundError + description: UserDefaultWalletNotFoundError, ProjectNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': @@ -5821,6 +5965,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_WorkspaceGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable get: tags: - workspaces @@ -5867,6 +6035,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_list_WorkspaceGet__' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/workspaces/{workspace_id}: get: tags: @@ -5889,6 +6081,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_WorkspaceGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable put: tags: - workspaces @@ -5916,6 +6132,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_WorkspaceGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable delete: tags: - workspaces @@ -5933,6 +6173,30 @@ paths: responses: '204': description: Successful Response + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/workspaces/{workspace_id}/groups/{group_id}: post: tags: @@ -5970,6 +6234,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_WorkspaceGroupGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable put: tags: - workspaces @@ -6006,6 +6294,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_WorkspaceGroupGet_' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable delete: tags: - workspaces @@ -6032,6 +6344,30 @@ paths: responses: '204': description: Successful Response + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/workspaces/{workspace_id}/groups: get: tags: @@ -6055,6 +6391,30 @@ paths: application/json: schema: $ref: '#/components/schemas/Envelope_list_WorkspaceGroupGet__' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Not Found + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Forbidden + '409': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Conflict + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + description: Service Unavailable /v0/email:test: post: tags: @@ -7408,6 +7768,19 @@ components: additionalProperties: false type: object title: EmptyModel + EnvelopeE_ErrorGet_: + properties: + error: + anyOf: + - $ref: '#/components/schemas/ErrorGet' + - type: 'null' + data: + anyOf: + - {} + - type: 'null' + title: Data + type: object + title: EnvelopeE[ErrorGet] Envelope_AppStatusCheck_: properties: data: @@ -7486,19 +7859,6 @@ components: title: Error type: object title: Envelope[ComputationTaskGet] - Envelope_Error_: - properties: - data: - anyOf: - - $ref: '#/components/schemas/Error' - - type: 'null' - error: - anyOf: - - {} - - type: 'null' - title: Error - type: object - title: Envelope[Error] Envelope_FileMetaDataGet_: properties: data: @@ -8897,60 +9257,69 @@ components: title: Error type: object title: Envelope[str] - Error: + ErrorGet: properties: - logs: - anyOf: - - items: - $ref: '#/components/schemas/Log' - type: array - - type: 'null' - title: Logs - description: log messages - errors: + message: + type: string + minLength: 5 + title: Message + description: Message displayed to the user + supportId: anyOf: - - items: - $ref: '#/components/schemas/ErrorItem' - type: array + - type: string + maxLength: 100 + minLength: 1 - type: 'null' - title: Errors - description: errors metadata + title: Supportid + description: ID to track the incident during support status: - anyOf: - - type: integer - - type: 'null' + type: integer title: Status - description: HTTP error code + default: 400 + deprecated: true + errors: + items: + $ref: '#/components/schemas/ErrorItemType' + type: array + title: Errors + default: [] + deprecated: true + logs: + items: + $ref: '#/components/schemas/LogMessageType' + type: array + title: Logs + default: [] + deprecated: true type: object - title: Error - ErrorItem: + required: + - message + title: ErrorGet + ErrorItemType: properties: code: type: string title: Code - description: Typically the name of the exception that produced it otherwise - some known error code message: type: string title: Message - description: Error message specific to this item resource: anyOf: - type: string - type: 'null' title: Resource - description: API resource affected by this error field: anyOf: - type: string - type: 'null' title: Field - description: Specific field within the resource type: object required: - code - message - title: ErrorItem + - resource + - field + title: ErrorItemType ExtractedResults: properties: progress: @@ -9944,6 +10313,23 @@ components: - WARNING - ERROR title: LogLevel + LogMessageType: + properties: + message: + type: string + title: Message + level: + type: string + title: Level + default: INFO + logger: + type: string + title: Logger + default: user + type: object + required: + - message + title: LogMessageType LoginBody: properties: email: From 15adfaff5bc41b27eb1cd1bd5053d247756a41f5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:52:16 +0100 Subject: [PATCH 07/59] error enveloped --- api/specs/web-server/_auth.py | 27 +++++++++--------- api/specs/web-server/_folders.py | 7 ++--- api/specs/web-server/_workspaces.py | 7 ++--- .../src/models_library/rest_error.py | 28 +++++++++++++++---- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/api/specs/web-server/_auth.py b/api/specs/web-server/_auth.py index ecb8c802b968..d47fe0aae5ee 100644 --- a/api/specs/web-server/_auth.py +++ b/api/specs/web-server/_auth.py @@ -6,7 +6,6 @@ from typing import Any -from _common import EnvelopeE from fastapi import APIRouter, status from models_library.api_schemas_webserver.auth import ( AccountRequestInfo, @@ -15,7 +14,7 @@ UnregisterCheck, ) from models_library.generics import Envelope -from models_library.rest_error import ErrorGet, Log +from models_library.rest_error import EnvelopedError, Log from pydantic import BaseModel, Field, confloat from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.login._2fa_handlers import Resend2faBody @@ -76,7 +75,7 @@ async def register(_body: RegisterBody): "/auth/unregister", response_model=Envelope[Log], status_code=status.HTTP_200_OK, - responses={status.HTTP_409_CONFLICT: {"model": EnvelopeE[ErrorGet]}}, + responses={status.HTTP_409_CONFLICT: {"model": EnvelopedError}}, ) async def unregister_account(_body: UnregisterCheck): ... @@ -108,7 +107,7 @@ async def phone_confirmation(_body: PhoneConfirmationBody): responses={ # status.HTTP_503_SERVICE_UNAVAILABLE status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized reset due to invalid token code", } }, @@ -123,7 +122,7 @@ async def login(_body: LoginBody): operation_id="auth_login_2fa", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized reset due to invalid token code", } }, @@ -138,7 +137,7 @@ async def login_2fa(_body: LoginTwoFactorAuthBody): operation_id="auth_resend_2fa_code", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized reset due to invalid token code", } }, @@ -162,7 +161,7 @@ async def logout(_body: LogoutBody): status_code=status.HTTP_204_NO_CONTENT, responses={ status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized reset due to invalid token code", } }, @@ -175,7 +174,7 @@ async def check_auth(): "/auth/reset-password", response_model=Envelope[Log], operation_id="auth_reset_password", - responses={status.HTTP_503_SERVICE_UNAVAILABLE: {"model": EnvelopeE[ErrorGet]}}, + responses={status.HTTP_503_SERVICE_UNAVAILABLE: {"model": EnvelopedError}}, ) async def reset_password(_body: ResetPasswordBody): """a non logged-in user requests a password reset""" @@ -187,7 +186,7 @@ async def reset_password(_body: ResetPasswordBody): operation_id="auth_reset_password_allowed", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized reset due to invalid token code", } }, @@ -202,11 +201,11 @@ async def reset_password_allowed(code: str, _body: ResetPasswordConfirmation): operation_id="auth_change_email", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized user. Login required", }, status.HTTP_503_SERVICE_UNAVAILABLE: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unable to send confirmation email", }, }, @@ -234,15 +233,15 @@ class PasswordCheckSchema(BaseModel): operation_id="auth_change_password", responses={ status.HTTP_401_UNAUTHORIZED: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "unauthorized user. Login required", }, status.HTTP_409_CONFLICT: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "mismatch between new and confirmation passwords", }, status.HTTP_422_UNPROCESSABLE_ENTITY: { - "model": EnvelopeE[ErrorGet], + "model": EnvelopedError, "description": "current password is invalid", }, }, diff --git a/api/specs/web-server/_folders.py b/api/specs/web-server/_folders.py index 1ca5b74e39c1..ff71d9ae13e3 100644 --- a/api/specs/web-server/_folders.py +++ b/api/specs/web-server/_folders.py @@ -9,7 +9,7 @@ from typing import Annotated -from _common import EnvelopeE, as_query +from _comon import as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.folders_v2 import ( FolderCreateBodyParams, @@ -17,7 +17,7 @@ FolderReplaceBodyParams, ) from models_library.generics import Envelope -from models_library.rest_error import ErrorGet +from models_library.rest_error import EnvelopedError from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.folders._exceptions_handlers import _TO_HTTP_ERROR_MAP from simcore_service_webserver.folders._models import ( @@ -32,8 +32,7 @@ "folders", ], responses={ - i.status_code: {"model": EnvelopeE[ErrorGet]} - for i in _TO_HTTP_ERROR_MAP.values() + i.status_code: {"model": EnvelopedError} for i in _TO_HTTP_ERROR_MAP.values() }, ) diff --git a/api/specs/web-server/_workspaces.py b/api/specs/web-server/_workspaces.py index a2cc811bd8c1..fce290fffb0c 100644 --- a/api/specs/web-server/_workspaces.py +++ b/api/specs/web-server/_workspaces.py @@ -9,7 +9,7 @@ from enum import Enum from typing import Annotated -from _common import EnvelopeE, as_query +from _common import as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.workspaces import ( WorkspaceCreateBodyParams, @@ -17,7 +17,7 @@ WorkspaceReplaceBodyParams, ) from models_library.generics import Envelope -from models_library.rest_error import ErrorGet +from models_library.rest_error import EnvelopedError from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.folders._exceptions_handlers import _TO_HTTP_ERROR_MAP from simcore_service_webserver.workspaces._groups_api import WorkspaceGroupGet @@ -34,8 +34,7 @@ "workspaces", ], responses={ - i.status_code: {"model": EnvelopeE[ErrorGet]} - for i in _TO_HTTP_ERROR_MAP.values() + i.status_code: {"model": EnvelopedError} for i in _TO_HTTP_ERROR_MAP.values() }, ) diff --git a/packages/models-library/src/models_library/rest_error.py b/packages/models-library/src/models_library/rest_error.py index 26f56cf18917..66a947495252 100644 --- a/packages/models-library/src/models_library/rest_error.py +++ b/packages/models-library/src/models_library/rest_error.py @@ -1,6 +1,7 @@ from dataclasses import dataclass -from typing import Annotated +from typing import Annotated, Any +from models_library.generics import Envelope from pydantic import BaseModel, ConfigDict, Field from .basic_types import IDStr, LogLevel @@ -75,8 +76,7 @@ class ErrorGet(BaseModel): Field(description="ID to track the incident during support", alias="supportId"), ] = None - # NOTE: The fields blow are DEPRECATED. - # Still here to keep compatibilty with front-end until updated + # NOTE: The fields blow are DEPRECATED. Still here to keep compatibilty with front-end until updated status: Annotated[int, Field(deprecated=True)] = 400 errors: Annotated[ list[ErrorItemType], @@ -93,11 +93,29 @@ class ErrorGet(BaseModel): frozen=True, json_schema_extra={ "examples": [ - {"msg": "Sorry you do not have sufficient access rights for product"}, { - "msg": "Opps this error was unexpected. We are working on that!", + "message": "Sorry you do not have sufficient access rights for product" + }, + { + "message": "Opps this error was unexpected. We are working on that!", "supportId": "OEC:12346789", }, ] }, ) + + +class EnvelopedError(Envelope[Any]): + error: ErrorGet # type: ignore + + model_config = ConfigDict( + json_schema_extra={ + "examples": [ + {"error": {"message": "display error message here"}}, + { + "error": {"message": "failure", "supportId": "OEC:123455"}, + "data": None, + }, + ] + }, + ) From 4a7a3edf061190643320b2c506b9f0407c5532e7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:53:11 +0100 Subject: [PATCH 08/59] fix --- api/specs/web-server/_folders.py | 2 +- .../api/v0/openapi.yaml | 166 +++++++++--------- 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/api/specs/web-server/_folders.py b/api/specs/web-server/_folders.py index ff71d9ae13e3..88a2b19ce9e8 100644 --- a/api/specs/web-server/_folders.py +++ b/api/specs/web-server/_folders.py @@ -9,7 +9,7 @@ from typing import Annotated -from _comon import as_query +from _common import as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.folders_v2 import ( FolderCreateBodyParams, diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 8dbf2542ee72..8220a7a2e847 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -93,7 +93,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/verify-phone-number: post: tags: @@ -160,7 +160,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/validate-code-login: post: tags: @@ -186,7 +186,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/two_factor:resend: post: tags: @@ -212,7 +212,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/logout: post: tags: @@ -248,7 +248,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/reset-password: post: tags: @@ -274,7 +274,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/reset-password/{code}: post: tags: @@ -307,7 +307,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/change-password: post: tags: @@ -333,19 +333,19 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' '409': description: mismatch between new and confirmation passwords content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' '422': description: current password is invalid content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' /v0/auth/confirmation/{code}: get: tags: @@ -2617,25 +2617,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable get: tags: @@ -2707,25 +2707,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/folders:search: get: @@ -2786,25 +2786,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/folders/{folder_id}: get: @@ -2832,25 +2832,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable put: tags: @@ -2883,25 +2883,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable delete: tags: @@ -2924,25 +2924,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/tasks: get: @@ -5969,25 +5969,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable get: tags: @@ -6039,25 +6039,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/workspaces/{workspace_id}: get: @@ -6085,25 +6085,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable put: tags: @@ -6136,25 +6136,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable delete: tags: @@ -6177,25 +6177,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/workspaces/{workspace_id}/groups/{group_id}: post: @@ -6238,25 +6238,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable put: tags: @@ -6298,25 +6298,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable delete: tags: @@ -6348,25 +6348,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/workspaces/{workspace_id}/groups: get: @@ -6395,25 +6395,25 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Not Found '403': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Forbidden '409': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Conflict '503': content: application/json: schema: - $ref: '#/components/schemas/EnvelopeE_ErrorGet_' + $ref: '#/components/schemas/EnvelopedError' description: Service Unavailable /v0/email:test: post: @@ -7768,19 +7768,6 @@ components: additionalProperties: false type: object title: EmptyModel - EnvelopeE_ErrorGet_: - properties: - error: - anyOf: - - $ref: '#/components/schemas/ErrorGet' - - type: 'null' - data: - anyOf: - - {} - - type: 'null' - title: Data - type: object - title: EnvelopeE[ErrorGet] Envelope_AppStatusCheck_: properties: data: @@ -9257,6 +9244,19 @@ components: title: Error type: object title: Envelope[str] + EnvelopedError: + properties: + data: + anyOf: + - {} + - type: 'null' + title: Data + error: + $ref: '#/components/schemas/ErrorGet' + type: object + required: + - error + title: EnvelopedError ErrorGet: properties: message: From e604522056b0a46a0ccf93b42a065cfc89cd58c6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 20:13:57 +0100 Subject: [PATCH 09/59] updates error envelope --- packages/models-library/src/models_library/rest_error.py | 4 ++-- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/models-library/src/models_library/rest_error.py b/packages/models-library/src/models_library/rest_error.py index 66a947495252..cacf891c4cea 100644 --- a/packages/models-library/src/models_library/rest_error.py +++ b/packages/models-library/src/models_library/rest_error.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Annotated, Any +from typing import Annotated from models_library.generics import Envelope from pydantic import BaseModel, ConfigDict, Field @@ -105,7 +105,7 @@ class ErrorGet(BaseModel): ) -class EnvelopedError(Envelope[Any]): +class EnvelopedError(Envelope[None]): error: ErrorGet # type: ignore model_config = ConfigDict( diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 8220a7a2e847..33979c6bf3dd 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -9247,9 +9247,7 @@ components: EnvelopedError: properties: data: - anyOf: - - {} - - type: 'null' + type: 'null' title: Data error: $ref: '#/components/schemas/ErrorGet' From b1d545aa1ed262d7785b5cb806e755721e98d880 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 20:29:27 +0100 Subject: [PATCH 10/59] fixes agent --- .../src/servicelib/logging_utils.py | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/packages/service-library/src/servicelib/logging_utils.py b/packages/service-library/src/servicelib/logging_utils.py index b6b67f8b1634..1ee9494a84be 100644 --- a/packages/service-library/src/servicelib/logging_utils.py +++ b/packages/service-library/src/servicelib/logging_utils.py @@ -51,6 +51,27 @@ } +class LogExtra(TypedDict): + log_uid: NotRequired[str] + log_oec: NotRequired[str] + + +def get_log_record_extra( + *, + user_id: int | str | None = None, + error_code: str | None = None, +) -> LogExtra | None: + extra: LogExtra = {} + + if user_id: + assert int(user_id) > 0 # nosec + extra["log_uid"] = f"{user_id}" + if error_code: + extra["log_oec"] = error_code + + return extra or None + + class CustomFormatter(logging.Formatter): """Custom Formatter does these 2 things: 1. Overrides 'funcName' with the value of 'func_name_override', if it exists. @@ -66,8 +87,10 @@ def format(self, record) -> str: record.funcName = record.func_name_override if hasattr(record, "file_name_override"): record.filename = record.file_name_override - if not hasattr(record, "log_uid"): - record.log_uid = None # Default value if user is not provided in the log + + for name in LogExtra.__optional_keys__: # pylint: disable=no-member + if not hasattr(record, name): + setattr(record, name, None) if self.log_format_local_dev_enabled: levelname = record.levelname @@ -79,11 +102,6 @@ def format(self, record) -> str: return super().format(record).replace("\n", "\\n") -class LogExtra(TypedDict): - log_uid: NotRequired[str] - log_oec: NotRequired[str] - - # SEE https://docs.python.org/3/library/logging.html#logrecord-attributes DEFAULT_FORMATTING = ( "log_level=%(levelname)s " @@ -352,22 +370,6 @@ def log_catch(logger: logging.Logger, *, reraise: bool = True) -> Iterator[None] LogMessageStr: TypeAlias = str -def get_log_record_extra( - *, - user_id: int | str | None = None, - error_code: str | None = None, -) -> LogExtra | None: - extra: LogExtra = {} - - if user_id: - assert int(user_id) > 0 # nosec - extra["log_uid"] = f"{user_id}" - if error_code: - extra["log_oec"] = error_code - - return extra or None - - def _un_capitalize(s: str) -> str: return s[:1].lower() + s[1:] if s else "" From ee8fd6c01476ca7353b4d46e6149d5f925b75f7e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 20:30:58 +0100 Subject: [PATCH 11/59] mypy --- packages/models-library/src/models_library/rest_error.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/rest_error.py b/packages/models-library/src/models_library/rest_error.py index cacf891c4cea..3ba2475c9b3a 100644 --- a/packages/models-library/src/models_library/rest_error.py +++ b/packages/models-library/src/models_library/rest_error.py @@ -106,7 +106,7 @@ class ErrorGet(BaseModel): class EnvelopedError(Envelope[None]): - error: ErrorGet # type: ignore + error: ErrorGet model_config = ConfigDict( json_schema_extra={ From dd83c483ac266fee57e233c88a2632f212872e77 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 20:01:57 +0100 Subject: [PATCH 12/59] exception handlers --- .../exceptions_handlers.py | 125 ++++++++++++++++++ .../unit/isolated/test_exceptions_handlers.py | 52 ++++++++ 2 files changed, 177 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 7e1ae0bd3e0d..28ec42d501e6 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -2,6 +2,9 @@ import logging from collections.abc import Iterable from typing import NamedTuple, TypeAlias +from collections.abc import Callable +from contextlib import contextmanager +from typing import NamedTuple, Protocol, TypeAlias from aiohttp import web from servicelib.aiohttp.typing_extension import Handler @@ -12,6 +15,74 @@ _logger = logging.getLogger(__name__) +class WebApiExceptionHandler(Protocol): + def __call__( + self, exception: BaseException, request: web.Request + ) -> web.HTTPError | BaseException | None: + """ + Callback to process an exception raised during a web request, allowing custom handling. + + This function can be implemented to suppress, reraise, or transform the exception + into an `web.HTTPError` (i.e.the errors specified in the web-api) + + Arguments: + exception -- exception raised in web handler during this request + request -- current request + + Returns: + - None: to suppress `exception` + - `exception`: to reraise it + - an instance of `web.HTTPError` to transform to HTTP Api exceptions + """ + + +@contextmanager +def _handled_exception_context( + exception_catch: type[BaseException] | tuple[type[BaseException], ...], + exception_handler: Callable, + **forward_ctx, +): + try: + yield + except exception_catch as e: + if exc := exception_handler(e, **forward_ctx): + assert isinstance(exc, BaseException) + raise exc from e + + _logger.debug( + "%s suppressed %s: %s", exception_handler.__name__, type(e).__name__, f"{e}" + ) + + +def create_exception_handlers_decorator( + exception_handler: WebApiExceptionHandler, + exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception, +): + """ + Creates a function to decorate routes handlers functions + that can catch and handle exceptions raised in the decorated functions + """ + + def _decorator(handler: Handler): + @functools.wraps(handler) + async def _wrapper(request: web.Request) -> web.StreamResponse: + with _handled_exception_context( + exception_types, + exception_handler, + request=request, + ): + return await handler(request) + + return _wrapper + + return _decorator + + +# +# Http Error Map Handler Customization +# + + class HttpErrorInfo(NamedTuple): status_code: int msg_template: str @@ -88,3 +159,57 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: return _wrapper return _decorator +def create__http_error_map_handler( + to_http_error_map: ExceptionToHttpErrorMap, +) -> WebApiExceptionHandler: + """ + Creates a custom `WebApiExceptionHandler` that maps specific exceptions to HTTP errors. + + Given an `ExceptionToHttpErrorMap`, this function returns a handler that checks if an exception + matches one in the map, returning an HTTP error with the mapped status code and message. + Server errors (5xx) include additional logging with request context. Unmapped exceptions are + returned as-is for re-raising. + + Arguments: + to_http_error_map -- Maps exceptions to HTTP status codes and messages. + + Returns: + A web api exception handler + """ + included: list[type[BaseException]] = _sort_exceptions_by_specificity( + list(to_http_error_map.keys()) + ) + + def _handler( + exception: BaseException, + request: web.Request, + ) -> web.HTTPError | BaseException | None: + if exc_cls := next((_ for _ in included if isinstance(exception, _)), None): + http_error_info = to_http_error_map[exc_cls] + + # safe formatting, i.e. does not raise + user_msg = http_error_info.msg_template.format_map( + _DefaultDict(getattr(exception, "__dict__", {})) + ) + + http_error_cls = get_http_error_class_or_none(http_error_info.status_code) + assert http_error_cls # nosec + + if is_5xx_server_error(http_error_info.status_code): + _logger.exception( + **create_troubleshotting_log_kwargs( + user_msg, + error=exception, + error_context={ + "request": request, + "request.remote": f"{request.remote}", + "request.method": f"{request.method}", + "request.path": f"{request.path}", + }, + ) + ) + return http_error_cls(reason=user_msg) + # NOTE: not in my list, return so it gets reraised + return exception + + return _handler diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index 27cde72283b5..83ade5070339 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -16,6 +16,9 @@ from simcore_service_webserver.exceptions_handlers import ( HttpErrorInfo, _sort_exceptions_by_specificity, + _handled_exception_context, + _sort_exceptions_by_specificity, + create__http_error_map_handler, create_exception_handlers_decorator, ) @@ -65,6 +68,55 @@ def test_sort_exceptions_by_specificity(): assert not issubclass(exc_after, exc), f"{got_exceptions_cls=}" +@pytest.fixture +def fake_request() -> web.Request: + return make_mocked_request("GET", "/foo") + + +def test_http_error_map_handler_factory(fake_request: web.Request): + + exc_handler = create__http_error_map_handler( + { + OneError: HttpErrorInfo( + status.HTTP_400_BAD_REQUEST, "Error One mapped to 400" + ) + } + ) + + # Converts exception in map + got_exc = exc_handler(OneError(), fake_request) + + assert isinstance(got_exc, web.HTTPBadRequest) + assert got_exc.reason == "Error One mapped to 400" + + # By-passes exceptions not listed + err = RuntimeError() + assert exc_handler(err, fake_request) is err + + +def test_handled_exception_context(fake_request: web.Request): + def _suppress_handler(exception, request): + assert request == fake_request + assert isinstance( + exception, BasePluginError + ), "only BasePluginError exceptions should call this handler" + return None # noqa: RET501, PLR1711 + + def _fun(raises): + with _handled_exception_context( + BasePluginError, _suppress_handler, request=fake_request + ): + raise raises + + # checks + _fun(raises=OneError) + _fun(raises=OtherError) + + with pytest.raises(ArithmeticError): + _fun(raises=ArithmeticError) + + + async def test_exception_handlers_decorator( caplog: pytest.LogCaptureFixture, ): From 78e868add891668c4c6e0c532458eca31f26d0d3 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 20:37:12 +0100 Subject: [PATCH 13/59] slowly conforming to fastapi --- .../exceptions_handlers.py | 137 +++++++++++------- 1 file changed, 84 insertions(+), 53 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 28ec42d501e6..2c2aa9d46d50 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -8,7 +8,7 @@ from aiohttp import web from servicelib.aiohttp.typing_extension import Handler -from servicelib.aiohttp.web_exceptions_extension import get_http_error_class_or_none +from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error @@ -18,12 +18,12 @@ class WebApiExceptionHandler(Protocol): def __call__( self, exception: BaseException, request: web.Request - ) -> web.HTTPError | BaseException | None: + ) -> web.HTTPException | BaseException | None: """ Callback to process an exception raised during a web request, allowing custom handling. This function can be implemented to suppress, reraise, or transform the exception - into an `web.HTTPError` (i.e.the errors specified in the web-api) + into an `web.HTTPException` (i.e. exceptions defined at the web-api) Arguments: exception -- exception raised in web handler during this request @@ -32,7 +32,7 @@ def __call__( Returns: - None: to suppress `exception` - `exception`: to reraise it - - an instance of `web.HTTPError` to transform to HTTP Api exceptions + - an instance of `web.HTTPException` to transform to HTTP Api exceptions (NOTE: that they can either be errors or success!) """ @@ -59,8 +59,7 @@ def create_exception_handlers_decorator( exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception, ): """ - Creates a function to decorate routes handlers functions - that can catch and handle exceptions raised in the decorated functions + Factory to create decorators that wraps exceptions on api-handler functions """ def _decorator(handler: Handler): @@ -79,10 +78,72 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: # -# Http Error Map Handler Customization +# CUSTOM EXCEPTIONS HANDLERS # +class _DefaultDict(dict): + def __missing__(self, key): + return f"'{key}=?'" + + +_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[ + int, type[web.HTTPException] +] = get_all_aiohttp_http_exceptions(web.HTTPException) + + +def make_handler_exception_to_status_code( + exception_cls: type[BaseException], status_code: int, user_msg_template: str +) -> WebApiExceptionHandler: + """ + Creates a custom `WebApiExceptionHandler` that maps specific exception to an http status code error + + Given an `ExceptionToHttpErrorMap`, this function returns a handler that checks if an exception + matches one in the map, returning an HTTP error with the mapped status code and message. + Server errors (5xx) include additional logging with request context. Unmapped exceptions are + returned as-is for re-raising. + + Arguments: + exception_cls: exception raise during the request + status_code: the http status code to associate at the web-api interface to this error + user_msg_template: a template string to pass to the HttpError + + Returns: + A web api exception handler + """ + + def _exception_handler( + exception: BaseException, + request: web.Request, + ) -> web.HTTPException | BaseException | None: + assert isinstance(exception, exception_cls) # nosec + + # safe formatting, i.e. does not raise + user_msg = user_msg_template.format_map( + _DefaultDict(getattr(exception, "__dict__", {})) + ) + + http_error_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code] + assert http_error_cls # nosec + + if is_5xx_server_error(status_code): + _logger.exception( + **create_troubleshotting_log_kwargs( + user_msg, + error=exception, + error_context={ + "request": request, + "request.remote": f"{request.remote}", + "request.method": f"{request.method}", + "request.path": f"{request.path}", + }, + ) + ) + return http_error_cls(reason=user_msg) + + return _exception_handler + + class HttpErrorInfo(NamedTuple): status_code: int msg_template: str @@ -91,11 +152,6 @@ class HttpErrorInfo(NamedTuple): ExceptionToHttpErrorMap: TypeAlias = dict[type[BaseException], HttpErrorInfo] -class _DefaultDict(dict): - def __missing__(self, key): - return f"'{key}=?'" - - def _sort_exceptions_by_specificity( exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True ) -> list[type[BaseException]]: @@ -162,54 +218,29 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: def create__http_error_map_handler( to_http_error_map: ExceptionToHttpErrorMap, ) -> WebApiExceptionHandler: - """ - Creates a custom `WebApiExceptionHandler` that maps specific exceptions to HTTP errors. - - Given an `ExceptionToHttpErrorMap`, this function returns a handler that checks if an exception - matches one in the map, returning an HTTP error with the mapped status code and message. - Server errors (5xx) include additional logging with request context. Unmapped exceptions are - returned as-is for re-raising. - Arguments: - to_http_error_map -- Maps exceptions to HTTP status codes and messages. + _exception_handlers = { + exc_cls: make_handler_exception_to_status_code( + exception_cls=exc_cls, + status_code=http_error_info.status_code, + user_msg_template=http_error_info.msg_template, + ) + for exc_cls, http_error_info in to_http_error_map.items() + } - Returns: - A web api exception handler - """ - included: list[type[BaseException]] = _sort_exceptions_by_specificity( - list(to_http_error_map.keys()) + _catch_exceptions = _sort_exceptions_by_specificity( + list(_exception_handlers.keys()) ) - def _handler( + def _exception_handler( exception: BaseException, request: web.Request, ) -> web.HTTPError | BaseException | None: - if exc_cls := next((_ for _ in included if isinstance(exception, _)), None): - http_error_info = to_http_error_map[exc_cls] - - # safe formatting, i.e. does not raise - user_msg = http_error_info.msg_template.format_map( - _DefaultDict(getattr(exception, "__dict__", {})) - ) - - http_error_cls = get_http_error_class_or_none(http_error_info.status_code) - assert http_error_cls # nosec - - if is_5xx_server_error(http_error_info.status_code): - _logger.exception( - **create_troubleshotting_log_kwargs( - user_msg, - error=exception, - error_context={ - "request": request, - "request.remote": f"{request.remote}", - "request.method": f"{request.method}", - "request.path": f"{request.path}", - }, - ) - ) - return http_error_cls(reason=user_msg) + if exc_cls := next( + (_ for _ in _catch_exceptions if isinstance(exception, _)), None + ): + return _exception_handlers[exc_cls](request=request, exception=exception) # NOTE: not in my list, return so it gets reraised return exception - return _handler + return _exception_handler From b82a939544eadccb5effbae9ae5c2682178a631d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 21:07:46 +0100 Subject: [PATCH 14/59] compiling old ideas --- .../exceptions_handlers.py | 136 +++++++++++++++++- .../unit/isolated/test_exceptions_handlers.py | 40 ++++++ 2 files changed, 172 insertions(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 2c2aa9d46d50..3775c8de785f 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -1,10 +1,9 @@ import functools import logging -from collections.abc import Iterable -from typing import NamedTuple, TypeAlias -from collections.abc import Callable +from collections.abc import Awaitable, Callable, MutableMapping from contextlib import contextmanager -from typing import NamedTuple, Protocol, TypeAlias +from http import HTTPStatus +from typing import Any, NamedTuple, Protocol, TypeAlias, cast from aiohttp import web from servicelib.aiohttp.typing_extension import Handler @@ -14,6 +13,10 @@ _logger = logging.getLogger(__name__) +# +# Definition +# + class WebApiExceptionHandler(Protocol): def __call__( @@ -244,3 +247,128 @@ def _exception_handler( return exception return _exception_handler + + +# OLD VERSION OF IT with really good ideas ! -------------------- + + +# +# Defines exception handler as somethign that returns responses, as fastapi, and not new exceptions! +# in reality this can be reinterpreted in aiohttp since all responses can be represented as exceptions. +# Not true because fastapi.HTTPException does actually the same! Better responses because this weay we do not +# need return None or the exception itself which as we saw in the tests, it causes troubles! +# + +ExceptionHandler: TypeAlias = Callable[ + [web.Request, Exception], Awaitable[web.Response] +] + +ExceptionsMap: TypeAlias = dict[type[Exception], type[web.HTTPException]] + +ExceptionHandlerRegistry: TypeAlias = dict[type[Exception], ExceptionHandler] + + +# injects the exceptions in a scope, e.g. an app state or some container like routes/ but to use in a module only e.g. +# as decorator or context manager? + + +def setup_exception_handlers(scope: MutableMapping[str, Any]): + scope["exceptions_handlers"] = {} + scope["exceptions_map"] = {} + # but this is very specific because it responds with only status! you migh want to have different + # type of bodies, etc + + +def _get_exception_handler_registry( + scope: MutableMapping[str, Any] +) -> ExceptionHandlerRegistry: + return scope.get("exceptions_handlers", {}) + + +def add_exception_handler( + scope: MutableMapping[str, Any], + exc_class: type[Exception], + handler: ExceptionHandler, +): + scope["exceptions_handlers"][exc_class] = handler + + +def _create_exception_handler_mapper( + exc_class: type[Exception], + http_exc_class: type[web.HTTPException], +) -> ExceptionHandler: + error_code = f"{exc_class.__name__}" # status_code.error_code + + async def _exception_handler(_: web.Request, exc: Exception) -> web.Response: + # TODO: a better way to add error_code. TODO: create the envelope! + return http_exc_class(reason=f"{exc} [{error_code}]") + + return _exception_handler + + +def add_exception_mapper( + scope: MutableMapping[str, Any], + exc_class: type[Exception], + http_exc_class: type[web.HTTPException], +): + # adds exception handler to scope + scope["exceptions_map"][exc_class] = http_exc_class + add_exception_handler( + scope, + exc_class, + handler=_create_exception_handler_mapper(exc_class, http_exc_class), + ) + + +async def handle_request_with_exception_handling_in_scope( + handler: Handler, + request: web.Request, + scope: MutableMapping[str, Any] | None = None, +) -> web.Response: + try: + resp = await handler(request) + return cast(web.Response, resp) + + except Exception as exc: # pylint: disable=broad-exception-caught + scope = scope or request.app + if exception_handler := _get_exception_handler_registry(scope).get( + type(exc), None + ): + resp = await exception_handler(request, exc) + else: + resp = web.HTTPInternalServerError() + + if isinstance(resp, web.HTTPError): + # NOTE: this should not happen anymore! as far as I understand!? + raise resp from exc + return resp + + +def handle_registered_exceptions(scope: MutableMapping[str, Any] | None = None): + def _decorator(handler: Handler): + @functools.wraps(handler) + async def _wrapper(request: web.Request) -> web.Response: + return await handle_request_with_exception_handling_in_scope( + handler, request, scope + ) + + return _wrapper + + return _decorator + + +# If I have all the status codes mapped, I can definitively use that info to create `responses` +# for fastapi to render the OAS preoperly +def openapi_error_responses( + exceptions_map: ExceptionsMap, +) -> dict[HTTPStatus, dict[str, Any]]: + responses = {} + + for exc_class, http_exc_class in exceptions_map.items(): + status_code = HTTPStatus(http_exc_class.status_code) + if status_code not in responses: + responses[status_code] = {"description": f"{exc_class.__name__}"} + else: + responses[status_code]["description"] += f", {exc_class.__name__}" + + return responses diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index 83ade5070339..86b2cf4e0ea1 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -13,6 +13,12 @@ from aiohttp.test_utils import make_mocked_request from servicelib.aiohttp import status from simcore_service_webserver.errors import WebServerBaseError +from simcore_service_webserver.exception_handlers import ( + add_exception_handler, + add_exception_mapper, + handle_registered_exceptions, + setup_exception_handlers, +) from simcore_service_webserver.exceptions_handlers import ( HttpErrorInfo, _sort_exceptions_by_specificity, @@ -167,3 +173,37 @@ async def _rest_handler(request: web.Request) -> web.Response: assert caplog.records assert caplog.records[0].levelno == logging.ERROR + # typically capture by last + with pytest.raises(ArithmeticError): + resp = await _rest_handler( + make_mocked_request("GET", "/foo?raise=ArithmeticError") + ) + + +def test_it(): + class MyException(Exception): + ... + + class OtherException(Exception): + ... + + app = web.Application() + + async def my_error_handler(request: web.Request, exc: MyException): + return web.HTTPNotFound() + + # define at the app level + setup_exception_handlers(app) + add_exception_handler(app, MyException, my_error_handler) + add_exception_mapper(app, OtherException, web.HTTPNotFound) + + async def foo(): + raise MyException + + routes = web.RouteTableDef() + + @routes.get("/home") + @handle_registered_exceptions() + async def home(_request: web.Request): + await foo() + return web.HTTPOk() From 25e1b758bb005b247bbd4de0068f9fe4ac134d04 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:13:49 +0100 Subject: [PATCH 15/59] split and renamed --- .../exceptions_handlers.py | 389 +----------------- .../exceptions_handlers_base.py | 80 ++++ .../exceptions_handlers_base_2.py | 130 ++++++ .../exceptions_handlers_http_error_map.py | 137 ++++++ .../folders/_exceptions_handlers.py | 9 +- .../projects/_trash_handlers.py | 9 +- .../unit/isolated/test_exceptions_handlers.py | 47 ++- 7 files changed, 401 insertions(+), 400 deletions(-) create mode 100644 services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py create mode 100644 services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py create mode 100644 services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 3775c8de785f..36f818000a1f 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -1,374 +1,15 @@ -import functools -import logging -from collections.abc import Awaitable, Callable, MutableMapping -from contextlib import contextmanager -from http import HTTPStatus -from typing import Any, NamedTuple, Protocol, TypeAlias, cast - -from aiohttp import web -from servicelib.aiohttp.typing_extension import Handler -from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions -from servicelib.logging_errors import create_troubleshotting_log_kwargs -from servicelib.status_codes_utils import is_5xx_server_error - -_logger = logging.getLogger(__name__) - -# -# Definition -# - - -class WebApiExceptionHandler(Protocol): - def __call__( - self, exception: BaseException, request: web.Request - ) -> web.HTTPException | BaseException | None: - """ - Callback to process an exception raised during a web request, allowing custom handling. - - This function can be implemented to suppress, reraise, or transform the exception - into an `web.HTTPException` (i.e. exceptions defined at the web-api) - - Arguments: - exception -- exception raised in web handler during this request - request -- current request - - Returns: - - None: to suppress `exception` - - `exception`: to reraise it - - an instance of `web.HTTPException` to transform to HTTP Api exceptions (NOTE: that they can either be errors or success!) - """ - - -@contextmanager -def _handled_exception_context( - exception_catch: type[BaseException] | tuple[type[BaseException], ...], - exception_handler: Callable, - **forward_ctx, -): - try: - yield - except exception_catch as e: - if exc := exception_handler(e, **forward_ctx): - assert isinstance(exc, BaseException) - raise exc from e - - _logger.debug( - "%s suppressed %s: %s", exception_handler.__name__, type(e).__name__, f"{e}" - ) - - -def create_exception_handlers_decorator( - exception_handler: WebApiExceptionHandler, - exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception, -): - """ - Factory to create decorators that wraps exceptions on api-handler functions - """ - - def _decorator(handler: Handler): - @functools.wraps(handler) - async def _wrapper(request: web.Request) -> web.StreamResponse: - with _handled_exception_context( - exception_types, - exception_handler, - request=request, - ): - return await handler(request) - - return _wrapper - - return _decorator - - -# -# CUSTOM EXCEPTIONS HANDLERS -# - - -class _DefaultDict(dict): - def __missing__(self, key): - return f"'{key}=?'" - - -_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[ - int, type[web.HTTPException] -] = get_all_aiohttp_http_exceptions(web.HTTPException) - - -def make_handler_exception_to_status_code( - exception_cls: type[BaseException], status_code: int, user_msg_template: str -) -> WebApiExceptionHandler: - """ - Creates a custom `WebApiExceptionHandler` that maps specific exception to an http status code error - - Given an `ExceptionToHttpErrorMap`, this function returns a handler that checks if an exception - matches one in the map, returning an HTTP error with the mapped status code and message. - Server errors (5xx) include additional logging with request context. Unmapped exceptions are - returned as-is for re-raising. - - Arguments: - exception_cls: exception raise during the request - status_code: the http status code to associate at the web-api interface to this error - user_msg_template: a template string to pass to the HttpError - - Returns: - A web api exception handler - """ - - def _exception_handler( - exception: BaseException, - request: web.Request, - ) -> web.HTTPException | BaseException | None: - assert isinstance(exception, exception_cls) # nosec - - # safe formatting, i.e. does not raise - user_msg = user_msg_template.format_map( - _DefaultDict(getattr(exception, "__dict__", {})) - ) - - http_error_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code] - assert http_error_cls # nosec - - if is_5xx_server_error(status_code): - _logger.exception( - **create_troubleshotting_log_kwargs( - user_msg, - error=exception, - error_context={ - "request": request, - "request.remote": f"{request.remote}", - "request.method": f"{request.method}", - "request.path": f"{request.path}", - }, - ) - ) - return http_error_cls(reason=user_msg) - - return _exception_handler - - -class HttpErrorInfo(NamedTuple): - status_code: int - msg_template: str - - -ExceptionToHttpErrorMap: TypeAlias = dict[type[BaseException], HttpErrorInfo] - - -def _sort_exceptions_by_specificity( - exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True -) -> list[type[BaseException]]: - return sorted( - exceptions, - key=lambda exc: sum(issubclass(e, exc) for e in exceptions if e is not exc), - reverse=not concrete_first, - ) - - -def create_exception_handlers_decorator( - exceptions_catch: type[BaseException] | tuple[type[BaseException], ...], - exc_to_status_map: ExceptionToHttpErrorMap, -): - mapped_classes: tuple[type[BaseException], ...] = tuple( - _sort_exceptions_by_specificity(exc_to_status_map.keys()) - ) - - assert all( # nosec - issubclass(cls, exceptions_catch) for cls in mapped_classes - ), f"Every {mapped_classes=} must inherit by one or more of {exceptions_catch=}" - - def _decorator(handler: Handler): - @functools.wraps(handler) - async def _wrapper(request: web.Request) -> web.StreamResponse: - try: - return await handler(request) - - except exceptions_catch as exc: - if exc_cls := next( - (cls for cls in mapped_classes if isinstance(exc, cls)), None - ): - http_error_info = exc_to_status_map[exc_cls] - - # safe formatting, i.e. does not raise - user_msg = http_error_info.msg_template.format_map( - _DefaultDict(getattr(exc, "__dict__", {})) - ) - - http_error_cls = get_http_error_class_or_none( - http_error_info.status_code - ) - assert http_error_cls # nosec - - if is_5xx_server_error(http_error_info.status_code): - _logger.exception( - **create_troubleshotting_log_kwargs( - user_msg, - error=exc, - error_context={ - "request": request, - "request.remote": f"{request.remote}", - "request.method": f"{request.method}", - "request.path": f"{request.path}", - }, - ) - ) - raise http_error_cls(reason=user_msg) from exc - raise # reraise - - return _wrapper - - return _decorator -def create__http_error_map_handler( - to_http_error_map: ExceptionToHttpErrorMap, -) -> WebApiExceptionHandler: - - _exception_handlers = { - exc_cls: make_handler_exception_to_status_code( - exception_cls=exc_cls, - status_code=http_error_info.status_code, - user_msg_template=http_error_info.msg_template, - ) - for exc_cls, http_error_info in to_http_error_map.items() - } - - _catch_exceptions = _sort_exceptions_by_specificity( - list(_exception_handlers.keys()) - ) - - def _exception_handler( - exception: BaseException, - request: web.Request, - ) -> web.HTTPError | BaseException | None: - if exc_cls := next( - (_ for _ in _catch_exceptions if isinstance(exception, _)), None - ): - return _exception_handlers[exc_cls](request=request, exception=exception) - # NOTE: not in my list, return so it gets reraised - return exception - - return _exception_handler - - -# OLD VERSION OF IT with really good ideas ! -------------------- - - -# -# Defines exception handler as somethign that returns responses, as fastapi, and not new exceptions! -# in reality this can be reinterpreted in aiohttp since all responses can be represented as exceptions. -# Not true because fastapi.HTTPException does actually the same! Better responses because this weay we do not -# need return None or the exception itself which as we saw in the tests, it causes troubles! -# - -ExceptionHandler: TypeAlias = Callable[ - [web.Request, Exception], Awaitable[web.Response] -] - -ExceptionsMap: TypeAlias = dict[type[Exception], type[web.HTTPException]] - -ExceptionHandlerRegistry: TypeAlias = dict[type[Exception], ExceptionHandler] - - -# injects the exceptions in a scope, e.g. an app state or some container like routes/ but to use in a module only e.g. -# as decorator or context manager? - - -def setup_exception_handlers(scope: MutableMapping[str, Any]): - scope["exceptions_handlers"] = {} - scope["exceptions_map"] = {} - # but this is very specific because it responds with only status! you migh want to have different - # type of bodies, etc - - -def _get_exception_handler_registry( - scope: MutableMapping[str, Any] -) -> ExceptionHandlerRegistry: - return scope.get("exceptions_handlers", {}) - - -def add_exception_handler( - scope: MutableMapping[str, Any], - exc_class: type[Exception], - handler: ExceptionHandler, -): - scope["exceptions_handlers"][exc_class] = handler - - -def _create_exception_handler_mapper( - exc_class: type[Exception], - http_exc_class: type[web.HTTPException], -) -> ExceptionHandler: - error_code = f"{exc_class.__name__}" # status_code.error_code - - async def _exception_handler(_: web.Request, exc: Exception) -> web.Response: - # TODO: a better way to add error_code. TODO: create the envelope! - return http_exc_class(reason=f"{exc} [{error_code}]") - - return _exception_handler - - -def add_exception_mapper( - scope: MutableMapping[str, Any], - exc_class: type[Exception], - http_exc_class: type[web.HTTPException], -): - # adds exception handler to scope - scope["exceptions_map"][exc_class] = http_exc_class - add_exception_handler( - scope, - exc_class, - handler=_create_exception_handler_mapper(exc_class, http_exc_class), - ) - - -async def handle_request_with_exception_handling_in_scope( - handler: Handler, - request: web.Request, - scope: MutableMapping[str, Any] | None = None, -) -> web.Response: - try: - resp = await handler(request) - return cast(web.Response, resp) - - except Exception as exc: # pylint: disable=broad-exception-caught - scope = scope or request.app - if exception_handler := _get_exception_handler_registry(scope).get( - type(exc), None - ): - resp = await exception_handler(request, exc) - else: - resp = web.HTTPInternalServerError() - - if isinstance(resp, web.HTTPError): - # NOTE: this should not happen anymore! as far as I understand!? - raise resp from exc - return resp - - -def handle_registered_exceptions(scope: MutableMapping[str, Any] | None = None): - def _decorator(handler: Handler): - @functools.wraps(handler) - async def _wrapper(request: web.Request) -> web.Response: - return await handle_request_with_exception_handling_in_scope( - handler, request, scope - ) - - return _wrapper - - return _decorator - - -# If I have all the status codes mapped, I can definitively use that info to create `responses` -# for fastapi to render the OAS preoperly -def openapi_error_responses( - exceptions_map: ExceptionsMap, -) -> dict[HTTPStatus, dict[str, Any]]: - responses = {} - - for exc_class, http_exc_class in exceptions_map.items(): - status_code = HTTPStatus(http_exc_class.status_code) - if status_code not in responses: - responses[status_code] = {"description": f"{exc_class.__name__}"} - else: - responses[status_code]["description"] += f", {exc_class.__name__}" - - return responses +from .exceptions_handlers_base import create_decorator_from_exception_handler +from .exceptions_handlers_http_error_map import ( + ExceptionToHttpErrorMap, + HttpErrorInfo, + create_exception_handler_from_http_error_map, +) + +__all__: tuple[str, ...] = ( + "create_decorator_from_exception_handler", + "create_exception_handler_from_http_error_map", + "ExceptionToHttpErrorMap", + "HttpErrorInfo", +) + +# nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py new file mode 100644 index 000000000000..3418f46f4ea8 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -0,0 +1,80 @@ +import functools +import logging +from contextlib import contextmanager +from typing import Protocol + +from aiohttp import web +from servicelib.aiohttp.typing_extension import Handler as WebHandler + +_logger = logging.getLogger(__name__) + +# +# Definition +# + + +class WebApiExceptionHandler(Protocol): + __name__: str + + def __call__( + self, exception: BaseException, request: web.Request + ) -> web.HTTPException | BaseException | None: + """ + Callback to process an exception raised during a web request, allowing custom handling. + + This function can be implemented to suppress, reraise, or transform the exception + into an `web.HTTPException` (i.e. exceptions defined at the web-api) + + Arguments: + exception -- exception raised in web handler during this request + request -- current request + + Returns: + - None: to suppress `exception` + - `exception`: to reraise it + - an instance of `web.HTTPException` to transform to HTTP Api exceptions (NOTE: that they can either be errors or success!) + """ + + +@contextmanager +def _handled_exception_context_manager( + exception_catch: type[BaseException] | tuple[type[BaseException], ...], + exception_handler: WebApiExceptionHandler, + **forward_ctx, +): + """Calls `exception_handler` on exceptions raised in this context and caught in `exception_catch`""" + try: + yield + except exception_catch as e: + if exc := exception_handler(e, **forward_ctx): + assert isinstance(exc, BaseException) + raise exc from e + + _logger.debug( + "%s suppressed %s: %s", exception_handler.__name__, type(e).__name__, f"{e}" + ) + + +def create_decorator_from_exception_handler( + exception_handler: WebApiExceptionHandler, + exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception, +): + """Returns a decorator for aiohttp's web.Handler functions + + Builds a decorator function that applies _handled_exception_context to an aiohttp Handler + """ + + def _decorator(handler: WebHandler): + @functools.wraps(handler) + async def _wrapper(request: web.Request) -> web.StreamResponse: + + with _handled_exception_context_manager( + exception_types, + exception_handler, + request=request, + ): + return await handler(request) + + return _wrapper + + return _decorator diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py new file mode 100644 index 000000000000..7468fec8fc80 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py @@ -0,0 +1,130 @@ +import functools +import logging +from collections.abc import Awaitable, Callable, MutableMapping +from http import HTTPStatus +from typing import Any, TypeAlias, cast + +from aiohttp import web + +_logger = logging.getLogger(__name__) + + +# Defines exception handler as somethign that returns responses, as fastapi, and not new exceptions! +# in reality this can be reinterpreted in aiohttp since all responses can be represented as exceptions. +# Not true because fastapi.HTTPException does actually the same! Better responses because this weay we do not +# need return None or the exception itself which as we saw in the tests, it causes troubles! +# + +ExceptionHandler: TypeAlias = Callable[ + [web.Request, Exception], Awaitable[web.Response] +] + +ExceptionsMap: TypeAlias = dict[type[Exception], type[web.HTTPException]] + +ExceptionHandlerRegistry: TypeAlias = dict[type[Exception], ExceptionHandler] + + +# injects the exceptions in a scope, e.g. an app state or some container like routes/ but to use in a module only e.g. +# as decorator or context manager? + + +def setup_exception_handlers(scope: MutableMapping[str, Any]): + scope["exceptions_handlers"] = {} + scope["exceptions_map"] = {} + # but this is very specific because it responds with only status! you migh want to have different + # type of bodies, etc + + +def _get_exception_handler_registry( + scope: MutableMapping[str, Any] +) -> ExceptionHandlerRegistry: + return scope.get("exceptions_handlers", {}) + + +def add_exception_handler( + scope: MutableMapping[str, Any], + exc_class: type[Exception], + handler: ExceptionHandler, +): + scope["exceptions_handlers"][exc_class] = handler + + +def _create_exception_handler_mapper( + exc_class: type[Exception], + http_exc_class: type[web.HTTPException], +) -> ExceptionHandler: + error_code = f"{exc_class.__name__}" # status_code.error_code + + async def _exception_handler(_: web.Request, exc: Exception) -> web.Response: + # TODO: a better way to add error_code. TODO: create the envelope! + return http_exc_class(reason=f"{exc} [{error_code}]") + + return _exception_handler + + +def add_exception_mapper( + scope: MutableMapping[str, Any], + exc_class: type[Exception], + http_exc_class: type[web.HTTPException], +): + # adds exception handler to scope + scope["exceptions_map"][exc_class] = http_exc_class + add_exception_handler( + scope, + exc_class, + handler=_create_exception_handler_mapper(exc_class, http_exc_class), + ) + + +async def handle_request_with_exception_handling_in_scope( + handler: Handler, + request: web.Request, + scope: MutableMapping[str, Any] | None = None, +) -> web.Response: + try: + resp = await handler(request) + return cast(web.Response, resp) + + except Exception as exc: # pylint: disable=broad-exception-caught + scope = scope or request.app + if exception_handler := _get_exception_handler_registry(scope).get( + type(exc), None + ): + resp = await exception_handler(request, exc) + else: + resp = web.HTTPInternalServerError() + + if isinstance(resp, web.HTTPError): + # NOTE: this should not happen anymore! as far as I understand!? + raise resp from exc + return resp + + +def handle_registered_exceptions(scope: MutableMapping[str, Any] | None = None): + def _decorator(handler: Handler): + @functools.wraps(handler) + async def _wrapper(request: web.Request) -> web.Response: + return await handle_request_with_exception_handling_in_scope( + handler, request, scope + ) + + return _wrapper + + return _decorator + + +# If I have all the status codes mapped, I can definitively use that info to create `responses` +# for fastapi to render the OAS preoperly +def openapi_error_responses( + exceptions_map: ExceptionsMap, +) -> dict[HTTPStatus, dict[str, Any]]: + responses = {} + + for exc_class, http_exc_class in exceptions_map.items(): + status_code = HTTPStatus(http_exc_class.status_code) + if status_code not in responses: + responses[status_code] = {"description": f"{exc_class.__name__}"} + else: + responses[status_code]["description"] += f", {exc_class.__name__}" + + return responses diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py new file mode 100644 index 000000000000..bccb887f5377 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -0,0 +1,137 @@ +import logging +from collections.abc import Iterable +from typing import NamedTuple, TypeAlias + +from aiohttp import web +from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions +from servicelib.logging_errors import create_troubleshotting_log_kwargs +from servicelib.status_codes_utils import is_5xx_server_error + +from .exceptions_handlers_base import WebApiExceptionHandler + +_logger = logging.getLogger(__name__) + + +_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[ + int, type[web.HTTPException] +] = get_all_aiohttp_http_exceptions(web.HTTPException) + + +class _DefaultDict(dict): + def __missing__(self, key): + return f"'{key}=?'" + + +class HttpErrorInfo(NamedTuple): + """Info provided to auto-create HTTPError""" + + status_code: int + msg_template: str # sets HTTPError.reason + + +ExceptionToHttpErrorMap: TypeAlias = dict[type[BaseException], HttpErrorInfo] + + +def create_exception_handler_from_http_error( + exception_cls: type[BaseException], + *, + status_code: int, + user_msg_template: str, +) -> WebApiExceptionHandler: + """ + Custom Exception-Handler factory + + Creates a custom `WebApiExceptionHandler` that maps specific exception to a given http status code error + + Given an `ExceptionToHttpErrorMap`, this function returns a handler that checks if an exception + matches one in the map, returning an HTTP error with the mapped status code and message. + Server errors (5xx) include additional logging with request context. Unmapped exceptions are + returned as-is for re-raising. + + Arguments: + exception_cls: exception raise during the request + status_code: the http status code to associate at the web-api interface to this error + user_msg_template: a template string to pass to the HttpError + + Returns: + A web api exception handler + """ + + def _exception_handler( + exception: BaseException, + request: web.Request, + ) -> web.HTTPException | BaseException | None: + assert isinstance(exception, exception_cls) # nosec + + # safe formatting, i.e. does not raise + user_msg = user_msg_template.format_map( + _DefaultDict(getattr(exception, "__dict__", {})) + ) + + http_error_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code] + assert http_error_cls # nosec + + if is_5xx_server_error(status_code): + _logger.exception( + **create_troubleshotting_log_kwargs( + user_msg, + error=exception, + error_context={ + "request": request, + "request.remote": f"{request.remote}", + "request.method": f"{request.method}", + "request.path": f"{request.path}", + }, + ) + ) + return http_error_cls(reason=user_msg) + + return _exception_handler + + +def _sort_exceptions_by_specificity( + exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True +) -> list[type[BaseException]]: + return sorted( + exceptions, + key=lambda exc: sum(issubclass(e, exc) for e in exceptions if e is not exc), + reverse=not concrete_first, + ) + + +def create_exception_handler_from_http_error_map( + exc_to_http_error_map: ExceptionToHttpErrorMap, +) -> WebApiExceptionHandler: + """ + Custom Exception-Handler factory + + Creates a custom `WebApiExceptionHandler` that maps one-to-one exception to status code error codes + + Analogous to `create_exception_handler_from_status_code` but ExceptionToHttpErrorMap as input + """ + + _exception_handlers = { + exc_cls: create_exception_handler_from_http_error( + exception_cls=exc_cls, + status_code=http_error_info.status_code, + user_msg_template=http_error_info.msg_template, + ) + for exc_cls, http_error_info in exc_to_http_error_map.items() + } + + _catch_exceptions = _sort_exceptions_by_specificity( + list(_exception_handlers.keys()) + ) + + def _exception_handler( + exception: BaseException, + request: web.Request, + ) -> web.HTTPError | BaseException | None: + if exc_cls := next( + (_ for _ in _catch_exceptions if isinstance(exception, _)), None + ): + return _exception_handlers[exc_cls](request=request, exception=exception) + # NOTE: not in my list, return so it gets reraised + return exception + + return _exception_handler diff --git a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py index c611809decdc..90fcb50b7dd9 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py @@ -5,7 +5,8 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_exception_handlers_decorator, + create_decorator_from_exception_handler, + create_exception_handler_from_http_error_map, ) from ..projects.exceptions import ( BaseProjectError, @@ -69,7 +70,7 @@ } -handle_plugin_requests_exceptions = create_exception_handlers_decorator( - exceptions_catch=(BaseProjectError, FoldersValueError, WorkspacesValueError), - exc_to_status_map=_TO_HTTP_ERROR_MAP, +handle_plugin_requests_exceptions = create_decorator_from_exception_handler( + exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), + exception_types=(BaseProjectError, FoldersValueError, WorkspacesValueError), ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py index 4593779e7357..2074b6d36236 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py @@ -12,7 +12,8 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_exception_handlers_decorator, + create_decorator_from_exception_handler, + create_exception_handler_from_http_error_map, ) from ..login.decorators import get_user_id, login_required from ..products.api import get_product_name @@ -45,10 +46,12 @@ } -_handle_exceptions = create_exception_handlers_decorator( - exceptions_catch=ProjectTrashError, exc_to_status_map=_TO_HTTP_ERROR_MAP +_handle_exceptions = create_decorator_from_exception_handler( + exception_types=ProjectTrashError, + exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), ) + # # ROUTES # diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index 86b2cf4e0ea1..27fdf736cb23 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -13,21 +13,26 @@ from aiohttp.test_utils import make_mocked_request from servicelib.aiohttp import status from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exception_handlers import ( +from simcore_service_webserver.exceptions_handlers import ( + HttpErrorInfo, + create_decorator_from_exception_handler, +) +from simcore_service_webserver.exceptions_handlers_base import ( + _handled_exception_context_manager, +) +from simcore_service_webserver.exceptions_handlers_base_2 import ( add_exception_handler, add_exception_mapper, handle_registered_exceptions, setup_exception_handlers, ) -from simcore_service_webserver.exceptions_handlers import ( - HttpErrorInfo, - _sort_exceptions_by_specificity, - _handled_exception_context, +from simcore_service_webserver.exceptions_handlers_http_error_map import ( _sort_exceptions_by_specificity, - create__http_error_map_handler, - create_exception_handlers_decorator, + create_exception_handler_from_http_error_map, ) +# Some custom errors in my service + class BasePluginError(WebServerBaseError): ... @@ -79,9 +84,10 @@ def fake_request() -> web.Request: return make_mocked_request("GET", "/foo") -def test_http_error_map_handler_factory(fake_request: web.Request): +def test_create_exception_handler_from_http_error_map(fake_request: web.Request): - exc_handler = create__http_error_map_handler( + # call exception handler factory + exc_handler = create_exception_handler_from_http_error_map( { OneError: HttpErrorInfo( status.HTTP_400_BAD_REQUEST, "Error One mapped to 400" @@ -100,7 +106,7 @@ def test_http_error_map_handler_factory(fake_request: web.Request): assert exc_handler(err, fake_request) is err -def test_handled_exception_context(fake_request: web.Request): +def test__handled_exception_context_manager(fake_request: web.Request): def _suppress_handler(exception, request): assert request == fake_request assert isinstance( @@ -109,7 +115,7 @@ def _suppress_handler(exception, request): return None # noqa: RET501, PLR1711 def _fun(raises): - with _handled_exception_context( + with _handled_exception_context_manager( BasePluginError, _suppress_handler, request=fake_request ): raise raises @@ -122,19 +128,22 @@ def _fun(raises): _fun(raises=ArithmeticError) - -async def test_exception_handlers_decorator( +async def test_create_decorator_from_exception_handler( caplog: pytest.LogCaptureFixture, ): - _handle_exceptions = create_exception_handlers_decorator( - exceptions_catch=BasePluginError, - exc_to_status_map={ + exc_handler = create_exception_handler_from_http_error_map( + { OneError: HttpErrorInfo( - status_code=status.HTTP_503_SERVICE_UNAVAILABLE, - msg_template="This is one error for front-end", + status.HTTP_503_SERVICE_UNAVAILABLE, + "Human readable error transmitted to the front-end", ) - }, + } + ) + + _handle_exceptions = create_decorator_from_exception_handler( + exception_handler=exc_handler, + exception_types=BasePluginError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! ) @_handle_exceptions From 2edc790bc2d4607f8c7046370b9ec57e6754df9b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:35:16 +0100 Subject: [PATCH 16/59] WIP: base2 --- .../exceptions_handlers_base_2.py | 42 ++++++++++++------- .../unit/isolated/test_exceptions_handlers.py | 23 +++++----- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py index 7468fec8fc80..30d6451e398b 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py @@ -1,13 +1,10 @@ import functools -import logging from collections.abc import Awaitable, Callable, MutableMapping from http import HTTPStatus from typing import Any, TypeAlias, cast from aiohttp import web - -_logger = logging.getLogger(__name__) - +from servicelib.aiohttp.typing_extension import Handler # Defines exception handler as somethign that returns responses, as fastapi, and not new exceptions! # in reality this can be reinterpreted in aiohttp since all responses can be represented as exceptions. @@ -28,25 +25,31 @@ # as decorator or context manager? -def setup_exception_handlers(scope: MutableMapping[str, Any]): - scope["exceptions_handlers"] = {} - scope["exceptions_map"] = {} +_EXCEPTIONS_HANDLERS_KEY = f"{__name__}._EXCEPTIONS_HANDLERS_KEY" +_EXCEPTIONS_MAP_KEY = f"{__name__}._EXCEPTIONS_MAP_KEY" + + +def setup_exception_handlers(scope: MutableMapping): + # init registry in the scope + scope[_EXCEPTIONS_HANDLERS_KEY] = {} + scope[_EXCEPTIONS_MAP_KEY] = {} # but this is very specific because it responds with only status! you migh want to have different # type of bodies, etc -def _get_exception_handler_registry( - scope: MutableMapping[str, Any] -) -> ExceptionHandlerRegistry: - return scope.get("exceptions_handlers", {}) +def _get_exception_handler_registry(scope: MutableMapping) -> ExceptionHandlerRegistry: + return scope.get(_EXCEPTIONS_HANDLERS_KEY, {}) def add_exception_handler( - scope: MutableMapping[str, Any], + scope: MutableMapping, exc_class: type[Exception], handler: ExceptionHandler, ): - scope["exceptions_handlers"][exc_class] = handler + """ + Registers in the scope an exception type to a handler + """ + scope[_EXCEPTIONS_HANDLERS_KEY][exc_class] = handler def _create_exception_handler_mapper( @@ -67,8 +70,13 @@ def add_exception_mapper( exc_class: type[Exception], http_exc_class: type[web.HTTPException], ): + """ + Create an exception handlers by mapping a class to an HTTPException + and registers it in the scope + """ + # adds exception handler to scope - scope["exceptions_map"][exc_class] = http_exc_class + scope[_EXCEPTIONS_MAP_KEY][exc_class] = http_exc_class add_exception_handler( scope, exc_class, @@ -77,6 +85,7 @@ def add_exception_mapper( async def handle_request_with_exception_handling_in_scope( + # Create using contextlib.contextmanager handler: Handler, request: web.Request, scope: MutableMapping[str, Any] | None = None, @@ -100,6 +109,7 @@ async def handle_request_with_exception_handling_in_scope( return resp +# decorator def handle_registered_exceptions(scope: MutableMapping[str, Any] | None = None): def _decorator(handler: Handler): @functools.wraps(handler) @@ -113,9 +123,9 @@ async def _wrapper(request: web.Request) -> web.Response: return _decorator -# If I have all the status codes mapped, I can definitively use that info to create `responses` -# for fastapi to render the OAS preoperly def openapi_error_responses( + # If I have all the status codes mapped, I can definitively use that info to create `responses` + # for fastapi to render the OAS preoperly exceptions_map: ExceptionsMap, ) -> dict[HTTPStatus, dict[str, Any]]: responses = {} diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index 27fdf736cb23..24879a050658 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -24,7 +24,7 @@ add_exception_handler, add_exception_mapper, handle_registered_exceptions, - setup_exception_handlers, + register_exception_handlers, ) from simcore_service_webserver.exceptions_handlers_http_error_map import ( _sort_exceptions_by_specificity, @@ -190,24 +190,23 @@ async def _rest_handler(request: web.Request) -> web.Response: def test_it(): - class MyException(Exception): - ... - - class OtherException(Exception): - ... app = web.Application() - async def my_error_handler(request: web.Request, exc: MyException): + async def my_error_handler(request: web.Request, exc: OneError): return web.HTTPNotFound() - # define at the app level - setup_exception_handlers(app) - add_exception_handler(app, MyException, my_error_handler) - add_exception_mapper(app, OtherException, web.HTTPNotFound) + # create register + register_exception_handlers(app) + + # register + add_exception_handler(app, OneError, my_error_handler) + + # this is a handler create mapping to status_code and reason + add_exception_mapper(app, OtherError, web.HTTPNotFound) async def foo(): - raise MyException + raise OneError routes = web.RouteTableDef() From 35eef018af3d2d78825d974cbff658b6bcf968a1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:54:18 +0100 Subject: [PATCH 17/59] cleanup --- .../src/simcore_service_webserver/exceptions_handlers_base_2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py index 30d6451e398b..60af8dd0d87d 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py @@ -66,7 +66,7 @@ async def _exception_handler(_: web.Request, exc: Exception) -> web.Response: def add_exception_mapper( - scope: MutableMapping[str, Any], + scope: MutableMapping, exc_class: type[Exception], http_exc_class: type[web.HTTPException], ): From 4592a706344b80d337a76d3197a660758f15d552 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 22 Nov 2024 17:29:16 +0100 Subject: [PATCH 18/59] cleanup --- .../exceptions_handlers_base_2.py | 23 ++++--- .../unit/isolated/test_exceptions_handlers.py | 34 ---------- .../test_exceptions_handlers_base_2.py | 65 +++++++++++++++++++ 3 files changed, 78 insertions(+), 44 deletions(-) create mode 100644 services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py index 60af8dd0d87d..7aa36be55e9b 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py @@ -29,16 +29,18 @@ _EXCEPTIONS_MAP_KEY = f"{__name__}._EXCEPTIONS_MAP_KEY" -def setup_exception_handlers(scope: MutableMapping): +def setup_exception_handlers(registry: MutableMapping): # init registry in the scope - scope[_EXCEPTIONS_HANDLERS_KEY] = {} - scope[_EXCEPTIONS_MAP_KEY] = {} + registry[_EXCEPTIONS_HANDLERS_KEY] = {} + registry[_EXCEPTIONS_MAP_KEY] = {} # but this is very specific because it responds with only status! you migh want to have different # type of bodies, etc -def _get_exception_handler_registry(scope: MutableMapping) -> ExceptionHandlerRegistry: - return scope.get(_EXCEPTIONS_HANDLERS_KEY, {}) +def _get_exception_handler_registry( + registry: MutableMapping, +) -> ExceptionHandlerRegistry: + return registry.get(_EXCEPTIONS_HANDLERS_KEY, {}) def add_exception_handler( @@ -66,7 +68,7 @@ async def _exception_handler(_: web.Request, exc: Exception) -> web.Response: def add_exception_mapper( - scope: MutableMapping, + registry: MutableMapping, exc_class: type[Exception], http_exc_class: type[web.HTTPException], ): @@ -76,9 +78,9 @@ def add_exception_mapper( """ # adds exception handler to scope - scope[_EXCEPTIONS_MAP_KEY][exc_class] = http_exc_class + registry[_EXCEPTIONS_MAP_KEY][exc_class] = http_exc_class add_exception_handler( - scope, + registry, exc_class, handler=_create_exception_handler_mapper(exc_class, http_exc_class), ) @@ -86,6 +88,7 @@ def add_exception_mapper( async def handle_request_with_exception_handling_in_scope( # Create using contextlib.contextmanager + # FIXME: !!! handler: Handler, request: web.Request, scope: MutableMapping[str, Any] | None = None, @@ -110,12 +113,12 @@ async def handle_request_with_exception_handling_in_scope( # decorator -def handle_registered_exceptions(scope: MutableMapping[str, Any] | None = None): +def handle_registered_exceptions(registry: MutableMapping[str, Any] | None = None): def _decorator(handler: Handler): @functools.wraps(handler) async def _wrapper(request: web.Request) -> web.Response: return await handle_request_with_exception_handling_in_scope( - handler, request, scope + handler, request, registry ) return _wrapper diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index 24879a050658..c337583b56d3 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -20,12 +20,6 @@ from simcore_service_webserver.exceptions_handlers_base import ( _handled_exception_context_manager, ) -from simcore_service_webserver.exceptions_handlers_base_2 import ( - add_exception_handler, - add_exception_mapper, - handle_registered_exceptions, - register_exception_handlers, -) from simcore_service_webserver.exceptions_handlers_http_error_map import ( _sort_exceptions_by_specificity, create_exception_handler_from_http_error_map, @@ -187,31 +181,3 @@ async def _rest_handler(request: web.Request) -> web.Response: resp = await _rest_handler( make_mocked_request("GET", "/foo?raise=ArithmeticError") ) - - -def test_it(): - - app = web.Application() - - async def my_error_handler(request: web.Request, exc: OneError): - return web.HTTPNotFound() - - # create register - register_exception_handlers(app) - - # register - add_exception_handler(app, OneError, my_error_handler) - - # this is a handler create mapping to status_code and reason - add_exception_mapper(app, OtherError, web.HTTPNotFound) - - async def foo(): - raise OneError - - routes = web.RouteTableDef() - - @routes.get("/home") - @handle_registered_exceptions() - async def home(_request: web.Request): - await foo() - return web.HTTPOk() diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py new file mode 100644 index 000000000000..8512950fd993 --- /dev/null +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py @@ -0,0 +1,65 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +import pytest +from aiohttp import web +from aiohttp.test_utils import make_mocked_request +from simcore_service_webserver.errors import WebServerBaseError +from simcore_service_webserver.exceptions_handlers_base_2 import ( + add_exception_handler, + add_exception_mapper, + handle_registered_exceptions, + setup_exception_handlers, +) + +# Some custom errors in my service + + +class BasePluginError(WebServerBaseError): + ... + + +class OneError(BasePluginError): + ... + + +class OtherError(BasePluginError): + ... + + +@pytest.fixture +def fake_request() -> web.Request: + return make_mocked_request("GET", "/foo") + + +def test_it(): + + app = web.Application() + + async def my_error_handler(request: web.Request, exc: OneError): + return web.HTTPNotFound() + + # create register + setup_exception_handlers(app) + + # register + add_exception_handler(app, OneError, my_error_handler) + + # this is a handler create mapping to status_code and reason + add_exception_mapper(app, OtherError, web.HTTPNotFound) + + async def foo(): + raise OneError + + routes = web.RouteTableDef() + + @routes.get("/home") + @handle_registered_exceptions() + async def home(_request: web.Request): + await foo() + return web.HTTPOk() From aa0368669eff2e6525ee8cc44babe1d97b2ef8ed Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 11:48:13 +0100 Subject: [PATCH 19/59] cleanup --- .../exceptions_handlers.py | 4 +- .../exceptions_handlers_base_2.py | 61 ++++++++++++------- .../folders/_exceptions_handlers.py | 2 +- .../unit/isolated/test_exceptions_handlers.py | 2 +- .../test_exceptions_handlers_base_2.py | 23 +++++-- 5 files changed, 62 insertions(+), 30 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 36f818000a1f..5072dbca5dd3 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -6,10 +6,10 @@ ) __all__: tuple[str, ...] = ( - "create_decorator_from_exception_handler", - "create_exception_handler_from_http_error_map", "ExceptionToHttpErrorMap", "HttpErrorInfo", + "create_decorator_from_exception_handler", + "create_exception_handler_from_http_error_map", ) # nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py index 7aa36be55e9b..466d34968359 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py @@ -1,10 +1,11 @@ import functools -from collections.abc import Awaitable, Callable, MutableMapping +from collections.abc import MutableMapping from http import HTTPStatus -from typing import Any, TypeAlias, cast +from typing import Any, Protocol, TypeAlias, cast from aiohttp import web from servicelib.aiohttp.typing_extension import Handler +from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions # Defines exception handler as somethign that returns responses, as fastapi, and not new exceptions! # in reality this can be reinterpreted in aiohttp since all responses can be represented as exceptions. @@ -12,11 +13,17 @@ # need return None or the exception itself which as we saw in the tests, it causes troubles! # -ExceptionHandler: TypeAlias = Callable[ - [web.Request, Exception], Awaitable[web.Response] -] -ExceptionsMap: TypeAlias = dict[type[Exception], type[web.HTTPException]] +class ExceptionHandler(Protocol): + # Based on concept in https://www.starlette.io/exceptions/ + __name__: str + + # TODO: do not know how to define exc so that it accepts any exception! + async def __call__(self, request: web.Request, exc: BaseException) -> web.Response: + ... + + +ErrorsToHttpExceptionsMap: TypeAlias = dict[type[Exception], type[web.HTTPException]] ExceptionHandlerRegistry: TypeAlias = dict[type[Exception], ExceptionHandler] @@ -29,7 +36,7 @@ _EXCEPTIONS_MAP_KEY = f"{__name__}._EXCEPTIONS_MAP_KEY" -def setup_exception_handlers(registry: MutableMapping): +def setup_exceptions_handlers(registry: MutableMapping): # init registry in the scope registry[_EXCEPTIONS_HANDLERS_KEY] = {} registry[_EXCEPTIONS_MAP_KEY] = {} @@ -37,21 +44,26 @@ def setup_exception_handlers(registry: MutableMapping): # type of bodies, etc -def _get_exception_handler_registry( +def _get_exception_handlers( registry: MutableMapping, ) -> ExceptionHandlerRegistry: return registry.get(_EXCEPTIONS_HANDLERS_KEY, {}) def add_exception_handler( - scope: MutableMapping, + registry: MutableMapping, exc_class: type[Exception], handler: ExceptionHandler, ): """ Registers in the scope an exception type to a handler """ - scope[_EXCEPTIONS_HANDLERS_KEY][exc_class] = handler + registry[_EXCEPTIONS_HANDLERS_KEY][exc_class] = handler + + +_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[ + int, type[web.HTTPException] +] = get_all_aiohttp_http_exceptions(web.HTTPException) def _create_exception_handler_mapper( @@ -60,32 +72,41 @@ def _create_exception_handler_mapper( ) -> ExceptionHandler: error_code = f"{exc_class.__name__}" # status_code.error_code - async def _exception_handler(_: web.Request, exc: Exception) -> web.Response: + async def _exception_handler( + request: web.Request, exc: BaseException + ) -> web.Response: # TODO: a better way to add error_code. TODO: create the envelope! + assert request # nosec return http_exc_class(reason=f"{exc} [{error_code}]") return _exception_handler def add_exception_mapper( - registry: MutableMapping, - exc_class: type[Exception], - http_exc_class: type[web.HTTPException], + registry: MutableMapping, exception_class: type[Exception], status_code: int ): """ Create an exception handlers by mapping a class to an HTTPException and registers it in the scope """ + try: + http_exception_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code] + except KeyError as err: + msg = f"Invalid status code. Got {status_code=}" + raise ValueError(msg) from err # adds exception handler to scope - registry[_EXCEPTIONS_MAP_KEY][exc_class] = http_exc_class + registry[_EXCEPTIONS_MAP_KEY][exception_class] = http_exception_cls add_exception_handler( registry, - exc_class, - handler=_create_exception_handler_mapper(exc_class, http_exc_class), + exception_class, + handler=_create_exception_handler_mapper(exception_class, http_exception_cls), ) +# ---------- + + async def handle_request_with_exception_handling_in_scope( # Create using contextlib.contextmanager # FIXME: !!! @@ -99,9 +120,7 @@ async def handle_request_with_exception_handling_in_scope( except Exception as exc: # pylint: disable=broad-exception-caught scope = scope or request.app - if exception_handler := _get_exception_handler_registry(scope).get( - type(exc), None - ): + if exception_handler := _get_exception_handlers(scope).get(type(exc), None): resp = await exception_handler(request, exc) else: resp = web.HTTPInternalServerError() @@ -129,7 +148,7 @@ async def _wrapper(request: web.Request) -> web.Response: def openapi_error_responses( # If I have all the status codes mapped, I can definitively use that info to create `responses` # for fastapi to render the OAS preoperly - exceptions_map: ExceptionsMap, + exceptions_map: ErrorsToHttpExceptionsMap, ) -> dict[HTTPStatus, dict[str, Any]]: responses = {} diff --git a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py index 90fcb50b7dd9..00e80f7ff8bc 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py @@ -71,6 +71,6 @@ handle_plugin_requests_exceptions = create_decorator_from_exception_handler( - exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), exception_types=(BaseProjectError, FoldersValueError, WorkspacesValueError), + exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), ) diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index c337583b56d3..4a7c03352369 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -136,8 +136,8 @@ async def test_create_decorator_from_exception_handler( ) _handle_exceptions = create_decorator_from_exception_handler( - exception_handler=exc_handler, exception_types=BasePluginError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! + exception_handler=exc_handler, ) @_handle_exceptions diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py index 8512950fd993..71a2cfcc04b5 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py @@ -9,12 +9,13 @@ import pytest from aiohttp import web from aiohttp.test_utils import make_mocked_request +from servicelib.aiohttp import status from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers_base_2 import ( add_exception_handler, add_exception_mapper, handle_registered_exceptions, - setup_exception_handlers, + setup_exceptions_handlers, ) # Some custom errors in my service @@ -41,18 +42,30 @@ def test_it(): app = web.Application() - async def my_error_handler(request: web.Request, exc: OneError): + async def my_error_handler(request: web.Request, exc: BaseException): + assert isinstance(exc, OneError) return web.HTTPNotFound() # create register - setup_exception_handlers(app) + setup_exceptions_handlers(app) - # register + # register exception handler add_exception_handler(app, OneError, my_error_handler) - # this is a handler create mapping to status_code and reason + # mapper is defined as a higher abstraction + # that automatically produces an exception handler + + # map two exceptions? add_exception_mapper(app, OtherError, web.HTTPNotFound) + # this is a handler create mapping to status_code and reason + add_exception_mapper(app, OtherError, status.HTTP_404_NOT_FOUND) + + # NOTE: that we respond always the same way to errors, i.e. using Error model + + # shouldn't this be automatic? + add_exception_mapper(app, web.HTTPNotFound, status.HTTP_404_NOT_FOUND) + async def foo(): raise OneError From 32b97690417a7b71f37a2d04b054375d0643c6cc Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 13:22:32 +0100 Subject: [PATCH 20/59] exceptions handlers async --- .../exceptions_handlers_base.py | 21 ++++++---- .../exceptions_handlers_http_error_map.py | 20 +++++----- .../folders/_exceptions_handlers.py | 1 + .../unit/isolated/test_exceptions_handlers.py | 39 ++++++++++++++----- 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index 3418f46f4ea8..9aaae4d08d3b 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -1,6 +1,6 @@ import functools import logging -from contextlib import contextmanager +from contextlib import asynccontextmanager from typing import Protocol from aiohttp import web @@ -16,8 +16,10 @@ class WebApiExceptionHandler(Protocol): __name__: str - def __call__( - self, exception: BaseException, request: web.Request + async def __call__( + self, + request: web.Request, + exception: BaseException, ) -> web.HTTPException | BaseException | None: """ Callback to process an exception raised during a web request, allowing custom handling. @@ -26,8 +28,8 @@ def __call__( into an `web.HTTPException` (i.e. exceptions defined at the web-api) Arguments: - exception -- exception raised in web handler during this request request -- current request + exception -- exception raised in web handler during this request Returns: - None: to suppress `exception` @@ -36,17 +38,20 @@ def __call__( """ -@contextmanager -def _handled_exception_context_manager( +@asynccontextmanager +async def _handled_exception_context_manager( exception_catch: type[BaseException] | tuple[type[BaseException], ...], exception_handler: WebApiExceptionHandler, **forward_ctx, ): """Calls `exception_handler` on exceptions raised in this context and caught in `exception_catch`""" try: + yield + except exception_catch as e: - if exc := exception_handler(e, **forward_ctx): + exc = await exception_handler(exception=e, **forward_ctx) + if exc: assert isinstance(exc, BaseException) raise exc from e @@ -68,7 +73,7 @@ def _decorator(handler: WebHandler): @functools.wraps(handler) async def _wrapper(request: web.Request) -> web.StreamResponse: - with _handled_exception_context_manager( + async with _handled_exception_context_manager( exception_types, exception_handler, request=request, diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index bccb887f5377..43545deb7aa5 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -36,7 +36,7 @@ def create_exception_handler_from_http_error( exception_cls: type[BaseException], *, status_code: int, - user_msg_template: str, + msg_template: str, ) -> WebApiExceptionHandler: """ Custom Exception-Handler factory @@ -51,20 +51,20 @@ def create_exception_handler_from_http_error( Arguments: exception_cls: exception raise during the request status_code: the http status code to associate at the web-api interface to this error - user_msg_template: a template string to pass to the HttpError + msg_template: a template string to pass to the HttpError Returns: A web api exception handler """ - def _exception_handler( - exception: BaseException, + async def _exception_handler( request: web.Request, + exception: BaseException, ) -> web.HTTPException | BaseException | None: assert isinstance(exception, exception_cls) # nosec # safe formatting, i.e. does not raise - user_msg = user_msg_template.format_map( + user_msg = msg_template.format_map( _DefaultDict(getattr(exception, "__dict__", {})) ) @@ -114,7 +114,7 @@ def create_exception_handler_from_http_error_map( exc_cls: create_exception_handler_from_http_error( exception_cls=exc_cls, status_code=http_error_info.status_code, - user_msg_template=http_error_info.msg_template, + msg_template=http_error_info.msg_template, ) for exc_cls, http_error_info in exc_to_http_error_map.items() } @@ -123,14 +123,16 @@ def create_exception_handler_from_http_error_map( list(_exception_handlers.keys()) ) - def _exception_handler( - exception: BaseException, + async def _exception_handler( request: web.Request, + exception: BaseException, ) -> web.HTTPError | BaseException | None: if exc_cls := next( (_ for _ in _catch_exceptions if isinstance(exception, _)), None ): - return _exception_handlers[exc_cls](request=request, exception=exception) + return await _exception_handlers[exc_cls]( + request=request, exception=exception + ) # NOTE: not in my list, return so it gets reraised return exception diff --git a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py index 00e80f7ff8bc..adc8fb4e7429 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py @@ -74,3 +74,4 @@ exception_types=(BaseProjectError, FoldersValueError, WorkspacesValueError), exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), ) +# this is one decorator with a single exception handler diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index 4a7c03352369..fd74a3c6a0c4 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -22,6 +22,7 @@ ) from simcore_service_webserver.exceptions_handlers_http_error_map import ( _sort_exceptions_by_specificity, + create_exception_handler_from_http_error, create_exception_handler_from_http_error_map, ) @@ -78,7 +79,25 @@ def fake_request() -> web.Request: return make_mocked_request("GET", "/foo") -def test_create_exception_handler_from_http_error_map(fake_request: web.Request): +async def test_factory__create_exception_handler_from_http_error( + fake_request: web.Request, +): + + one_error_to_404 = create_exception_handler_from_http_error( + OneError, + status_code=status.HTTP_404_NOT_FOUND, + msg_template="one error message for the user: {code} {value}", + ) + + # calling exception handler + caught = OneError() + response = await one_error_to_404(fake_request, caught) + assert response + assert response.status == status.HTTP_404_NOT_FOUND + assert "one error" in response.text + + +async def test_create_exception_handler_from_http_error_map(fake_request: web.Request): # call exception handler factory exc_handler = create_exception_handler_from_http_error_map( @@ -90,36 +109,36 @@ def test_create_exception_handler_from_http_error_map(fake_request: web.Request) ) # Converts exception in map - got_exc = exc_handler(OneError(), fake_request) + got_exc = await exc_handler(fake_request, OneError()) assert isinstance(got_exc, web.HTTPBadRequest) assert got_exc.reason == "Error One mapped to 400" # By-passes exceptions not listed err = RuntimeError() - assert exc_handler(err, fake_request) is err + assert await exc_handler(fake_request, err) is err -def test__handled_exception_context_manager(fake_request: web.Request): - def _suppress_handler(exception, request): +async def test__handled_exception_context_manager(fake_request: web.Request): + async def _suppress_handler(request, exception): assert request == fake_request assert isinstance( exception, BasePluginError ), "only BasePluginError exceptions should call this handler" return None # noqa: RET501, PLR1711 - def _fun(raises): - with _handled_exception_context_manager( + async def _fun(raises): + async with _handled_exception_context_manager( BasePluginError, _suppress_handler, request=fake_request ): raise raises # checks - _fun(raises=OneError) - _fun(raises=OtherError) + await _fun(raises=OneError) + await _fun(raises=OtherError) with pytest.raises(ArithmeticError): - _fun(raises=ArithmeticError) + await _fun(raises=ArithmeticError) async def test_create_decorator_from_exception_handler( From 605c41ac7df8a83fb8813c508e5d344876991f07 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:15:45 +0100 Subject: [PATCH 21/59] handler return json.response --- .../exceptions_handlers_base.py | 51 ++++++----- .../exceptions_handlers_http_error_map.py | 29 +++--- .../unit/isolated/test_exceptions_handlers.py | 88 +++++++++++-------- 3 files changed, 95 insertions(+), 73 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index 9aaae4d08d3b..3d306f3edcfa 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -1,7 +1,8 @@ import functools import logging from contextlib import asynccontextmanager -from typing import Protocol +from dataclasses import dataclass +from typing import AsyncIterator, Protocol from aiohttp import web from servicelib.aiohttp.typing_extension import Handler as WebHandler @@ -13,55 +14,54 @@ # -class WebApiExceptionHandler(Protocol): +class AiohttpExceptionHandler(Protocol): __name__: str async def __call__( self, request: web.Request, exception: BaseException, - ) -> web.HTTPException | BaseException | None: + ) -> web.Response: """ Callback to process an exception raised during a web request, allowing custom handling. - This function can be implemented to suppress, reraise, or transform the exception - into an `web.HTTPException` (i.e. exceptions defined at the web-api) + This function can be implemented to suppress, (transform &) reraise as web.HTTPError + or return a web.Response Arguments: request -- current request exception -- exception raised in web handler during this request - - Returns: - - None: to suppress `exception` - - `exception`: to reraise it - - an instance of `web.HTTPException` to transform to HTTP Api exceptions (NOTE: that they can either be errors or success!) """ + ... # pylint: disable=unnecessary-ellipsis + + +@dataclass +class _ExceptionContext: + response: web.Response | None = None @asynccontextmanager async def _handled_exception_context_manager( exception_catch: type[BaseException] | tuple[type[BaseException], ...], - exception_handler: WebApiExceptionHandler, + exception_handler: AiohttpExceptionHandler, **forward_ctx, -): - """Calls `exception_handler` on exceptions raised in this context and caught in `exception_catch`""" +) -> AsyncIterator[_ExceptionContext]: + """Calls `exception_handler` on exceptions raised in this context + and caught in `exception_catch` + """ + ctx = _ExceptionContext() try: - yield + yield ctx except exception_catch as e: - exc = await exception_handler(exception=e, **forward_ctx) - if exc: - assert isinstance(exc, BaseException) - raise exc from e - - _logger.debug( - "%s suppressed %s: %s", exception_handler.__name__, type(e).__name__, f"{e}" - ) + response = await exception_handler(exception=e, **forward_ctx) + assert isinstance(response, web.Response) # nosec + ctx.response = response def create_decorator_from_exception_handler( - exception_handler: WebApiExceptionHandler, + exception_handler: AiohttpExceptionHandler, exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception, ): """Returns a decorator for aiohttp's web.Handler functions @@ -77,9 +77,12 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: exception_types, exception_handler, request=request, - ): + ) as exc_ctx: + return await handler(request) + return exc_ctx.response + return _wrapper return _decorator diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index 43545deb7aa5..bdd6601ddb45 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -7,14 +7,14 @@ from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error -from .exceptions_handlers_base import WebApiExceptionHandler +from .exceptions_handlers_base import AiohttpExceptionHandler _logger = logging.getLogger(__name__) -_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[ +_STATUS_CODE_TO_HTTP_ERRORS: dict[ int, type[web.HTTPException] -] = get_all_aiohttp_http_exceptions(web.HTTPException) +] = get_all_aiohttp_http_exceptions(web.HTTPError) class _DefaultDict(dict): @@ -37,7 +37,7 @@ def create_exception_handler_from_http_error( *, status_code: int, msg_template: str, -) -> WebApiExceptionHandler: +) -> AiohttpExceptionHandler: """ Custom Exception-Handler factory @@ -60,7 +60,7 @@ def create_exception_handler_from_http_error( async def _exception_handler( request: web.Request, exception: BaseException, - ) -> web.HTTPException | BaseException | None: + ) -> web.Response: assert isinstance(exception, exception_cls) # nosec # safe formatting, i.e. does not raise @@ -68,9 +68,6 @@ async def _exception_handler( _DefaultDict(getattr(exception, "__dict__", {})) ) - http_error_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code] - assert http_error_cls # nosec - if is_5xx_server_error(status_code): _logger.exception( **create_troubleshotting_log_kwargs( @@ -84,7 +81,17 @@ async def _exception_handler( }, ) ) - return http_error_cls(reason=user_msg) + + # TODO: make this part customizable? e.g. so we can inject e.g. oec? + # TODO: connect with ErrorModel + return web.json_response( + { + "error": { + "msg": user_msg, + } + }, + status=status_code, + ) return _exception_handler @@ -101,7 +108,7 @@ def _sort_exceptions_by_specificity( def create_exception_handler_from_http_error_map( exc_to_http_error_map: ExceptionToHttpErrorMap, -) -> WebApiExceptionHandler: +) -> AiohttpExceptionHandler: """ Custom Exception-Handler factory @@ -126,7 +133,7 @@ def create_exception_handler_from_http_error_map( async def _exception_handler( request: web.Request, exception: BaseException, - ) -> web.HTTPError | BaseException | None: + ) -> web.Response: if exc_cls := next( (_ for _ in _catch_exceptions if isinstance(exception, _)), None ): diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index fd74a3c6a0c4..e9b49eaa80d2 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -14,6 +14,7 @@ from servicelib.aiohttp import status from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers import ( + ExceptionToHttpErrorMap, HttpErrorInfo, create_decorator_from_exception_handler, ) @@ -29,29 +30,29 @@ # Some custom errors in my service -class BasePluginError(WebServerBaseError): +class BaseError(WebServerBaseError): ... -class OneError(BasePluginError): +class OneError(BaseError): ... -class OtherError(BasePluginError): +class OtherError(BaseError): ... def test_sort_concrete_first(): - assert _sort_exceptions_by_specificity([Exception, BasePluginError]) == [ - BasePluginError, + assert _sort_exceptions_by_specificity([Exception, BaseError]) == [ + BaseError, Exception, ] assert _sort_exceptions_by_specificity( - [Exception, BasePluginError], concrete_first=False + [Exception, BaseError], concrete_first=False ) == [ Exception, - BasePluginError, + BaseError, ] @@ -62,7 +63,7 @@ def test_sort_exceptions_by_specificity(): Exception, OtherError, OneError, - BasePluginError, + BaseError, ValueError, ArithmeticError, ZeroDivisionError, @@ -82,7 +83,6 @@ def fake_request() -> web.Request: async def test_factory__create_exception_handler_from_http_error( fake_request: web.Request, ): - one_error_to_404 = create_exception_handler_from_http_error( OneError, status_code=status.HTTP_404_NOT_FOUND, @@ -92,12 +92,14 @@ async def test_factory__create_exception_handler_from_http_error( # calling exception handler caught = OneError() response = await one_error_to_404(fake_request, caught) - assert response assert response.status == status.HTTP_404_NOT_FOUND - assert "one error" in response.text + assert response.text is not None + assert "one error message" in response.text -async def test_create_exception_handler_from_http_error_map(fake_request: web.Request): +async def test_factory__create_exception_handler_from_http_error_map( + fake_request: web.Request, +): # call exception handler factory exc_handler = create_exception_handler_from_http_error_map( @@ -109,8 +111,10 @@ async def test_create_exception_handler_from_http_error_map(fake_request: web.Re ) # Converts exception in map - got_exc = await exc_handler(fake_request, OneError()) + with pytest.raises(web.HTTPBadRequest) as exc_info: + await exc_handler(fake_request, OneError()) + got_exc = exc_info.value assert isinstance(got_exc, web.HTTPBadRequest) assert got_exc.reason == "Error One mapped to 400" @@ -120,46 +124,53 @@ async def test_create_exception_handler_from_http_error_map(fake_request: web.Re async def test__handled_exception_context_manager(fake_request: web.Request): - async def _suppress_handler(request, exception): + + expected_response = web.json_response({"error": {"msg": "Foo"}}) + + async def _custom_handler(request, exception): assert request == fake_request assert isinstance( - exception, BasePluginError + exception, BaseError ), "only BasePluginError exceptions should call this handler" - return None # noqa: RET501, PLR1711 + return expected_response - async def _fun(raises): - async with _handled_exception_context_manager( - BasePluginError, _suppress_handler, request=fake_request - ): - raise raises + exc_handling_ctx = _handled_exception_context_manager( + BaseError, _custom_handler, request=fake_request + ) + + # handles any BaseError returning a response + async with exc_handling_ctx as ctx: + raise OneError + assert ctx.response == expected_response - # checks - await _fun(raises=OneError) - await _fun(raises=OtherError) + async with exc_handling_ctx as ctx: + raise OtherError + assert ctx.response == expected_response + # otherwise thru with pytest.raises(ArithmeticError): - await _fun(raises=ArithmeticError) + async with exc_handling_ctx: + raise ArithmeticError async def test_create_decorator_from_exception_handler( caplog: pytest.LogCaptureFixture, ): + # Create an SINGLE exception handler that acts as umbrella for all these exceptions + http_error_map: ExceptionToHttpErrorMap = { + OneError: HttpErrorInfo( + status.HTTP_503_SERVICE_UNAVAILABLE, + "Human readable error transmitted to the front-end", + ) + } - exc_handler = create_exception_handler_from_http_error_map( - { - OneError: HttpErrorInfo( - status.HTTP_503_SERVICE_UNAVAILABLE, - "Human readable error transmitted to the front-end", - ) - } - ) - - _handle_exceptions = create_decorator_from_exception_handler( - exception_types=BasePluginError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! + exc_handler = create_exception_handler_from_http_error_map(http_error_map) + _exc_handling_ctx = create_decorator_from_exception_handler( + exception_types=BaseError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! exception_handler=exc_handler, ) - @_handle_exceptions + @_exc_handling_ctx async def _rest_handler(request: web.Request) -> web.Response: if request.query.get("raise") == "OneError": raise OneError @@ -193,8 +204,9 @@ async def _rest_handler(request: web.Request) -> web.Response: assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE assert "front-end" in resp.reason - assert caplog.records + assert caplog.records, "Expected 5XX troubleshooting logged as error" assert caplog.records[0].levelno == logging.ERROR + # typically capture by last with pytest.raises(ArithmeticError): resp = await _rest_handler( From 5b6254118885a345e5f6444a9b95fc05b0f15179 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 21:36:50 +0100 Subject: [PATCH 22/59] json-error --- .../exceptions_handlers_http_error_map.py | 19 +++++++++++-------- .../unit/isolated/test_exceptions_handlers.py | 12 ++++++------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index bdd6601ddb45..a3bafb1995fa 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -3,6 +3,10 @@ from typing import NamedTuple, TypeAlias from aiohttp import web +from common_library.error_codes import create_error_code +from common_library.json_serialization import json_dumps +from models_library.basic_types import IDStr +from models_library.rest_error import ErrorGet from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error @@ -68,11 +72,15 @@ async def _exception_handler( _DefaultDict(getattr(exception, "__dict__", {})) ) + error = ErrorGet(msg=user_msg) + if is_5xx_server_error(status_code): + oec = create_error_code(exception) _logger.exception( **create_troubleshotting_log_kwargs( user_msg, error=exception, + error_code=oec, error_context={ "request": request, "request.remote": f"{request.remote}", @@ -81,16 +89,11 @@ async def _exception_handler( }, ) ) + error = ErrorGet(msg=user_msg, support_id=IDStr(oec)) - # TODO: make this part customizable? e.g. so we can inject e.g. oec? - # TODO: connect with ErrorModel return web.json_response( - { - "error": { - "msg": user_msg, - } - }, - status=status_code, + data={"error": error.model_dump(exclude_unset=True, mode="json")}, + dumps=json_dumps, ) return _exception_handler diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index e9b49eaa80d2..f8b9c89356b6 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -12,6 +12,7 @@ from aiohttp import web from aiohttp.test_utils import make_mocked_request from servicelib.aiohttp import status +from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers import ( ExceptionToHttpErrorMap, @@ -94,7 +95,8 @@ 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.text + assert "one error message" in response.reason + assert response.content_type == MIMETYPE_APPLICATION_JSON async def test_factory__create_exception_handler_from_http_error_map( @@ -111,12 +113,10 @@ async def test_factory__create_exception_handler_from_http_error_map( ) # Converts exception in map - with pytest.raises(web.HTTPBadRequest) as exc_info: - await exc_handler(fake_request, OneError()) - got_exc = exc_info.value - assert isinstance(got_exc, web.HTTPBadRequest) - assert got_exc.reason == "Error One mapped to 400" + response = await exc_handler(fake_request, OneError()) + assert response.status == status.HTTP_400_BAD_REQUEST + assert response.reason == "Error One mapped to 400" # By-passes exceptions not listed err = RuntimeError() From 34c7dc2dbd8bbe1edc4ec66570a103398cb83b17 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 11:04:20 +0100 Subject: [PATCH 23/59] args --- .../exceptions_handlers_base.py | 6 +- .../isolated/test_exceptions_handlers_base.py | 94 +++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index 3d306f3edcfa..69661aefa304 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -46,8 +46,8 @@ async def _handled_exception_context_manager( exception_handler: AiohttpExceptionHandler, **forward_ctx, ) -> AsyncIterator[_ExceptionContext]: - """Calls `exception_handler` on exceptions raised in this context - and caught in `exception_catch` + """ + Calls `exception_handler` on exceptions raised in this context and caught in `exception_catch` """ ctx = _ExceptionContext() try: @@ -61,8 +61,8 @@ async def _handled_exception_context_manager( def create_decorator_from_exception_handler( + exception_types: type[BaseException] | tuple[type[BaseException], ...], exception_handler: AiohttpExceptionHandler, - exception_types: type[BaseException] | tuple[type[BaseException], ...] = Exception, ): """Returns a decorator for aiohttp's web.Handler functions diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py new file mode 100644 index 000000000000..c5b79e5bda8c --- /dev/null +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py @@ -0,0 +1,94 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +import pytest +from aiohttp import web +from aiohttp.test_utils import make_mocked_request +from simcore_service_webserver.errors import WebServerBaseError +from simcore_service_webserver.exceptions_handlers_base import ( + _handled_exception_context_manager, + create_decorator_from_exception_handler, +) + +# Some custom errors in my service + + +class BaseError(WebServerBaseError): + ... + + +class OneError(BaseError): + ... + + +class OtherError(BaseError): + ... + + +async def test__handled_exception_context_manager(): + + expected_request = make_mocked_request("GET", "/foo") + expected_response = web.json_response({"error": {"msg": "Foo"}}) + + # define exception-handler function + async def _custom_handler(request, exception): + assert request == expected_request + assert isinstance( + exception, BaseError + ), "only BasePluginError exceptions should call this handler" + return expected_response + + # exception-handler -> context manager + + # handles any BaseError returning a response + async with _handled_exception_context_manager( + BaseError, _custom_handler, request=expected_request + ) as ctx1: + raise OneError + assert ctx1.response == expected_response + + async with _handled_exception_context_manager( + BaseError, _custom_handler, request=expected_request + ) as ctx2: + raise OtherError + assert ctx2.response == expected_response + + # otherwise thru + with pytest.raises(ArithmeticError): + async with _handled_exception_context_manager( + BaseError, _custom_handler, request=expected_request + ): + raise ArithmeticError + + +@pytest.mark.parametrize("exception_cls", [OneError, OtherError]) +async def test_create_decorator_from_exception_handler(exception_cls: type[Exception]): + expected_request = make_mocked_request("GET", "/foo") + expected_exception = exception_cls() + expected_response = web.Response(reason="suppressed") + + # creates exception handler + async def _suppress_all(request: web.Request, exception): + assert exception == expected_exception + assert request == expected_request + return expected_response + + # create a decorator + _exc_handling_decorator = create_decorator_from_exception_handler( + exception_types=BaseError, # NOTE: base class + exception_handler=_suppress_all, + ) + + # using decorators + @_exc_handling_decorator + async def _rest_handler(request: web.Request) -> web.Response: + raise expected_exception + + # emulates request/response workflow + resp = await _rest_handler(expected_request) + assert resp == expected_response From d3894f29cf69252b1f6164a08975b0fc72cdde49 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 11:07:11 +0100 Subject: [PATCH 24/59] naming --- .../src/simcore_service_webserver/exceptions_handlers_base.py | 4 ++-- .../exceptions_handlers_http_error_map.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index 69661aefa304..f84541d3553a 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -42,7 +42,7 @@ class _ExceptionContext: @asynccontextmanager async def _handled_exception_context_manager( - exception_catch: type[BaseException] | tuple[type[BaseException], ...], + exception_types: type[BaseException] | tuple[type[BaseException], ...], exception_handler: AiohttpExceptionHandler, **forward_ctx, ) -> AsyncIterator[_ExceptionContext]: @@ -54,7 +54,7 @@ async def _handled_exception_context_manager( yield ctx - except exception_catch as e: + except exception_types as e: response = await exception_handler(exception=e, **forward_ctx) assert isinstance(response, web.Response) # nosec ctx.response = response diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index a3bafb1995fa..6b8ad98400a8 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -39,6 +39,7 @@ class HttpErrorInfo(NamedTuple): def create_exception_handler_from_http_error( exception_cls: type[BaseException], *, + # handler is produced from here status_code: int, msg_template: str, ) -> AiohttpExceptionHandler: From 01f03e567749c4aa15b1e8e5ffddb545646bcf66 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 13:29:31 +0100 Subject: [PATCH 25/59] refactor concept --- .../exceptions_handlers_base.py | 80 +++++++++++++++++-- .../exceptions_handlers_http_error_map.py | 22 +---- .../unit/isolated/test_exceptions_handlers.py | 34 -------- .../isolated/test_exceptions_handlers_base.py | 34 ++++++++ 4 files changed, 112 insertions(+), 58 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index f84541d3553a..e8a4eadbf59b 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -1,8 +1,10 @@ import functools import logging -from contextlib import asynccontextmanager +from collections.abc import AsyncIterator, Iterable +from contextlib import AbstractAsyncContextManager, asynccontextmanager from dataclasses import dataclass -from typing import AsyncIterator, Protocol +from types import TracebackType +from typing import Protocol from aiohttp import web from servicelib.aiohttp.typing_extension import Handler as WebHandler @@ -23,10 +25,7 @@ async def __call__( exception: BaseException, ) -> web.Response: """ - Callback to process an exception raised during a web request, allowing custom handling. - - This function can be implemented to suppress, (transform &) reraise as web.HTTPError - or return a web.Response + Callback that handles an exception produced during a request and transforms it into a response Arguments: request -- current request @@ -35,6 +34,74 @@ async def __call__( ... # pylint: disable=unnecessary-ellipsis +def _sort_exceptions_by_specificity( + exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True +) -> list[type[BaseException]]: + return sorted( + exceptions, + key=lambda exc: sum(issubclass(e, exc) for e in exceptions if e is not exc), + reverse=not concrete_first, + ) + + +class AsyncDynamicTryExceptContext(AbstractAsyncContextManager): + """Context manager to handle exceptions if they match any in the exception_handlers dictionary""" + + def __init__( + self, + exception_handlers: dict[type[BaseException], AiohttpExceptionHandler], + *, + request: web.Request, + ): + self.exception_handlers = exception_handlers + self.request = request + self.response = None + + async def __aenter__(self): + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc_value: BaseException | None, + traceback: TracebackType | None, + ) -> bool: + # FIXME: the specificity is not resolved by the __aexit__ caller + if exc_type is not None and exc_type in self.exception_handlers: + assert exc_value # nosec + + exc_handler = self.exception_handlers[exc_type] + self.response = await exc_handler(request=self.request, exception=exc_value) + return True # suppress + return False # reraise + + def get_response(self): + return self.response + + +def async_try_except_decorator( + exception_handlers: dict[type[BaseException], AiohttpExceptionHandler] +): + def _decorator(handler: WebHandler): + @functools.wraps(handler) + async def _wrapper(request: web.Request) -> web.StreamResponse: + cm = AsyncDynamicTryExceptContext(exception_handlers, request=request) + async with cm: + return await handler(request) + + # If an exception was handled, return the exception handler's return value + response = cm.get_response() + assert response is not None # nosec + return response + + return _wrapper + + return _decorator + + +# ---------------- + + @dataclass class _ExceptionContext: response: web.Response | None = None @@ -55,6 +122,7 @@ async def _handled_exception_context_manager( yield ctx except exception_types as e: + # NOTE: exception_types are automatically sorted by specififyt response = await exception_handler(exception=e, **forward_ctx) assert isinstance(response, web.Response) # nosec ctx.response = response diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index 6b8ad98400a8..fa82f359c705 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -1,5 +1,4 @@ import logging -from collections.abc import Iterable from typing import NamedTuple, TypeAlias from aiohttp import web @@ -11,7 +10,10 @@ from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error -from .exceptions_handlers_base import AiohttpExceptionHandler +from .exceptions_handlers_base import ( + AiohttpExceptionHandler, + _sort_exceptions_by_specificity, +) _logger = logging.getLogger(__name__) @@ -37,9 +39,6 @@ class HttpErrorInfo(NamedTuple): def create_exception_handler_from_http_error( - exception_cls: type[BaseException], - *, - # handler is produced from here status_code: int, msg_template: str, ) -> AiohttpExceptionHandler: @@ -54,7 +53,6 @@ def create_exception_handler_from_http_error( returned as-is for re-raising. Arguments: - exception_cls: exception raise during the request status_code: the http status code to associate at the web-api interface to this error msg_template: a template string to pass to the HttpError @@ -66,7 +64,6 @@ async def _exception_handler( request: web.Request, exception: BaseException, ) -> web.Response: - assert isinstance(exception, exception_cls) # nosec # safe formatting, i.e. does not raise user_msg = msg_template.format_map( @@ -100,16 +97,6 @@ async def _exception_handler( return _exception_handler -def _sort_exceptions_by_specificity( - exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True -) -> list[type[BaseException]]: - return sorted( - exceptions, - key=lambda exc: sum(issubclass(e, exc) for e in exceptions if e is not exc), - reverse=not concrete_first, - ) - - def create_exception_handler_from_http_error_map( exc_to_http_error_map: ExceptionToHttpErrorMap, ) -> AiohttpExceptionHandler: @@ -123,7 +110,6 @@ def create_exception_handler_from_http_error_map( _exception_handlers = { exc_cls: create_exception_handler_from_http_error( - exception_cls=exc_cls, status_code=http_error_info.status_code, msg_template=http_error_info.msg_template, ) diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py index f8b9c89356b6..64f6c653d7eb 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py @@ -23,7 +23,6 @@ _handled_exception_context_manager, ) from simcore_service_webserver.exceptions_handlers_http_error_map import ( - _sort_exceptions_by_specificity, create_exception_handler_from_http_error, create_exception_handler_from_http_error_map, ) @@ -43,39 +42,6 @@ class OtherError(BaseError): ... -def test_sort_concrete_first(): - assert _sort_exceptions_by_specificity([Exception, BaseError]) == [ - BaseError, - Exception, - ] - - assert _sort_exceptions_by_specificity( - [Exception, BaseError], concrete_first=False - ) == [ - Exception, - BaseError, - ] - - -def test_sort_exceptions_by_specificity(): - - got_exceptions_cls = _sort_exceptions_by_specificity( - [ - Exception, - OtherError, - OneError, - BaseError, - ValueError, - ArithmeticError, - ZeroDivisionError, - ] - ) - - for from_, exc in enumerate(got_exceptions_cls, start=1): - for exc_after in got_exceptions_cls[from_:]: - assert not issubclass(exc_after, exc), f"{got_exceptions_cls=}" - - @pytest.fixture def fake_request() -> web.Request: return make_mocked_request("GET", "/foo") diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py index c5b79e5bda8c..5f042dff8580 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py @@ -12,6 +12,7 @@ from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers_base import ( _handled_exception_context_manager, + _sort_exceptions_by_specificity, create_decorator_from_exception_handler, ) @@ -30,6 +31,39 @@ class OtherError(BaseError): ... +def test_sort_concrete_first(): + assert _sort_exceptions_by_specificity([Exception, BaseError]) == [ + BaseError, + Exception, + ] + + assert _sort_exceptions_by_specificity( + [Exception, BaseError], concrete_first=False + ) == [ + Exception, + BaseError, + ] + + +def test_sort_exceptions_by_specificity(): + + got_exceptions_cls = _sort_exceptions_by_specificity( + [ + Exception, + OtherError, + OneError, + BaseError, + ValueError, + ArithmeticError, + ZeroDivisionError, + ] + ) + + for from_, exc in enumerate(got_exceptions_cls, start=1): + for exc_after in got_exceptions_cls[from_:]: + assert not issubclass(exc_after, exc), f"{got_exceptions_cls=}" + + async def test__handled_exception_context_manager(): expected_request = make_mocked_request("GET", "/foo") From bdb9010fe4a0c31537fc7826683fef91e3a19830 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:16:52 +0100 Subject: [PATCH 26/59] base ready --- .../exceptions_handlers_base.py | 114 ++++++------------ .../workspaces/_exceptions_handlers.py | 4 + .../isolated/test_exceptions_handlers_base.py | 52 ++++---- 3 files changed, 68 insertions(+), 102 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index e8a4eadbf59b..254af97c42e3 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -1,8 +1,7 @@ import functools import logging -from collections.abc import AsyncIterator, Iterable -from contextlib import AbstractAsyncContextManager, asynccontextmanager -from dataclasses import dataclass +from collections.abc import Iterable +from contextlib import AbstractAsyncContextManager from types import TracebackType from typing import Protocol @@ -45,19 +44,41 @@ def _sort_exceptions_by_specificity( class AsyncDynamicTryExceptContext(AbstractAsyncContextManager): - """Context manager to handle exceptions if they match any in the exception_handlers dictionary""" + """Context manager to handle exceptions if they match any in the + exception_handlers_map""" def __init__( self, - exception_handlers: dict[type[BaseException], AiohttpExceptionHandler], + exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler], *, request: web.Request, ): - self.exception_handlers = exception_handlers - self.request = request - self.response = None + self._exception_handlers_map = exception_handlers_map + self._exceptions_types_priorized = _sort_exceptions_by_specificity( + list(self._exception_handlers_map.keys()), concrete_first=True + ) + self._request = request + self._response = None + + def _get_exc_handler_or_none( + self, exc_type: type[BaseException], exc_value: BaseException + ) -> AiohttpExceptionHandler | None: + exc_handler = self._exception_handlers_map.get(exc_type) + if not exc_handler and ( + base_exc_type := next( + ( + _type + for _type in self._exceptions_types_priorized + if isinstance(exc_value, _type) + ), + None, + ) + ): + exc_handler = self._exception_handlers_map[base_exc_type] + return exc_handler async def __aenter__(self): + self._response = None return self async def __aexit__( @@ -66,26 +87,28 @@ async def __aexit__( exc_value: BaseException | None, traceback: TracebackType | None, ) -> bool: - # FIXME: the specificity is not resolved by the __aexit__ caller - if exc_type is not None and exc_type in self.exception_handlers: - assert exc_value # nosec - - exc_handler = self.exception_handlers[exc_type] - self.response = await exc_handler(request=self.request, exception=exc_value) + if ( + exc_value is not None + and exc_type is not None + and (exc_handler := self._get_exc_handler_or_none(exc_type, exc_value)) + ): + self._response = await exc_handler( + request=self._request, exception=exc_value + ) return True # suppress return False # reraise def get_response(self): - return self.response + return self._response def async_try_except_decorator( - exception_handlers: dict[type[BaseException], AiohttpExceptionHandler] + exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ): def _decorator(handler: WebHandler): @functools.wraps(handler) async def _wrapper(request: web.Request) -> web.StreamResponse: - cm = AsyncDynamicTryExceptContext(exception_handlers, request=request) + cm = AsyncDynamicTryExceptContext(exception_handlers_map, request=request) async with cm: return await handler(request) @@ -97,60 +120,3 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: return _wrapper return _decorator - - -# ---------------- - - -@dataclass -class _ExceptionContext: - response: web.Response | None = None - - -@asynccontextmanager -async def _handled_exception_context_manager( - exception_types: type[BaseException] | tuple[type[BaseException], ...], - exception_handler: AiohttpExceptionHandler, - **forward_ctx, -) -> AsyncIterator[_ExceptionContext]: - """ - Calls `exception_handler` on exceptions raised in this context and caught in `exception_catch` - """ - ctx = _ExceptionContext() - try: - - yield ctx - - except exception_types as e: - # NOTE: exception_types are automatically sorted by specififyt - response = await exception_handler(exception=e, **forward_ctx) - assert isinstance(response, web.Response) # nosec - ctx.response = response - - -def create_decorator_from_exception_handler( - exception_types: type[BaseException] | tuple[type[BaseException], ...], - exception_handler: AiohttpExceptionHandler, -): - """Returns a decorator for aiohttp's web.Handler functions - - Builds a decorator function that applies _handled_exception_context to an aiohttp Handler - """ - - def _decorator(handler: WebHandler): - @functools.wraps(handler) - async def _wrapper(request: web.Request) -> web.StreamResponse: - - async with _handled_exception_context_manager( - exception_types, - exception_handler, - request=request, - ) as exc_ctx: - - return await handler(request) - - return exc_ctx.response - - return _wrapper - - return _decorator diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py index f6470f461f7d..5e52e2988f8c 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py @@ -22,6 +22,10 @@ _logger = logging.getLogger(__name__) +def tr(msg) -> str: + return msg + + _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { WorkspaceGroupNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py index 5f042dff8580..24b3efab0b1d 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py @@ -11,9 +11,10 @@ from aiohttp.test_utils import make_mocked_request from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers_base import ( - _handled_exception_context_manager, + AiohttpExceptionHandler, + AsyncDynamicTryExceptContext, _sort_exceptions_by_specificity, - create_decorator_from_exception_handler, + async_try_except_decorator, ) # Some custom errors in my service @@ -70,33 +71,35 @@ async def test__handled_exception_context_manager(): expected_response = web.json_response({"error": {"msg": "Foo"}}) # define exception-handler function - async def _custom_handler(request, exception): + async def _base_exc_handler(request, exception): assert request == expected_request - assert isinstance( - exception, BaseError - ), "only BasePluginError exceptions should call this handler" + assert isinstance(exception, BaseError) + assert not isinstance(exception, OtherError) return expected_response - # exception-handler -> context manager + async def _concrete_exc_handler(request, exception): + assert request == expected_request + assert isinstance(exception, OtherError) + return expected_response + + exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] = { + BaseError: _base_exc_handler, + OtherError: _concrete_exc_handler, + } # handles any BaseError returning a response - async with _handled_exception_context_manager( - BaseError, _custom_handler, request=expected_request - ) as ctx1: + cm = AsyncDynamicTryExceptContext(exception_handlers_map, request=expected_request) + async with cm: raise OneError - assert ctx1.response == expected_response + assert cm.get_response() == expected_response - async with _handled_exception_context_manager( - BaseError, _custom_handler, request=expected_request - ) as ctx2: + async with cm: raise OtherError - assert ctx2.response == expected_response + assert cm.get_response() == expected_response - # otherwise thru + # reraises with pytest.raises(ArithmeticError): - async with _handled_exception_context_manager( - BaseError, _custom_handler, request=expected_request - ): + async with cm: raise ArithmeticError @@ -104,7 +107,7 @@ async def _custom_handler(request, exception): async def test_create_decorator_from_exception_handler(exception_cls: type[Exception]): expected_request = make_mocked_request("GET", "/foo") expected_exception = exception_cls() - expected_response = web.Response(reason="suppressed") + expected_response = web.Response(reason=f"suppressed {exception_cls}") # creates exception handler async def _suppress_all(request: web.Request, exception): @@ -112,14 +115,7 @@ async def _suppress_all(request: web.Request, exception): assert request == expected_request return expected_response - # create a decorator - _exc_handling_decorator = create_decorator_from_exception_handler( - exception_types=BaseError, # NOTE: base class - exception_handler=_suppress_all, - ) - - # using decorators - @_exc_handling_decorator + @async_try_except_decorator({BaseError: _suppress_all}) async def _rest_handler(request: web.Request) -> web.Response: raise expected_exception From f760de3eeb59c31121fd0302cd72d1c32fbeeace Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:16:52 +0100 Subject: [PATCH 27/59] maps --- .../exceptions_handlers_base.py | 19 +- .../exceptions_handlers_http_error_map.py | 48 ++--- ...test_exceptions_handlers_http_error_map.py | 178 ++++++++++++++++++ 3 files changed, 200 insertions(+), 45 deletions(-) create mode 100644 services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index 254af97c42e3..1b7a3d54d295 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -3,7 +3,7 @@ from collections.abc import Iterable from contextlib import AbstractAsyncContextManager from types import TracebackType -from typing import Protocol +from typing import Protocol, TypeAlias from aiohttp import web from servicelib.aiohttp.typing_extension import Handler as WebHandler @@ -43,19 +43,22 @@ def _sort_exceptions_by_specificity( ) +ExceptionHandlersMap: TypeAlias = dict[type[BaseException], AiohttpExceptionHandler] + + class AsyncDynamicTryExceptContext(AbstractAsyncContextManager): """Context manager to handle exceptions if they match any in the exception_handlers_map""" def __init__( self, - exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler], + exception_handlers_map: ExceptionHandlersMap, *, request: web.Request, ): - self._exception_handlers_map = exception_handlers_map - self._exceptions_types_priorized = _sort_exceptions_by_specificity( - list(self._exception_handlers_map.keys()), concrete_first=True + self._exc_handlers_map = exception_handlers_map + self._exc_types_priorized = _sort_exceptions_by_specificity( + list(self._exc_handlers_map.keys()), concrete_first=True ) self._request = request self._response = None @@ -63,18 +66,18 @@ def __init__( def _get_exc_handler_or_none( self, exc_type: type[BaseException], exc_value: BaseException ) -> AiohttpExceptionHandler | None: - exc_handler = self._exception_handlers_map.get(exc_type) + exc_handler = self._exc_handlers_map.get(exc_type) if not exc_handler and ( base_exc_type := next( ( _type - for _type in self._exceptions_types_priorized + for _type in self._exc_types_priorized if isinstance(exc_value, _type) ), None, ) ): - exc_handler = self._exception_handlers_map[base_exc_type] + exc_handler = self._exc_handlers_map[base_exc_type] return exc_handler async def __aenter__(self): diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index fa82f359c705..b64cf356cdeb 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -10,10 +10,7 @@ from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error -from .exceptions_handlers_base import ( - AiohttpExceptionHandler, - _sort_exceptions_by_specificity, -) +from .exceptions_handlers_base import AiohttpExceptionHandler, ExceptionHandlersMap _logger = logging.getLogger(__name__) @@ -92,45 +89,22 @@ async def _exception_handler( return web.json_response( data={"error": error.model_dump(exclude_unset=True, mode="json")}, dumps=json_dumps, + reason=user_msg, + status=status_code, ) return _exception_handler -def create_exception_handler_from_http_error_map( +def to_exceptions_handlers_map( exc_to_http_error_map: ExceptionToHttpErrorMap, -) -> AiohttpExceptionHandler: - """ - Custom Exception-Handler factory - - Creates a custom `WebApiExceptionHandler` that maps one-to-one exception to status code error codes - - Analogous to `create_exception_handler_from_status_code` but ExceptionToHttpErrorMap as input - """ - - _exception_handlers = { - exc_cls: create_exception_handler_from_http_error( - status_code=http_error_info.status_code, - msg_template=http_error_info.msg_template, +) -> ExceptionHandlersMap: + """Converts { exc_type: (status, msg), ...} -> {exc_type: callable }""" + exc_handlers_map: ExceptionHandlersMap = { + exc_type: create_exception_handler_from_http_error( + status_code=info.status_code, msg_template=info.msg_template ) - for exc_cls, http_error_info in exc_to_http_error_map.items() + for exc_type, info in exc_to_http_error_map.items() } - _catch_exceptions = _sort_exceptions_by_specificity( - list(_exception_handlers.keys()) - ) - - async def _exception_handler( - request: web.Request, - exception: BaseException, - ) -> web.Response: - if exc_cls := next( - (_ for _ in _catch_exceptions if isinstance(exception, _)), None - ): - return await _exception_handlers[exc_cls]( - request=request, exception=exception - ) - # NOTE: not in my list, return so it gets reraised - return exception - - return _exception_handler + return exc_handlers_map diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py new file mode 100644 index 000000000000..b5f505ae9736 --- /dev/null +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py @@ -0,0 +1,178 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +import pytest +from aiohttp import web +from aiohttp.test_utils import make_mocked_request +from servicelib.aiohttp import status +from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON +from simcore_service_webserver.errors import WebServerBaseError +from simcore_service_webserver.exceptions_handlers_base import ( + AsyncDynamicTryExceptContext, +) +from simcore_service_webserver.exceptions_handlers_http_error_map import ( + ExceptionToHttpErrorMap, + HttpErrorInfo, + create_exception_handler_from_http_error, + to_exceptions_handlers_map, +) + +# Some custom errors in my service + + +class BaseError(WebServerBaseError): + ... + + +class OneError(BaseError): + ... + + +class OtherError(BaseError): + ... + + +@pytest.fixture +def fake_request() -> web.Request: + return make_mocked_request("GET", "/foo") + + +async def test_factory__create_exception_handler_from_http_error( + fake_request: web.Request, +): + one_error_to_404 = create_exception_handler_from_http_error( + status_code=status.HTTP_404_NOT_FOUND, + msg_template="one error message for the user: {code} {value}", + ) + + # calling exception handler + caught = OneError() + 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.content_type == MIMETYPE_APPLICATION_JSON + + +async def test_factory__create_exception_handler_from_http_error_map( + fake_request: web.Request, +): + exc_to_http_error_map: ExceptionToHttpErrorMap = { + OneError: HttpErrorInfo(status.HTTP_400_BAD_REQUEST, "Error One mapped to 400") + } + + cm = AsyncDynamicTryExceptContext( + to_exceptions_handlers_map(exc_to_http_error_map), request=fake_request + ) + async with cm: + raise OneError + + response = cm.get_response() + assert response is not None + assert response.status == status.HTTP_400_BAD_REQUEST + assert response.reason == "Error One mapped to 400" + + # By-passes exceptions not listed + err = RuntimeError() + with pytest.raises(RuntimeError) as err_info: + async with cm: + raise err + + assert cm.get_response() is None + assert err_info.value == err + + +# async def test__handled_exception_context_manager(fake_request: web.Request): + +# expected_response = web.json_response({"error": {"msg": "Foo"}}) + +# async def _custom_handler(request, exception): +# assert request == fake_request +# assert isinstance( +# exception, BaseError +# ), "only BasePluginError exceptions should call this handler" +# return expected_response + +# exc_handling_ctx = _handled_exception_context_manager( +# BaseError, _custom_handler, request=fake_request +# ) + +# # handles any BaseError returning a response +# async with exc_handling_ctx as ctx: +# raise OneError +# assert ctx.response == expected_response + +# async with exc_handling_ctx as ctx: +# raise OtherError +# assert ctx.response == expected_response + +# # otherwise thru +# with pytest.raises(ArithmeticError): +# async with exc_handling_ctx: +# raise ArithmeticError + + +# async def test_create_decorator_from_exception_handler( +# caplog: pytest.LogCaptureFixture, +# ): +# # Create an SINGLE exception handler that acts as umbrella for all these exceptions +# http_error_map: ExceptionToHttpErrorMap = { +# OneError: HttpErrorInfo( +# status.HTTP_503_SERVICE_UNAVAILABLE, +# "Human readable error transmitted to the front-end", +# ) +# } + +# exc_handler = create_exception_handler_from_http_error_map(http_error_map) +# _exc_handling_ctx = create_decorator_from_exception_handler( +# exception_types=BaseError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! +# exception_handler=exc_handler, +# ) + +# @_exc_handling_ctx +# async def _rest_handler(request: web.Request) -> web.Response: +# if request.query.get("raise") == "OneError": +# raise OneError +# if request.query.get("raise") == "ArithmeticError": +# raise ArithmeticError + +# return web.Response(reason="all good") + +# with caplog.at_level(logging.ERROR): + +# # emulates successful call +# resp = await _rest_handler(make_mocked_request("GET", "/foo")) +# assert resp.status == status.HTTP_200_OK +# assert resp.reason == "all good" + +# assert not caplog.records + +# # this will be passed and catched by the outermost error middleware +# with pytest.raises(ArithmeticError): +# await _rest_handler( +# make_mocked_request("GET", "/foo?raise=ArithmeticError") +# ) + +# assert not caplog.records + +# # this is a 5XX will be converted to response but is logged as error as well +# with pytest.raises(web.HTTPException) as exc_info: +# await _rest_handler(make_mocked_request("GET", "/foo?raise=OneError")) + +# resp = exc_info.value +# assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE +# assert "front-end" in resp.reason + +# assert caplog.records, "Expected 5XX troubleshooting logged as error" +# assert caplog.records[0].levelno == logging.ERROR + +# # typically capture by last +# with pytest.raises(ArithmeticError): +# resp = await _rest_handler( +# make_mocked_request("GET", "/foo?raise=ArithmeticError") +# ) From 89a6058d757507232ce56f2f0302db66e5a8435f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:54:55 +0100 Subject: [PATCH 28/59] tests --- .../exceptions_handlers_http_error_map.py | 2 +- ...test_exceptions_handlers_http_error_map.py | 170 ++++++++---------- 2 files changed, 74 insertions(+), 98 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index b64cf356cdeb..72fcd364a35d 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -99,7 +99,7 @@ async def _exception_handler( def to_exceptions_handlers_map( exc_to_http_error_map: ExceptionToHttpErrorMap, ) -> ExceptionHandlersMap: - """Converts { exc_type: (status, msg), ...} -> {exc_type: callable }""" + """Converts { exc_type: (status, msg), ... } -> { exc_type: callable, ... }""" exc_handlers_map: ExceptionHandlersMap = { exc_type: create_exception_handler_from_http_error( status_code=info.status_code, msg_template=info.msg_template diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py index b5f505ae9736..498c4527ed99 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py @@ -6,6 +6,8 @@ # pylint: disable=unused-variable +import logging + import pytest from aiohttp import web from aiohttp.test_utils import make_mocked_request @@ -14,6 +16,7 @@ from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers_base import ( AsyncDynamicTryExceptContext, + async_try_except_decorator, ) from simcore_service_webserver.exceptions_handlers_http_error_map import ( ExceptionToHttpErrorMap, @@ -59,120 +62,93 @@ async def test_factory__create_exception_handler_from_http_error( assert response.content_type == MIMETYPE_APPLICATION_JSON -async def test_factory__create_exception_handler_from_http_error_map( +async def test_handling_different_exceptions_with_context( fake_request: web.Request, + caplog: pytest.LogCaptureFixture, ): exc_to_http_error_map: ExceptionToHttpErrorMap = { - OneError: HttpErrorInfo(status.HTTP_400_BAD_REQUEST, "Error One mapped to 400") + OneError: HttpErrorInfo(status.HTTP_400_BAD_REQUEST, "Error {code} to 400"), + OtherError: HttpErrorInfo(status.HTTP_500_INTERNAL_SERVER_ERROR, "{code}"), } - cm = AsyncDynamicTryExceptContext( to_exceptions_handlers_map(exc_to_http_error_map), request=fake_request ) - async with cm: - raise OneError - - response = cm.get_response() - assert response is not None - assert response.status == status.HTTP_400_BAD_REQUEST - assert response.reason == "Error One mapped to 400" - # By-passes exceptions not listed - err = RuntimeError() - with pytest.raises(RuntimeError) as err_info: + with caplog.at_level(logging.ERROR): + # handles as 4XX async with cm: - raise err - - assert cm.get_response() is None - assert err_info.value == err - - -# async def test__handled_exception_context_manager(fake_request: web.Request): - -# expected_response = web.json_response({"error": {"msg": "Foo"}}) - -# async def _custom_handler(request, exception): -# assert request == fake_request -# assert isinstance( -# exception, BaseError -# ), "only BasePluginError exceptions should call this handler" -# return expected_response - -# exc_handling_ctx = _handled_exception_context_manager( -# BaseError, _custom_handler, request=fake_request -# ) - -# # handles any BaseError returning a response -# async with exc_handling_ctx as ctx: -# raise OneError -# assert ctx.response == expected_response - -# async with exc_handling_ctx as ctx: -# raise OtherError -# assert ctx.response == expected_response - -# # otherwise thru -# with pytest.raises(ArithmeticError): -# async with exc_handling_ctx: -# raise ArithmeticError - - -# async def test_create_decorator_from_exception_handler( -# caplog: pytest.LogCaptureFixture, -# ): -# # Create an SINGLE exception handler that acts as umbrella for all these exceptions -# http_error_map: ExceptionToHttpErrorMap = { -# OneError: HttpErrorInfo( -# status.HTTP_503_SERVICE_UNAVAILABLE, -# "Human readable error transmitted to the front-end", -# ) -# } - -# exc_handler = create_exception_handler_from_http_error_map(http_error_map) -# _exc_handling_ctx = create_decorator_from_exception_handler( -# exception_types=BaseError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! -# exception_handler=exc_handler, -# ) + raise OneError + + response = cm.get_response() + 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 not caplog.records + + # unhandled -> reraises + err = RuntimeError() + with pytest.raises(RuntimeError) as err_info: + async with cm: + raise err + + assert cm.get_response() is None + assert err_info.value == err + + # handles as 5XX and logs + async with cm: + raise OtherError -# @_exc_handling_ctx -# async def _rest_handler(request: web.Request) -> web.Response: -# if request.query.get("raise") == "OneError": -# raise OneError -# if request.query.get("raise") == "ArithmeticError": -# raise ArithmeticError + response = cm.get_response() + 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 caplog.records, "Expected 5XX troubleshooting logged as error" + assert caplog.records[0].levelno == logging.ERROR -# return web.Response(reason="all good") -# with caplog.at_level(logging.ERROR): +async def test_handling_different_exceptions_with_decorator( + fake_request: web.Request, + caplog: pytest.LogCaptureFixture, +): + exc_to_http_error_map: ExceptionToHttpErrorMap = { + OneError: HttpErrorInfo(status.HTTP_503_SERVICE_UNAVAILABLE, "{code}"), + } -# # emulates successful call -# resp = await _rest_handler(make_mocked_request("GET", "/foo")) -# assert resp.status == status.HTTP_200_OK -# assert resp.reason == "all good" + exc_handling_decorator = async_try_except_decorator( + to_exceptions_handlers_map(exc_to_http_error_map) + ) -# assert not caplog.records + @exc_handling_decorator + async def _rest_handler(request: web.Request) -> web.Response: + if request.query.get("raise") == "OneError": + raise OneError + if request.query.get("raise") == "ArithmeticError": + raise ArithmeticError + return web.json_response(reason="all good") -# # this will be passed and catched by the outermost error middleware -# with pytest.raises(ArithmeticError): -# await _rest_handler( -# make_mocked_request("GET", "/foo?raise=ArithmeticError") -# ) + with caplog.at_level(logging.ERROR): -# assert not caplog.records + # emulates successful call + resp = await _rest_handler(make_mocked_request("GET", "/foo")) + assert resp.status == status.HTTP_200_OK + assert resp.reason == "all good" -# # this is a 5XX will be converted to response but is logged as error as well -# with pytest.raises(web.HTTPException) as exc_info: -# await _rest_handler(make_mocked_request("GET", "/foo?raise=OneError")) + assert not caplog.records -# resp = exc_info.value -# assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE -# assert "front-end" in resp.reason + # reraised + with pytest.raises(ArithmeticError): + await _rest_handler( + make_mocked_request("GET", "/foo?raise=ArithmeticError") + ) -# assert caplog.records, "Expected 5XX troubleshooting logged as error" -# assert caplog.records[0].levelno == logging.ERROR + assert not caplog.records -# # typically capture by last -# with pytest.raises(ArithmeticError): -# resp = await _rest_handler( -# make_mocked_request("GET", "/foo?raise=ArithmeticError") -# ) + # handles as 5XX and logs + resp = await _rest_handler(make_mocked_request("GET", "/foo?raise=OneError")) + assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE + assert caplog.records, "Expected 5XX troubleshooting logged as error" + assert caplog.records[0].levelno == logging.ERROR From a1dcd60abd9c22b48d3baea5e2ff25e0b17b9ff1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 19:07:09 +0100 Subject: [PATCH 29/59] using new functions --- .../exceptions_handlers.py | 8 +- .../folders/_exceptions_handlers.py | 16 +- .../projects/_trash_handlers.py | 15 +- .../workspaces/_exceptions_handlers.py | 19 +- .../unit/isolated/test_exceptions_handlers.py | 180 ------------------ .../isolated/test_exceptions_handlers_base.py | 2 +- 6 files changed, 20 insertions(+), 220 deletions(-) delete mode 100644 services/web/server/tests/unit/isolated/test_exceptions_handlers.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 5072dbca5dd3..7dc858d2486d 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -1,15 +1,15 @@ -from .exceptions_handlers_base import create_decorator_from_exception_handler +from .exceptions_handlers_base import async_try_except_decorator from .exceptions_handlers_http_error_map import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_exception_handler_from_http_error_map, + to_exceptions_handlers_map, ) __all__: tuple[str, ...] = ( "ExceptionToHttpErrorMap", "HttpErrorInfo", - "create_decorator_from_exception_handler", - "create_exception_handler_from_http_error_map", + "async_try_except_decorator", + "to_exceptions_handlers_map", ) # nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py index adc8fb4e7429..f4601d4c04ef 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py @@ -5,19 +5,14 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_decorator_from_exception_handler, - create_exception_handler_from_http_error_map, -) -from ..projects.exceptions import ( - BaseProjectError, - ProjectRunningConflictError, - ProjectStoppingError, + async_try_except_decorator, + to_exceptions_handlers_map, ) +from ..projects.exceptions import ProjectRunningConflictError, ProjectStoppingError from ..workspaces.errors import ( WorkspaceAccessForbiddenError, WorkspaceFolderInconsistencyError, WorkspaceNotFoundError, - WorkspacesValueError, ) from .errors import ( FolderAccessForbiddenError, @@ -70,8 +65,7 @@ } -handle_plugin_requests_exceptions = create_decorator_from_exception_handler( - exception_types=(BaseProjectError, FoldersValueError, WorkspacesValueError), - exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), +handle_plugin_requests_exceptions = async_try_except_decorator( + to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) ) # this is one decorator with a single exception handler diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py index 2074b6d36236..ab44109d2948 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py @@ -12,8 +12,8 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_decorator_from_exception_handler, - create_exception_handler_from_http_error_map, + async_try_except_decorator, + to_exceptions_handlers_map, ) from ..login.decorators import get_user_id, login_required from ..products.api import get_product_name @@ -21,11 +21,7 @@ from ..security.decorators import permission_required from . import _trash_api from ._common_models import RemoveQueryParams -from .exceptions import ( - ProjectRunningConflictError, - ProjectStoppingError, - ProjectTrashError, -) +from .exceptions import ProjectRunningConflictError, ProjectStoppingError _logger = logging.getLogger(__name__) @@ -46,9 +42,8 @@ } -_handle_exceptions = create_decorator_from_exception_handler( - exception_types=ProjectTrashError, - exception_handler=create_exception_handler_from_http_error_map(_TO_HTTP_ERROR_MAP), +_handle_exceptions = async_try_except_decorator( + to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) ) diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py index 5e52e2988f8c..cb3e19eeee0a 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py @@ -5,27 +5,19 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_exception_handlers_decorator, -) -from ..projects.exceptions import ( - BaseProjectError, - ProjectRunningConflictError, - ProjectStoppingError, + async_try_except_decorator, + to_exceptions_handlers_map, ) +from ..projects.exceptions import ProjectRunningConflictError, ProjectStoppingError from .errors import ( WorkspaceAccessForbiddenError, WorkspaceGroupNotFoundError, WorkspaceNotFoundError, - WorkspacesValueError, ) _logger = logging.getLogger(__name__) -def tr(msg) -> str: - return msg - - _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { WorkspaceGroupNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, @@ -51,7 +43,6 @@ def tr(msg) -> str: } -handle_plugin_requests_exceptions = create_exception_handlers_decorator( - exceptions_catch=(BaseProjectError, WorkspacesValueError), - exc_to_status_map=_TO_HTTP_ERROR_MAP, +handle_plugin_requests_exceptions = async_try_except_decorator( + to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) ) diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers.py deleted file mode 100644 index 64f6c653d7eb..000000000000 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers.py +++ /dev/null @@ -1,180 +0,0 @@ -# pylint: disable=protected-access -# pylint: disable=redefined-outer-name -# pylint: disable=too-many-arguments -# pylint: disable=too-many-statements -# pylint: disable=unused-argument -# pylint: disable=unused-variable - - -import logging - -import pytest -from aiohttp import web -from aiohttp.test_utils import make_mocked_request -from servicelib.aiohttp import status -from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON -from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exceptions_handlers import ( - ExceptionToHttpErrorMap, - HttpErrorInfo, - create_decorator_from_exception_handler, -) -from simcore_service_webserver.exceptions_handlers_base import ( - _handled_exception_context_manager, -) -from simcore_service_webserver.exceptions_handlers_http_error_map import ( - create_exception_handler_from_http_error, - create_exception_handler_from_http_error_map, -) - -# Some custom errors in my service - - -class BaseError(WebServerBaseError): - ... - - -class OneError(BaseError): - ... - - -class OtherError(BaseError): - ... - - -@pytest.fixture -def fake_request() -> web.Request: - return make_mocked_request("GET", "/foo") - - -async def test_factory__create_exception_handler_from_http_error( - fake_request: web.Request, -): - one_error_to_404 = create_exception_handler_from_http_error( - OneError, - status_code=status.HTTP_404_NOT_FOUND, - msg_template="one error message for the user: {code} {value}", - ) - - # calling exception handler - caught = OneError() - 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.content_type == MIMETYPE_APPLICATION_JSON - - -async def test_factory__create_exception_handler_from_http_error_map( - fake_request: web.Request, -): - - # call exception handler factory - exc_handler = create_exception_handler_from_http_error_map( - { - OneError: HttpErrorInfo( - status.HTTP_400_BAD_REQUEST, "Error One mapped to 400" - ) - } - ) - - # Converts exception in map - - response = await exc_handler(fake_request, OneError()) - assert response.status == status.HTTP_400_BAD_REQUEST - assert response.reason == "Error One mapped to 400" - - # By-passes exceptions not listed - err = RuntimeError() - assert await exc_handler(fake_request, err) is err - - -async def test__handled_exception_context_manager(fake_request: web.Request): - - expected_response = web.json_response({"error": {"msg": "Foo"}}) - - async def _custom_handler(request, exception): - assert request == fake_request - assert isinstance( - exception, BaseError - ), "only BasePluginError exceptions should call this handler" - return expected_response - - exc_handling_ctx = _handled_exception_context_manager( - BaseError, _custom_handler, request=fake_request - ) - - # handles any BaseError returning a response - async with exc_handling_ctx as ctx: - raise OneError - assert ctx.response == expected_response - - async with exc_handling_ctx as ctx: - raise OtherError - assert ctx.response == expected_response - - # otherwise thru - with pytest.raises(ArithmeticError): - async with exc_handling_ctx: - raise ArithmeticError - - -async def test_create_decorator_from_exception_handler( - caplog: pytest.LogCaptureFixture, -): - # Create an SINGLE exception handler that acts as umbrella for all these exceptions - http_error_map: ExceptionToHttpErrorMap = { - OneError: HttpErrorInfo( - status.HTTP_503_SERVICE_UNAVAILABLE, - "Human readable error transmitted to the front-end", - ) - } - - exc_handler = create_exception_handler_from_http_error_map(http_error_map) - _exc_handling_ctx = create_decorator_from_exception_handler( - exception_types=BaseError, # <--- FIXME" this is redundant because exception has been already passed in exc_handler! - exception_handler=exc_handler, - ) - - @_exc_handling_ctx - async def _rest_handler(request: web.Request) -> web.Response: - if request.query.get("raise") == "OneError": - raise OneError - if request.query.get("raise") == "ArithmeticError": - raise ArithmeticError - - return web.Response(reason="all good") - - with caplog.at_level(logging.ERROR): - - # emulates successful call - resp = await _rest_handler(make_mocked_request("GET", "/foo")) - assert resp.status == status.HTTP_200_OK - assert resp.reason == "all good" - - assert not caplog.records - - # this will be passed and catched by the outermost error middleware - with pytest.raises(ArithmeticError): - await _rest_handler( - make_mocked_request("GET", "/foo?raise=ArithmeticError") - ) - - assert not caplog.records - - # this is a 5XX will be converted to response but is logged as error as well - with pytest.raises(web.HTTPException) as exc_info: - await _rest_handler(make_mocked_request("GET", "/foo?raise=OneError")) - - resp = exc_info.value - assert resp.status == status.HTTP_503_SERVICE_UNAVAILABLE - assert "front-end" in resp.reason - - assert caplog.records, "Expected 5XX troubleshooting logged as error" - assert caplog.records[0].levelno == logging.ERROR - - # typically capture by last - with pytest.raises(ArithmeticError): - resp = await _rest_handler( - make_mocked_request("GET", "/foo?raise=ArithmeticError") - ) diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py index 24b3efab0b1d..87af9c093cc8 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py @@ -104,7 +104,7 @@ async def _concrete_exc_handler(request, exception): @pytest.mark.parametrize("exception_cls", [OneError, OtherError]) -async def test_create_decorator_from_exception_handler(exception_cls: type[Exception]): +async def test_async_try_except_decorator(exception_cls: type[Exception]): expected_request = make_mocked_request("GET", "/foo") expected_exception = exception_cls() expected_response = web.Response(reason=f"suppressed {exception_cls}") From 7a25fd436620fe0a1fa7757a26d54fa3c60f76ec Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 19:36:55 +0100 Subject: [PATCH 30/59] renames --- .../exceptions_handlers.py | 4 +- .../exceptions_handlers_base.py | 26 +++++++--- .../exceptions_handlers_http_error_map.py | 48 ++++++++++++++----- .../folders/_exceptions_handlers.py | 4 +- .../projects/_trash_handlers.py | 4 +- .../workspaces/_exceptions_handlers.py | 4 +- .../isolated/test_exceptions_handlers_base.py | 10 ++-- ...test_exceptions_handlers_http_error_map.py | 12 ++--- 8 files changed, 75 insertions(+), 37 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py index 7dc858d2486d..909157b1e160 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers.py @@ -1,4 +1,4 @@ -from .exceptions_handlers_base import async_try_except_decorator +from .exceptions_handlers_base import exception_handling_decorator from .exceptions_handlers_http_error_map import ( ExceptionToHttpErrorMap, HttpErrorInfo, @@ -8,7 +8,7 @@ __all__: tuple[str, ...] = ( "ExceptionToHttpErrorMap", "HttpErrorInfo", - "async_try_except_decorator", + "exception_handling_decorator", "to_exceptions_handlers_map", ) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index 1b7a3d54d295..df3a7f95be30 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -7,6 +7,7 @@ from aiohttp import web from servicelib.aiohttp.typing_extension import Handler as WebHandler +from servicelib.aiohttp.typing_extension import Middleware as WebMiddleware _logger = logging.getLogger(__name__) @@ -46,10 +47,7 @@ def _sort_exceptions_by_specificity( ExceptionHandlersMap: TypeAlias = dict[type[BaseException], AiohttpExceptionHandler] -class AsyncDynamicTryExceptContext(AbstractAsyncContextManager): - """Context manager to handle exceptions if they match any in the - exception_handlers_map""" - +class ExceptionHandlingContextManager(AbstractAsyncContextManager): def __init__( self, exception_handlers_map: ExceptionHandlersMap, @@ -105,13 +103,15 @@ def get_response(self): return self._response -def async_try_except_decorator( +def exception_handling_decorator( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ): def _decorator(handler: WebHandler): @functools.wraps(handler) async def _wrapper(request: web.Request) -> web.StreamResponse: - cm = AsyncDynamicTryExceptContext(exception_handlers_map, request=request) + cm = ExceptionHandlingContextManager( + exception_handlers_map, request=request + ) async with cm: return await handler(request) @@ -123,3 +123,17 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: return _wrapper return _decorator + + +def exception_handling_middleware( + exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] +) -> WebMiddleware: + _handle_excs = exception_handling_decorator( + exception_handlers_map=exception_handlers_map + ) + + @web.middleware + async def middleware_handler(request: web.Request, handler: WebHandler): + return await _handle_excs(handler)(request) + + return middleware_handler diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py index 72fcd364a35d..d524234c368e 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py @@ -8,16 +8,22 @@ from models_library.rest_error import ErrorGet from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions from servicelib.logging_errors import create_troubleshotting_log_kwargs -from servicelib.status_codes_utils import is_5xx_server_error +from servicelib.status_codes_utils import is_5xx_server_error, is_error from .exceptions_handlers_base import AiohttpExceptionHandler, ExceptionHandlersMap _logger = logging.getLogger(__name__) -_STATUS_CODE_TO_HTTP_ERRORS: dict[ - int, type[web.HTTPException] -] = get_all_aiohttp_http_exceptions(web.HTTPError) +def create_error_response(error: ErrorGet, status_code: int) -> web.Response: + assert is_error(status_code), f"{status_code=} must be an error [{error=}]" # nosec + + return web.json_response( + data={"error": error.model_dump(exclude_unset=True, mode="json")}, + dumps=json_dumps, + reason=error.msg, + status=status_code, + ) class _DefaultDict(dict): @@ -35,7 +41,7 @@ class HttpErrorInfo(NamedTuple): ExceptionToHttpErrorMap: TypeAlias = dict[type[BaseException], HttpErrorInfo] -def create_exception_handler_from_http_error( +def create_exception_handler_from_http_info( status_code: int, msg_template: str, ) -> AiohttpExceptionHandler: @@ -56,10 +62,13 @@ def create_exception_handler_from_http_error( Returns: A web api exception handler """ + assert is_error( # nosec + status_code + ), f"{status_code=} must be an error [{msg_template=}]" async def _exception_handler( request: web.Request, - exception: BaseException, + exception: BaseException, # TODO: for different type of exceptions e.g HTTPError ) -> web.Response: # safe formatting, i.e. does not raise @@ -86,12 +95,7 @@ async def _exception_handler( ) error = ErrorGet(msg=user_msg, support_id=IDStr(oec)) - return web.json_response( - data={"error": error.model_dump(exclude_unset=True, mode="json")}, - dumps=json_dumps, - reason=user_msg, - status=status_code, - ) + return create_error_response(error, status_code=status_code) return _exception_handler @@ -101,10 +105,28 @@ def to_exceptions_handlers_map( ) -> ExceptionHandlersMap: """Converts { exc_type: (status, msg), ... } -> { exc_type: callable, ... }""" exc_handlers_map: ExceptionHandlersMap = { - exc_type: create_exception_handler_from_http_error( + exc_type: create_exception_handler_from_http_info( status_code=info.status_code, msg_template=info.msg_template ) for exc_type, info in exc_to_http_error_map.items() } return exc_handlers_map + + +_STATUS_CODE_TO_HTTP_ERRORS: dict[ + int, type[web.HTTPError] +] = get_all_aiohttp_http_exceptions(web.HTTPError) + + +def create_http_error_exception_handlers_map(): + """ + Creates handles for all web.HTTPError + """ + exc_handlers_map: ExceptionHandlersMap = { + exc_type: create_exception_handler_from_http_info( + status_code=code, msg_template="{reason}" + ) + for code, exc_type in _STATUS_CODE_TO_HTTP_ERRORS.items() + } + return exc_handlers_map diff --git a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py index f4601d4c04ef..849fcf256e3e 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py @@ -5,7 +5,7 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - async_try_except_decorator, + exception_handling_decorator, to_exceptions_handlers_map, ) from ..projects.exceptions import ProjectRunningConflictError, ProjectStoppingError @@ -65,7 +65,7 @@ } -handle_plugin_requests_exceptions = async_try_except_decorator( +handle_plugin_requests_exceptions = exception_handling_decorator( to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) ) # this is one decorator with a single exception handler diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py index ab44109d2948..720fb05218e4 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py @@ -12,7 +12,7 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - async_try_except_decorator, + exception_handling_decorator, to_exceptions_handlers_map, ) from ..login.decorators import get_user_id, login_required @@ -42,7 +42,7 @@ } -_handle_exceptions = async_try_except_decorator( +_handle_exceptions = exception_handling_decorator( to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) ) diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py index cb3e19eeee0a..d72cf7a42446 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py @@ -5,7 +5,7 @@ from ..exceptions_handlers import ( ExceptionToHttpErrorMap, HttpErrorInfo, - async_try_except_decorator, + exception_handling_decorator, to_exceptions_handlers_map, ) from ..projects.exceptions import ProjectRunningConflictError, ProjectStoppingError @@ -43,6 +43,6 @@ } -handle_plugin_requests_exceptions = async_try_except_decorator( +handle_plugin_requests_exceptions = exception_handling_decorator( to_exceptions_handlers_map(_TO_HTTP_ERROR_MAP) ) diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py index 87af9c093cc8..1fdc4125275e 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py @@ -12,9 +12,9 @@ from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers_base import ( AiohttpExceptionHandler, - AsyncDynamicTryExceptContext, + ExceptionHandlingContextManager, _sort_exceptions_by_specificity, - async_try_except_decorator, + exception_handling_decorator, ) # Some custom errors in my service @@ -88,7 +88,9 @@ async def _concrete_exc_handler(request, exception): } # handles any BaseError returning a response - cm = AsyncDynamicTryExceptContext(exception_handlers_map, request=expected_request) + cm = ExceptionHandlingContextManager( + exception_handlers_map, request=expected_request + ) async with cm: raise OneError assert cm.get_response() == expected_response @@ -115,7 +117,7 @@ async def _suppress_all(request: web.Request, exception): assert request == expected_request return expected_response - @async_try_except_decorator({BaseError: _suppress_all}) + @exception_handling_decorator({BaseError: _suppress_all}) async def _rest_handler(request: web.Request) -> web.Response: raise expected_exception diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py index 498c4527ed99..330b8500aa06 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py @@ -15,13 +15,13 @@ from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from simcore_service_webserver.errors import WebServerBaseError from simcore_service_webserver.exceptions_handlers_base import ( - AsyncDynamicTryExceptContext, - async_try_except_decorator, + ExceptionHandlingContextManager, + exception_handling_decorator, ) from simcore_service_webserver.exceptions_handlers_http_error_map import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_exception_handler_from_http_error, + create_exception_handler_from_http_info, to_exceptions_handlers_map, ) @@ -48,7 +48,7 @@ def fake_request() -> web.Request: async def test_factory__create_exception_handler_from_http_error( fake_request: web.Request, ): - one_error_to_404 = create_exception_handler_from_http_error( + one_error_to_404 = create_exception_handler_from_http_info( status_code=status.HTTP_404_NOT_FOUND, msg_template="one error message for the user: {code} {value}", ) @@ -70,7 +70,7 @@ async def test_handling_different_exceptions_with_context( OneError: HttpErrorInfo(status.HTTP_400_BAD_REQUEST, "Error {code} to 400"), OtherError: HttpErrorInfo(status.HTTP_500_INTERNAL_SERVER_ERROR, "{code}"), } - cm = AsyncDynamicTryExceptContext( + cm = ExceptionHandlingContextManager( to_exceptions_handlers_map(exc_to_http_error_map), request=fake_request ) @@ -118,7 +118,7 @@ async def test_handling_different_exceptions_with_decorator( OneError: HttpErrorInfo(status.HTTP_503_SERVICE_UNAVAILABLE, "{code}"), } - exc_handling_decorator = async_try_except_decorator( + exc_handling_decorator = exception_handling_decorator( to_exceptions_handlers_map(exc_to_http_error_map) ) From 8cd958995591dd60cc2a87e02507dbb777b6b4f3 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:47:07 +0100 Subject: [PATCH 31/59] doc --- .../exceptions_handlers_base.py | 24 +++++++++++++++---- .../test_exceptions_handlers_base_2.py | 3 --- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index df3a7f95be30..f1b66959ce82 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -48,6 +48,22 @@ def _sort_exceptions_by_specificity( class ExceptionHandlingContextManager(AbstractAsyncContextManager): + """ + Essentially a dynamic try-except context manager to handle exceptions raised by a aiohttp handler, roughly: + ``` + try: + + resp = await handler(request) + + except exc_type1 as exc1: + resp = await exc_handler1(request) + except exc_type2 as exc1: + resp = await exc_handler2(request) + + # except ... dynamically as in `exception_handlers_map` + ``` + """ + def __init__( self, exception_handlers_map: ExceptionHandlersMap, @@ -55,11 +71,11 @@ def __init__( request: web.Request, ): self._exc_handlers_map = exception_handlers_map - self._exc_types_priorized = _sort_exceptions_by_specificity( + self._exc_types_by_specificity = _sort_exceptions_by_specificity( list(self._exc_handlers_map.keys()), concrete_first=True ) - self._request = request - self._response = None + self._request: web.Request = request + self._response: web.Response | None = None def _get_exc_handler_or_none( self, exc_type: type[BaseException], exc_value: BaseException @@ -69,7 +85,7 @@ def _get_exc_handler_or_none( base_exc_type := next( ( _type - for _type in self._exc_types_priorized + for _type in self._exc_types_by_specificity if isinstance(exc_value, _type) ), None, diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py index 71a2cfcc04b5..91fb0a3a9953 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py @@ -55,9 +55,6 @@ async def my_error_handler(request: web.Request, exc: BaseException): # mapper is defined as a higher abstraction # that automatically produces an exception handler - # map two exceptions? - add_exception_mapper(app, OtherError, web.HTTPNotFound) - # this is a handler create mapping to status_code and reason add_exception_mapper(app, OtherError, status.HTTP_404_NOT_FOUND) From b55506ba681b282482f22afb27c15ec744db2d28 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:47:25 +0100 Subject: [PATCH 32/59] doc --- .../exceptions_handlers_base.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py index f1b66959ce82..d159dc2abe16 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py @@ -49,19 +49,22 @@ def _sort_exceptions_by_specificity( class ExceptionHandlingContextManager(AbstractAsyncContextManager): """ - Essentially a dynamic try-except context manager to handle exceptions raised by a aiohttp handler, roughly: + + Essentially a dynamic try-except context manager to + handle exceptions raised by a aiohttp handler, i.e. roughly ``` - try: + try: - resp = await handler(request) + resp = await handler(request) - except exc_type1 as exc1: - resp = await exc_handler1(request) - except exc_type2 as exc1: - resp = await exc_handler2(request) + except exc_type1 as exc1: + resp = await exc_handler1(request) + except exc_type2 as exc1: + resp = await exc_handler2(request) - # except ... dynamically as in `exception_handlers_map` + # and so on ... as in `exception_handlers_map[exc_type] == exc_handler` ``` + """ def __init__( @@ -122,6 +125,8 @@ def get_response(self): def exception_handling_decorator( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ): + """ """ + def _decorator(handler: WebHandler): @functools.wraps(handler) async def _wrapper(request: web.Request) -> web.StreamResponse: From c8649b3a7954d2d6b4d6171f3fa65038d35053b1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:51:35 +0100 Subject: [PATCH 33/59] rename --- .../{exceptions_handlers.py => exception_handling.py} | 4 ++-- ...ons_handlers_base.py => exception_handling_base.py} | 10 +++------- ...rs_http_error_map.py => exception_handling_http.py} | 2 +- .../folders/_exceptions_handlers.py | 2 +- .../projects/_trash_handlers.py | 2 +- .../workspaces/_exceptions_handlers.py | 2 +- .../tests/unit/isolated/test_exception_handling.py | 0 .../unit/isolated/test_exceptions_handlers_base.py | 2 +- .../test_exceptions_handlers_http_error_map.py | 4 ++-- 9 files changed, 12 insertions(+), 16 deletions(-) rename services/web/server/src/simcore_service_webserver/{exceptions_handlers.py => exception_handling.py} (68%) rename services/web/server/src/simcore_service_webserver/{exceptions_handlers_base.py => exception_handling_base.py} (99%) rename services/web/server/src/simcore_service_webserver/{exceptions_handlers_http_error_map.py => exception_handling_http.py} (98%) create mode 100644 services/web/server/tests/unit/isolated/test_exception_handling.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/exception_handling.py similarity index 68% rename from services/web/server/src/simcore_service_webserver/exceptions_handlers.py rename to services/web/server/src/simcore_service_webserver/exception_handling.py index 909157b1e160..6c4ba31f2169 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling.py @@ -1,5 +1,5 @@ -from .exceptions_handlers_base import exception_handling_decorator -from .exceptions_handlers_http_error_map import ( +from .exception_handling_base import exception_handling_decorator +from .exception_handling_http import ( ExceptionToHttpErrorMap, HttpErrorInfo, to_exceptions_handlers_map, diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py similarity index 99% rename from services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py rename to services/web/server/src/simcore_service_webserver/exception_handling_base.py index d159dc2abe16..1c7910086ead 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -11,10 +11,6 @@ _logger = logging.getLogger(__name__) -# -# Definition -# - class AiohttpExceptionHandler(Protocol): __name__: str @@ -34,6 +30,9 @@ async def __call__( ... # pylint: disable=unnecessary-ellipsis +ExceptionHandlersMap: TypeAlias = dict[type[BaseException], AiohttpExceptionHandler] + + def _sort_exceptions_by_specificity( exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True ) -> list[type[BaseException]]: @@ -44,9 +43,6 @@ def _sort_exceptions_by_specificity( ) -ExceptionHandlersMap: TypeAlias = dict[type[BaseException], AiohttpExceptionHandler] - - class ExceptionHandlingContextManager(AbstractAsyncContextManager): """ diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py b/services/web/server/src/simcore_service_webserver/exception_handling_http.py similarity index 98% rename from services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py rename to services/web/server/src/simcore_service_webserver/exception_handling_http.py index d524234c368e..cd710565719d 100644 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_http_error_map.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_http.py @@ -10,7 +10,7 @@ from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error, is_error -from .exceptions_handlers_base import AiohttpExceptionHandler, ExceptionHandlersMap +from .exception_handling_base import AiohttpExceptionHandler, ExceptionHandlersMap _logger = logging.getLogger(__name__) diff --git a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py index 849fcf256e3e..5d98db3647d1 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_exceptions_handlers.py @@ -2,7 +2,7 @@ from servicelib.aiohttp import status -from ..exceptions_handlers import ( +from ..exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo, exception_handling_decorator, diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py index 720fb05218e4..78fa6ce2bdc8 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_handlers.py @@ -9,7 +9,7 @@ from .._meta import API_VTAG as VTAG from ..application_settings_utils import requires_dev_feature_enabled -from ..exceptions_handlers import ( +from ..exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo, exception_handling_decorator, diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py index d72cf7a42446..1bb16355b80f 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_exceptions_handlers.py @@ -2,7 +2,7 @@ from servicelib.aiohttp import status -from ..exceptions_handlers import ( +from ..exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo, exception_handling_decorator, diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py index 1fdc4125275e..0435893c5cec 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py @@ -10,7 +10,7 @@ from aiohttp import web from aiohttp.test_utils import make_mocked_request from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exceptions_handlers_base import ( +from simcore_service_webserver.exception_handling_base import ( AiohttpExceptionHandler, ExceptionHandlingContextManager, _sort_exceptions_by_specificity, diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py index 330b8500aa06..cf2095425862 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py @@ -14,11 +14,11 @@ from servicelib.aiohttp import status from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exceptions_handlers_base import ( +from simcore_service_webserver.exception_handling_base import ( ExceptionHandlingContextManager, exception_handling_decorator, ) -from simcore_service_webserver.exceptions_handlers_http_error_map import ( +from simcore_service_webserver.exception_handling_http import ( ExceptionToHttpErrorMap, HttpErrorInfo, create_exception_handler_from_http_info, From d94761c9cceb4cf57099bacbb71156b72bcdf0b9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:55:06 +0100 Subject: [PATCH 34/59] rename --- .../exception_handling.py | 2 +- ..._http.py => exception_handling_factory.py} | 28 +++++++++---------- ...test_exceptions_handlers_http_error_map.py | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) rename services/web/server/src/simcore_service_webserver/{exception_handling_http.py => exception_handling_factory.py} (100%) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling.py b/services/web/server/src/simcore_service_webserver/exception_handling.py index 6c4ba31f2169..615367b2ca52 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling.py @@ -1,5 +1,5 @@ from .exception_handling_base import exception_handling_decorator -from .exception_handling_http import ( +from .exception_handling_factory import ( ExceptionToHttpErrorMap, HttpErrorInfo, to_exceptions_handlers_map, diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_http.py b/services/web/server/src/simcore_service_webserver/exception_handling_factory.py similarity index 100% rename from services/web/server/src/simcore_service_webserver/exception_handling_http.py rename to services/web/server/src/simcore_service_webserver/exception_handling_factory.py index cd710565719d..28c6ac6177b4 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_http.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_factory.py @@ -15,15 +15,9 @@ _logger = logging.getLogger(__name__) -def create_error_response(error: ErrorGet, status_code: int) -> web.Response: - assert is_error(status_code), f"{status_code=} must be an error [{error=}]" # nosec - - return web.json_response( - data={"error": error.model_dump(exclude_unset=True, mode="json")}, - dumps=json_dumps, - reason=error.msg, - status=status_code, - ) +_STATUS_CODE_TO_HTTP_ERRORS: dict[ + int, type[web.HTTPError] +] = get_all_aiohttp_http_exceptions(web.HTTPError) class _DefaultDict(dict): @@ -41,6 +35,17 @@ class HttpErrorInfo(NamedTuple): ExceptionToHttpErrorMap: TypeAlias = dict[type[BaseException], HttpErrorInfo] +def create_error_response(error: ErrorGet, status_code: int) -> web.Response: + assert is_error(status_code), f"{status_code=} must be an error [{error=}]" # nosec + + return web.json_response( + data={"error": error.model_dump(exclude_unset=True, mode="json")}, + dumps=json_dumps, + reason=error.msg, + status=status_code, + ) + + def create_exception_handler_from_http_info( status_code: int, msg_template: str, @@ -114,11 +119,6 @@ def to_exceptions_handlers_map( return exc_handlers_map -_STATUS_CODE_TO_HTTP_ERRORS: dict[ - int, type[web.HTTPError] -] = get_all_aiohttp_http_exceptions(web.HTTPError) - - def create_http_error_exception_handlers_map(): """ Creates handles for all web.HTTPError diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py index cf2095425862..e9ead8473dda 100644 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py +++ b/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py @@ -18,7 +18,7 @@ ExceptionHandlingContextManager, exception_handling_decorator, ) -from simcore_service_webserver.exception_handling_http import ( +from simcore_service_webserver.exception_handling_factory import ( ExceptionToHttpErrorMap, HttpErrorInfo, create_exception_handler_from_http_info, From 5d03bfbf61d3a6bf151ae4508013249995edc8da Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:39:54 +0100 Subject: [PATCH 35/59] tests usage decorators --- .../unit/isolated/test_exception_handling.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index e69de29bb2d1..e63d91e4fbed 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -0,0 +1,71 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +from collections.abc import Callable + +from aiohttp import web +from servicelib.aiohttp import status +from simcore_service_webserver.exception_handling import exception_handling_decorator + + +async def test_handling_exceptions_decorating_a_route(aiohttp_client: Callable): + + # custom exception handler + async def value_error_as_422( + request: web.Request, exception: BaseException + ) -> web.Response: + return web.json_response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) + + # decorator + exc_handling = exception_handling_decorator({ValueError: value_error_as_422}) + + # adding new routes + routes = web.RouteTableDef() + + @routes.get("/{what}") + @exc_handling # < ----- using decorator + async def _handler(request: web.Request): + match request.match_info["what"]: + case "ValueError": + raise ValueError # handled + case "IndexError": + raise IndexError # not-handled + case "HTTPConflict": + raise web.HTTPConflict # not-handled + case "HTTPOk": + # non errors should NOT be raised, + # SEE https://github.com/ITISFoundation/osparc-simcore/pull/6829 + # but if it is so ... + raise web.HTTPOk # not-handled + + return web.Response() + + app = web.Application() + app.add_routes(routes) + + # testing from the client side + client = await aiohttp_client(app) + + # success + resp = await client.get("/ok") + assert resp.status == status.HTTP_200_OK + + # handled non-HTTPException exception + resp = await client.get("/ValueError") + assert resp.status == status.HTTP_422_UNPROCESSABLE_ENTITY + + # undhandled non-HTTPException + resp = await client.get("/IndexError") + assert resp.status == status.HTTP_500_INTERNAL_SERVER_ERROR + + # undhandled HTTPError + resp = await client.get("/HTTPConflict") + assert resp.status == status.HTTP_409_CONFLICT + + # undhandled HTTPSuccess + resp = await client.get("/HTTPOk") + assert resp.status == status.HTTP_200_OK From 047f0c540842141217d9ac5fa0fc2c4d51a77b84 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:01:58 +0100 Subject: [PATCH 36/59] rest errors --- .../src/models_library/rest_error.py | 23 +++++++++++++++++++ .../exception_handling_factory.py | 6 ++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/models-library/src/models_library/rest_error.py b/packages/models-library/src/models_library/rest_error.py index 3ba2475c9b3a..2a9c02170b8d 100644 --- a/packages/models-library/src/models_library/rest_error.py +++ b/packages/models-library/src/models_library/rest_error.py @@ -63,6 +63,29 @@ def from_error(cls, err: BaseException): ) +@dataclass +class LogMessageType: + # NOTE: deprecated! + message: str + level: str = "INFO" + logger: str = "user" + + +@dataclass +class ErrorItemType: + # NOTE: deprecated! + code: str + message: str + resource: str | None + field: str | None + + @classmethod + def from_error(cls, err: BaseException): + return cls( + code=err.__class__.__name__, message=str(err), resource=None, field=None + ) + + class ErrorGet(BaseModel): message: Annotated[ str, 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 28c6ac6177b4..804b9cec5a4d 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 @@ -41,7 +41,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=error.msg, + reason=error.message, status=status_code, ) @@ -81,7 +81,7 @@ async def _exception_handler( _DefaultDict(getattr(exception, "__dict__", {})) ) - error = ErrorGet(msg=user_msg) + error = ErrorGet(message=user_msg) if is_5xx_server_error(status_code): oec = create_error_code(exception) @@ -98,7 +98,7 @@ async def _exception_handler( }, ) ) - error = ErrorGet(msg=user_msg, support_id=IDStr(oec)) + error = ErrorGet(message=user_msg, support_id=IDStr(oec)) return create_error_response(error, status_code=status_code) From 112922e458ac0c66da00a475a252a3c96370b688 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:17:44 +0100 Subject: [PATCH 37/59] rename --- ...xceptions_handlers_base.py => test_exception_handling_base.py} | 0 ...dlers_http_error_map.py => test_exception_handling_factory.py} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename services/web/server/tests/unit/isolated/{test_exceptions_handlers_base.py => test_exception_handling_base.py} (100%) rename services/web/server/tests/unit/isolated/{test_exceptions_handlers_http_error_map.py => test_exception_handling_factory.py} (100%) diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py b/services/web/server/tests/unit/isolated/test_exception_handling_base.py similarity index 100% rename from services/web/server/tests/unit/isolated/test_exceptions_handlers_base.py rename to services/web/server/tests/unit/isolated/test_exception_handling_base.py diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py b/services/web/server/tests/unit/isolated/test_exception_handling_factory.py similarity index 100% rename from services/web/server/tests/unit/isolated/test_exceptions_handlers_http_error_map.py rename to services/web/server/tests/unit/isolated/test_exception_handling_factory.py From 1347afadec496ab405bb3ea94cc0646fb0155100 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:22:01 +0100 Subject: [PATCH 38/59] cleanup --- .../tests/unit/isolated/test_exception_handling.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index e63d91e4fbed..5124f9a0a6b2 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -15,19 +15,23 @@ async def test_handling_exceptions_decorating_a_route(aiohttp_client: Callable): # custom exception handler - async def value_error_as_422( + async def _value_error_as_422( request: web.Request, exception: BaseException ) -> web.Response: return web.json_response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) - # decorator - exc_handling = exception_handling_decorator({ValueError: value_error_as_422}) + # 1. create decorator + exc_handling = exception_handling_decorator( + exception_handlers_map={ + ValueError: _value_error_as_422, + } + ) # adding new routes routes = web.RouteTableDef() @routes.get("/{what}") - @exc_handling # < ----- using decorator + @exc_handling # < ----- 2. using decorator async def _handler(request: web.Request): match request.match_info["what"]: case "ValueError": @@ -47,7 +51,7 @@ async def _handler(request: web.Request): app = web.Application() app.add_routes(routes) - # testing from the client side + # 3. testing from the client side client = await aiohttp_client(app) # success From 5dd12965b6bf7df5dc4cd30792e57592d0d2a23c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:22:57 +0100 Subject: [PATCH 39/59] drop base2 --- .../exceptions_handlers_base_2.py | 162 ------------------ .../test_exceptions_handlers_base_2.py | 75 -------- 2 files changed, 237 deletions(-) delete mode 100644 services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py delete mode 100644 services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py diff --git a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py b/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py deleted file mode 100644 index 466d34968359..000000000000 --- a/services/web/server/src/simcore_service_webserver/exceptions_handlers_base_2.py +++ /dev/null @@ -1,162 +0,0 @@ -import functools -from collections.abc import MutableMapping -from http import HTTPStatus -from typing import Any, Protocol, TypeAlias, cast - -from aiohttp import web -from servicelib.aiohttp.typing_extension import Handler -from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions - -# Defines exception handler as somethign that returns responses, as fastapi, and not new exceptions! -# in reality this can be reinterpreted in aiohttp since all responses can be represented as exceptions. -# Not true because fastapi.HTTPException does actually the same! Better responses because this weay we do not -# need return None or the exception itself which as we saw in the tests, it causes troubles! -# - - -class ExceptionHandler(Protocol): - # Based on concept in https://www.starlette.io/exceptions/ - __name__: str - - # TODO: do not know how to define exc so that it accepts any exception! - async def __call__(self, request: web.Request, exc: BaseException) -> web.Response: - ... - - -ErrorsToHttpExceptionsMap: TypeAlias = dict[type[Exception], type[web.HTTPException]] - -ExceptionHandlerRegistry: TypeAlias = dict[type[Exception], ExceptionHandler] - - -# injects the exceptions in a scope, e.g. an app state or some container like routes/ but to use in a module only e.g. -# as decorator or context manager? - - -_EXCEPTIONS_HANDLERS_KEY = f"{__name__}._EXCEPTIONS_HANDLERS_KEY" -_EXCEPTIONS_MAP_KEY = f"{__name__}._EXCEPTIONS_MAP_KEY" - - -def setup_exceptions_handlers(registry: MutableMapping): - # init registry in the scope - registry[_EXCEPTIONS_HANDLERS_KEY] = {} - registry[_EXCEPTIONS_MAP_KEY] = {} - # but this is very specific because it responds with only status! you migh want to have different - # type of bodies, etc - - -def _get_exception_handlers( - registry: MutableMapping, -) -> ExceptionHandlerRegistry: - return registry.get(_EXCEPTIONS_HANDLERS_KEY, {}) - - -def add_exception_handler( - registry: MutableMapping, - exc_class: type[Exception], - handler: ExceptionHandler, -): - """ - Registers in the scope an exception type to a handler - """ - registry[_EXCEPTIONS_HANDLERS_KEY][exc_class] = handler - - -_STATUS_CODE_TO_HTTP_EXCEPTIONS: dict[ - int, type[web.HTTPException] -] = get_all_aiohttp_http_exceptions(web.HTTPException) - - -def _create_exception_handler_mapper( - exc_class: type[Exception], - http_exc_class: type[web.HTTPException], -) -> ExceptionHandler: - error_code = f"{exc_class.__name__}" # status_code.error_code - - async def _exception_handler( - request: web.Request, exc: BaseException - ) -> web.Response: - # TODO: a better way to add error_code. TODO: create the envelope! - assert request # nosec - return http_exc_class(reason=f"{exc} [{error_code}]") - - return _exception_handler - - -def add_exception_mapper( - registry: MutableMapping, exception_class: type[Exception], status_code: int -): - """ - Create an exception handlers by mapping a class to an HTTPException - and registers it in the scope - """ - try: - http_exception_cls = _STATUS_CODE_TO_HTTP_EXCEPTIONS[status_code] - except KeyError as err: - msg = f"Invalid status code. Got {status_code=}" - raise ValueError(msg) from err - - # adds exception handler to scope - registry[_EXCEPTIONS_MAP_KEY][exception_class] = http_exception_cls - add_exception_handler( - registry, - exception_class, - handler=_create_exception_handler_mapper(exception_class, http_exception_cls), - ) - - -# ---------- - - -async def handle_request_with_exception_handling_in_scope( - # Create using contextlib.contextmanager - # FIXME: !!! - handler: Handler, - request: web.Request, - scope: MutableMapping[str, Any] | None = None, -) -> web.Response: - try: - resp = await handler(request) - return cast(web.Response, resp) - - except Exception as exc: # pylint: disable=broad-exception-caught - scope = scope or request.app - if exception_handler := _get_exception_handlers(scope).get(type(exc), None): - resp = await exception_handler(request, exc) - else: - resp = web.HTTPInternalServerError() - - if isinstance(resp, web.HTTPError): - # NOTE: this should not happen anymore! as far as I understand!? - raise resp from exc - return resp - - -# decorator -def handle_registered_exceptions(registry: MutableMapping[str, Any] | None = None): - def _decorator(handler: Handler): - @functools.wraps(handler) - async def _wrapper(request: web.Request) -> web.Response: - return await handle_request_with_exception_handling_in_scope( - handler, request, registry - ) - - return _wrapper - - return _decorator - - -def openapi_error_responses( - # If I have all the status codes mapped, I can definitively use that info to create `responses` - # for fastapi to render the OAS preoperly - exceptions_map: ErrorsToHttpExceptionsMap, -) -> dict[HTTPStatus, dict[str, Any]]: - responses = {} - - for exc_class, http_exc_class in exceptions_map.items(): - status_code = HTTPStatus(http_exc_class.status_code) - if status_code not in responses: - responses[status_code] = {"description": f"{exc_class.__name__}"} - else: - responses[status_code]["description"] += f", {exc_class.__name__}" - - return responses diff --git a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py b/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py deleted file mode 100644 index 91fb0a3a9953..000000000000 --- a/services/web/server/tests/unit/isolated/test_exceptions_handlers_base_2.py +++ /dev/null @@ -1,75 +0,0 @@ -# pylint: disable=protected-access -# pylint: disable=redefined-outer-name -# pylint: disable=too-many-arguments -# pylint: disable=too-many-statements -# pylint: disable=unused-argument -# pylint: disable=unused-variable - - -import pytest -from aiohttp import web -from aiohttp.test_utils import make_mocked_request -from servicelib.aiohttp import status -from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exceptions_handlers_base_2 import ( - add_exception_handler, - add_exception_mapper, - handle_registered_exceptions, - setup_exceptions_handlers, -) - -# Some custom errors in my service - - -class BasePluginError(WebServerBaseError): - ... - - -class OneError(BasePluginError): - ... - - -class OtherError(BasePluginError): - ... - - -@pytest.fixture -def fake_request() -> web.Request: - return make_mocked_request("GET", "/foo") - - -def test_it(): - - app = web.Application() - - async def my_error_handler(request: web.Request, exc: BaseException): - assert isinstance(exc, OneError) - return web.HTTPNotFound() - - # create register - setup_exceptions_handlers(app) - - # register exception handler - add_exception_handler(app, OneError, my_error_handler) - - # mapper is defined as a higher abstraction - # that automatically produces an exception handler - - # this is a handler create mapping to status_code and reason - add_exception_mapper(app, OtherError, status.HTTP_404_NOT_FOUND) - - # NOTE: that we respond always the same way to errors, i.e. using Error model - - # shouldn't this be automatic? - add_exception_mapper(app, web.HTTPNotFound, status.HTTP_404_NOT_FOUND) - - async def foo(): - raise OneError - - routes = web.RouteTableDef() - - @routes.get("/home") - @handle_registered_exceptions() - async def home(_request: web.Request): - await foo() - return web.HTTPOk() From 978e105ff4259ca676d2732cb1ac5eb2daaa64e6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:43:22 +0100 Subject: [PATCH 40/59] cleanup --- .../exception_handling.py | 3 +- .../exception_handling_base.py | 14 ++-- .../unit/isolated/test_exception_handling.py | 65 +++++++++++++++---- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling.py b/services/web/server/src/simcore_service_webserver/exception_handling.py index 615367b2ca52..98f7c3022491 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling.py @@ -1,4 +1,4 @@ -from .exception_handling_base import exception_handling_decorator +from .exception_handling_base import ExceptionHandlersMap, exception_handling_decorator from .exception_handling_factory import ( ExceptionToHttpErrorMap, HttpErrorInfo, @@ -6,6 +6,7 @@ ) __all__: tuple[str, ...] = ( + "ExceptionHandlersMap", "ExceptionToHttpErrorMap", "HttpErrorInfo", "exception_handling_decorator", diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index 1c7910086ead..cf16076739cd 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -44,10 +44,8 @@ def _sort_exceptions_by_specificity( class ExceptionHandlingContextManager(AbstractAsyncContextManager): - """ - - Essentially a dynamic try-except context manager to - handle exceptions raised by a aiohttp handler, i.e. roughly + """Essentially a dynamic try-except context manager to + handle exceptions raised by a web handler, i.e. ``` try: @@ -121,7 +119,10 @@ def get_response(self): def exception_handling_decorator( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ): - """ """ + """Creates a decorator to handle all registered exceptions raised in a given route handler + + SEE usage example in test_exception_handling + """ def _decorator(handler: WebHandler): @functools.wraps(handler) @@ -145,6 +146,9 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: def exception_handling_middleware( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ) -> WebMiddleware: + """ + Creates a middleware to handle all registered exceptions raised in any app's route + """ _handle_excs = exception_handling_decorator( exception_handlers_map=exception_handlers_map ) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index 5124f9a0a6b2..524f121c8807 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -7,25 +7,62 @@ from collections.abc import Callable +import pytest from aiohttp import web +from models_library.rest_error import ErrorGet from servicelib.aiohttp import status -from simcore_service_webserver.exception_handling import exception_handling_decorator +from simcore_service_webserver.exception_handling import ( + ExceptionHandlersMap, + HttpErrorInfo, + exception_handling_decorator, + to_exceptions_handlers_map, +) + + +@pytest.fixture +def exception_handlers_map(build_method: str) -> ExceptionHandlersMap: + """ + Two different ways to build the exception_handlers_map + """ + exception_handlers_map: ExceptionHandlersMap = {} + + if build_method == "custom": + + async def _value_error_as_422( + request: web.Request, exception: BaseException + ) -> web.Response: + # custom exception handler + return web.json_response( + reason=f"{build_method=}", status=status.HTTP_422_UNPROCESSABLE_ENTITY + ) + + exception_handlers_map = { + ValueError: _value_error_as_422, + } + + elif build_method == "http_map": + exception_handlers_map = to_exceptions_handlers_map( + { + ValueError: HttpErrorInfo( + status.HTTP_422_UNPROCESSABLE_ENTITY, f"{build_method=}" + ) + } + ) + else: + pytest.fail(f"Undefined {build_method=}") + return exception_handlers_map -async def test_handling_exceptions_decorating_a_route(aiohttp_client: Callable): - # custom exception handler - async def _value_error_as_422( - request: web.Request, exception: BaseException - ) -> web.Response: - return web.json_response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) +@pytest.mark.parametrize("build_method", ["custom", "http_map"]) +async def test_handling_exceptions_decorating_a_route( + aiohttp_client: Callable, + exception_handlers_map: ExceptionHandlersMap, + build_method: str, +): # 1. create decorator - exc_handling = exception_handling_decorator( - exception_handlers_map={ - ValueError: _value_error_as_422, - } - ) + exc_handling = exception_handling_decorator(exception_handlers_map) # adding new routes routes = web.RouteTableDef() @@ -61,6 +98,10 @@ async def _handler(request: web.Request): # handled non-HTTPException exception resp = await client.get("/ValueError") assert resp.status == status.HTTP_422_UNPROCESSABLE_ENTITY + if build_method == "http_map": + body = await resp.json() + error = ErrorGet.model_validate(body["error"]) + assert error.message == f"{build_method=}" # undhandled non-HTTPException resp = await client.get("/IndexError") From 750c47247d6c6befd9ae401927481dccd1802afe Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 16:04:59 +0100 Subject: [PATCH 41/59] mypy --- .../exception_handling_base.py | 4 +- .../exception_handling_factory.py | 5 +-- .../unit/isolated/test_exception_handling.py | 42 +++++++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index cf16076739cd..31a18022a22c 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -112,7 +112,7 @@ async def __aexit__( return True # suppress return False # reraise - def get_response(self): + def get_response(self) -> web.Response | None: return self._response @@ -126,7 +126,7 @@ def exception_handling_decorator( def _decorator(handler: WebHandler): @functools.wraps(handler) - async def _wrapper(request: web.Request) -> web.StreamResponse: + async def _wrapper(request: web.Request): cm = ExceptionHandlingContextManager( exception_handlers_map, request=request ) 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 804b9cec5a4d..ffa233ff9ec9 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 @@ -4,7 +4,6 @@ from aiohttp import web from common_library.error_codes import create_error_code from common_library.json_serialization import json_dumps -from models_library.basic_types import IDStr from models_library.rest_error import ErrorGet from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions from servicelib.logging_errors import create_troubleshotting_log_kwargs @@ -81,7 +80,7 @@ async def _exception_handler( _DefaultDict(getattr(exception, "__dict__", {})) ) - error = ErrorGet(message=user_msg) + error = ErrorGet.model_construct(message=user_msg) if is_5xx_server_error(status_code): oec = create_error_code(exception) @@ -98,7 +97,7 @@ async def _exception_handler( }, ) ) - error = ErrorGet(message=user_msg, support_id=IDStr(oec)) + error = ErrorGet.model_construct(message=user_msg, support_id=oec) return create_error_response(error, status_code=status_code) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index 524f121c8807..9322543d5e37 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -17,6 +17,9 @@ exception_handling_decorator, to_exceptions_handlers_map, ) +from simcore_service_webserver.exception_handling_base import ( + exception_handling_middleware, +) @pytest.fixture @@ -114,3 +117,42 @@ async def _handler(request: web.Request): # undhandled HTTPSuccess resp = await client.get("/HTTPOk") assert resp.status == status.HTTP_200_OK + + +@pytest.mark.parametrize("build_method", ["custom", "http_map"]) +async def test_handling_exceptions_with_middelware( + aiohttp_client: Callable, + exception_handlers_map: ExceptionHandlersMap, + build_method: str, +): + # adding new routes + routes = web.RouteTableDef() + + @routes.get("/{what}") # NO decorantor now + async def _handler(request: web.Request): + match request.match_info["what"]: + case "ValueError": + raise ValueError # handled + return web.Response() + + app = web.Application() + app.add_routes(routes) + + # 1. create & install middleware + exc_handling = exception_handling_middleware(exception_handlers_map) + app.middlewares.append(exc_handling) + + # 2. testing from the client side + client = await aiohttp_client(app) + + # success + resp = await client.get("/ok") + assert resp.status == status.HTTP_200_OK + + # handled non-HTTPException exception + resp = await client.get("/ValueError") + assert resp.status == status.HTTP_422_UNPROCESSABLE_ENTITY + if build_method == "http_map": + body = await resp.json() + error = ErrorGet.model_validate(body["error"]) + assert error.message == f"{build_method=}" From d83a587dc9b8a5a0c91593f138e290eabcc1184d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 16:37:54 +0100 Subject: [PATCH 42/59] cleanup --- .../exception_handling_base.py | 23 ++++-- .../unit/isolated/test_exception_handling.py | 79 ++++++++++++++----- 2 files changed, 75 insertions(+), 27 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index 31a18022a22c..adb25aa29502 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -44,8 +44,10 @@ def _sort_exceptions_by_specificity( class ExceptionHandlingContextManager(AbstractAsyncContextManager): - """Essentially a dynamic try-except context manager to - handle exceptions raised by a web handler, i.e. + """ + A dynamic try-except context manager for handling exceptions in web handlers. + Maps exception types to corresponding handlers, allowing structured error management, i.e. + essentially something like ``` try: @@ -55,10 +57,10 @@ class ExceptionHandlingContextManager(AbstractAsyncContextManager): resp = await exc_handler1(request) except exc_type2 as exc1: resp = await exc_handler2(request) + # etc - # and so on ... as in `exception_handlers_map[exc_type] == exc_handler` ``` - + and `exception_handlers_map` defines the mapping of exception types (`exc_type*`) to their handlers (`exc_handler*`). """ def __init__( @@ -113,15 +115,19 @@ async def __aexit__( return False # reraise def get_response(self) -> web.Response | None: + """ + Returns the response generated by the exception handler, if an exception was handled. Otherwise None + """ return self._response def exception_handling_decorator( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ): - """Creates a decorator to handle all registered exceptions raised in a given route handler + """Creates a decorator to manage exceptions raised in a given route handler. + Ensures consistent exception management across decorated handlers. - SEE usage example in test_exception_handling + SEE examples test_exception_handling """ def _decorator(handler: WebHandler): @@ -146,8 +152,9 @@ async def _wrapper(request: web.Request): def exception_handling_middleware( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] ) -> WebMiddleware: - """ - Creates a middleware to handle all registered exceptions raised in any app's route + """Constructs middleware to handle exceptions raised across app routes + + SEE examples test_exception_handling """ _handle_excs = exception_handling_decorator( exception_handlers_map=exception_handlers_map diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index 9322543d5e37..ee27a4b033e8 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -11,6 +11,7 @@ from aiohttp import web from models_library.rest_error import ErrorGet from servicelib.aiohttp import status +from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON, MIMETYPE_TEXT_PLAIN from simcore_service_webserver.exception_handling import ( ExceptionHandlersMap, HttpErrorInfo, @@ -20,6 +21,11 @@ from simcore_service_webserver.exception_handling_base import ( exception_handling_middleware, ) +from simcore_service_webserver.exception_handling_factory import ( + create_http_error_exception_handlers_map, +) + +from services.web.server.tests.conftest import TestClient @pytest.fixture @@ -29,9 +35,9 @@ def exception_handlers_map(build_method: str) -> ExceptionHandlersMap: """ exception_handlers_map: ExceptionHandlersMap = {} - if build_method == "custom": + if build_method == "function": - async def _value_error_as_422( + async def _value_error_as_422_func( request: web.Request, exception: BaseException ) -> web.Response: # custom exception handler @@ -40,7 +46,7 @@ async def _value_error_as_422( ) exception_handlers_map = { - ValueError: _value_error_as_422, + ValueError: _value_error_as_422_func, } elif build_method == "http_map": @@ -57,7 +63,7 @@ async def _value_error_as_422( return exception_handlers_map -@pytest.mark.parametrize("build_method", ["custom", "http_map"]) +@pytest.mark.parametrize("build_method", ["function", "http_map"]) async def test_handling_exceptions_decorating_a_route( aiohttp_client: Callable, exception_handlers_map: ExceptionHandlersMap, @@ -70,10 +76,11 @@ async def test_handling_exceptions_decorating_a_route( # adding new routes routes = web.RouteTableDef() - @routes.get("/{what}") + @routes.post("/{what}") @exc_handling # < ----- 2. using decorator async def _handler(request: web.Request): - match request.match_info["what"]: + what = request.match_info["what"] + match what: case "ValueError": raise ValueError # handled case "IndexError": @@ -86,20 +93,20 @@ async def _handler(request: web.Request): # but if it is so ... raise web.HTTPOk # not-handled - return web.Response() + return web.Response(text=what) app = web.Application() app.add_routes(routes) # 3. testing from the client side - client = await aiohttp_client(app) + client: TestClient = await aiohttp_client(app) # success - resp = await client.get("/ok") + resp = await client.post("/ok") assert resp.status == status.HTTP_200_OK # handled non-HTTPException exception - resp = await client.get("/ValueError") + resp = await client.post("/ValueError") assert resp.status == status.HTTP_422_UNPROCESSABLE_ENTITY if build_method == "http_map": body = await resp.json() @@ -107,28 +114,27 @@ async def _handler(request: web.Request): assert error.message == f"{build_method=}" # undhandled non-HTTPException - resp = await client.get("/IndexError") + resp = await client.post("/IndexError") assert resp.status == status.HTTP_500_INTERNAL_SERVER_ERROR # undhandled HTTPError - resp = await client.get("/HTTPConflict") + resp = await client.post("/HTTPConflict") assert resp.status == status.HTTP_409_CONFLICT # undhandled HTTPSuccess - resp = await client.get("/HTTPOk") + resp = await client.post("/HTTPOk") assert resp.status == status.HTTP_200_OK -@pytest.mark.parametrize("build_method", ["custom", "http_map"]) +@pytest.mark.parametrize("build_method", ["function", "http_map"]) async def test_handling_exceptions_with_middelware( aiohttp_client: Callable, exception_handlers_map: ExceptionHandlersMap, build_method: str, ): - # adding new routes routes = web.RouteTableDef() - @routes.get("/{what}") # NO decorantor now + @routes.post("/{what}") # NO decorantor now async def _handler(request: web.Request): match request.match_info["what"]: case "ValueError": @@ -143,16 +149,51 @@ async def _handler(request: web.Request): app.middlewares.append(exc_handling) # 2. testing from the client side - client = await aiohttp_client(app) + client: TestClient = await aiohttp_client(app) # success - resp = await client.get("/ok") + resp = await client.post("/ok") assert resp.status == status.HTTP_200_OK # handled non-HTTPException exception - resp = await client.get("/ValueError") + resp = await client.post("/ValueError") assert resp.status == status.HTTP_422_UNPROCESSABLE_ENTITY if build_method == "http_map": body = await resp.json() error = ErrorGet.model_validate(body["error"]) assert error.message == f"{build_method=}" + + +@pytest.mark.parametrize("with_middleware", [True, False]) +async def test_raising_aiohttp_http_errors( + aiohttp_client: Callable, with_middleware: bool +): + routes = web.RouteTableDef() + + @routes.post("/raise-http-error") + async def _handler1(request: web.Request): + # 1. raises aiohttp.web_exceptions.HttpError + raise web.HTTPConflict + + app = web.Application() + app.add_routes(routes) + + # 2. create & install middleware handlers for ALL http (optional) + if with_middleware: + exc_handling = exception_handling_middleware( + exception_handlers_map=create_http_error_exception_handlers_map() + ) + app.middlewares.append(exc_handling) + + # 3. testing from the client side + client: TestClient = await aiohttp_client(app) + + resp = await client.post("/raise-http-error") + assert resp.status == status.HTTP_409_CONFLICT + + if with_middleware: + assert resp.content_type == MIMETYPE_APPLICATION_JSON + ErrorGet.model_construct((await resp.json())["error"]) + else: + # default + assert resp.content_type == MIMETYPE_TEXT_PLAIN From 3a4d80088f28a1b86d815d049c13541a78b399d7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 17:06:34 +0100 Subject: [PATCH 43/59] eneveloped error --- api/specs/web-server/_folders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/specs/web-server/_folders.py b/api/specs/web-server/_folders.py index 88a2b19ce9e8..a7c511f6636f 100644 --- a/api/specs/web-server/_folders.py +++ b/api/specs/web-server/_folders.py @@ -9,7 +9,7 @@ from typing import Annotated -from _common import as_query +from _common import EnvelopeE, as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.folders_v2 import ( FolderCreateBodyParams, From f46e52ee0cd0ec4546f464110da27815c45686ed Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 17:11:36 +0100 Subject: [PATCH 44/59] fixes --- .../simcore_service_webserver/api/v0/openapi.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 33979c6bf3dd..384ddd220c86 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -7768,6 +7768,19 @@ components: additionalProperties: false type: object title: EmptyModel + EnvelopeE_ErrorGet_: + properties: + error: + anyOf: + - $ref: '#/components/schemas/ErrorGet' + - type: 'null' + data: + anyOf: + - {} + - type: 'null' + title: Data + type: object + title: EnvelopeE[ErrorGet] Envelope_AppStatusCheck_: properties: data: From 91bc1fcb069bea622bb58495ba57de50357c97df Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 17:22:40 +0100 Subject: [PATCH 45/59] updates workslpaces --- api/specs/web-server/_workspaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/specs/web-server/_workspaces.py b/api/specs/web-server/_workspaces.py index fce290fffb0c..bf750cd92204 100644 --- a/api/specs/web-server/_workspaces.py +++ b/api/specs/web-server/_workspaces.py @@ -9,7 +9,7 @@ from enum import Enum from typing import Annotated -from _common import as_query +from _common import EnvelopeE, as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.workspaces import ( WorkspaceCreateBodyParams, From d944f07c3c8a97399e935b7d5244394baa9cd03b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:12:20 +0100 Subject: [PATCH 46/59] wront import --- .../web/server/tests/unit/isolated/test_exception_handling.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index ee27a4b033e8..8da32c6f34c9 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -9,6 +9,7 @@ import pytest from aiohttp import web +from aiohttp.test_utils import TestClient from models_library.rest_error import ErrorGet from servicelib.aiohttp import status from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON, MIMETYPE_TEXT_PLAIN @@ -25,8 +26,6 @@ create_http_error_exception_handlers_map, ) -from services.web.server.tests.conftest import TestClient - @pytest.fixture def exception_handlers_map(build_method: str) -> ExceptionHandlersMap: From 5ad382f492f61bf8783a11ccbef6512d70a411c1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:17:51 +0100 Subject: [PATCH 47/59] fixes tests --- services/web/server/tests/unit/with_dbs/03/test_trash.py | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index 2489ea6107c2..76f4aefb46b8 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -133,7 +133,6 @@ async def test_trash_projects( # noqa: PLR0915 could_not_trash = is_project_running and not force if could_not_trash: - assert error["status"] == status.HTTP_409_CONFLICT assert "Current study is in use" in error["message"] # GET From c53b7bf42292f6b9d04641c9de9230b854788ece Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 27 Nov 2024 20:38:21 +0100 Subject: [PATCH 48/59] updates oas --- .../simcore_service_webserver/api/v0/openapi.yaml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 384ddd220c86..33979c6bf3dd 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -7768,19 +7768,6 @@ components: additionalProperties: false type: object title: EmptyModel - EnvelopeE_ErrorGet_: - properties: - error: - anyOf: - - $ref: '#/components/schemas/ErrorGet' - - type: 'null' - data: - anyOf: - - {} - - type: 'null' - title: Data - type: object - title: EnvelopeE[ErrorGet] Envelope_AppStatusCheck_: properties: data: From 135e3ce454207a26643e316f1c48f3707d84e734 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:12:40 +0100 Subject: [PATCH 49/59] fixes merge --- api/specs/web-server/_folders.py | 2 +- api/specs/web-server/_workspaces.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/specs/web-server/_folders.py b/api/specs/web-server/_folders.py index a7c511f6636f..88a2b19ce9e8 100644 --- a/api/specs/web-server/_folders.py +++ b/api/specs/web-server/_folders.py @@ -9,7 +9,7 @@ from typing import Annotated -from _common import EnvelopeE, as_query +from _common import as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.folders_v2 import ( FolderCreateBodyParams, diff --git a/api/specs/web-server/_workspaces.py b/api/specs/web-server/_workspaces.py index bf750cd92204..fce290fffb0c 100644 --- a/api/specs/web-server/_workspaces.py +++ b/api/specs/web-server/_workspaces.py @@ -9,7 +9,7 @@ from enum import Enum from typing import Annotated -from _common import EnvelopeE, as_query +from _common import as_query from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.workspaces import ( WorkspaceCreateBodyParams, From 382d94c9f204e1ba0d6c930c44801d33b4dd7239 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:16:35 +0100 Subject: [PATCH 50/59] cleanup --- .../src/simcore_service_webserver/exception_handling_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ffa233ff9ec9..b1481fbd7d6d 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 @@ -72,7 +72,7 @@ def create_exception_handler_from_http_info( async def _exception_handler( request: web.Request, - exception: BaseException, # TODO: for different type of exceptions e.g HTTPError + exception: BaseException, ) -> web.Response: # safe formatting, i.e. does not raise From 3e4ddd9dee26ca2a544fab05b3ca7fb23748a5ab Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:18:56 +0100 Subject: [PATCH 51/59] cleanup --- .../exception_handling_factory.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 b1481fbd7d6d..76a62e862871 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 @@ -107,7 +107,10 @@ async def _exception_handler( def to_exceptions_handlers_map( exc_to_http_error_map: ExceptionToHttpErrorMap, ) -> ExceptionHandlersMap: - """Converts { exc_type: (status, msg), ... } -> { exc_type: callable, ... }""" + """Data adapter to convert ExceptionToHttpErrorMap ot ExceptionHandlersMap, i.e. + - from { exc_type: (status, msg), ... } + - to { exc_type: callable, ... } + """ exc_handlers_map: ExceptionHandlersMap = { exc_type: create_exception_handler_from_http_info( status_code=info.status_code, msg_template=info.msg_template @@ -118,9 +121,9 @@ def to_exceptions_handlers_map( return exc_handlers_map -def create_http_error_exception_handlers_map(): +def create_http_error_exception_handlers_map() -> ExceptionHandlersMap: """ - Creates handles for all web.HTTPError + Auto create handlers for **all** web.HTTPError """ exc_handlers_map: ExceptionHandlersMap = { exc_type: create_exception_handler_from_http_info( From 9fc06bc98b95bc488a98176e53d89d0e2bdeb91e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 21:31:14 +0100 Subject: [PATCH 52/59] @sanderegg review: ellipis --- .../src/simcore_service_webserver/exception_handling_base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index adb25aa29502..83a67ee07716 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -27,7 +27,6 @@ async def __call__( request -- current request exception -- exception raised in web handler during this request """ - ... # pylint: disable=unnecessary-ellipsis ExceptionHandlersMap: TypeAlias = dict[type[BaseException], AiohttpExceptionHandler] From 9b10abb74fd11e097f9430d764e0647d08591430 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 21:34:47 +0100 Subject: [PATCH 53/59] @sanderegg review: doc --- .../src/simcore_service_webserver/exception_handling_base.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index 83a67ee07716..9d07ca218464 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -35,6 +35,10 @@ async def __call__( def _sort_exceptions_by_specificity( exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True ) -> list[type[BaseException]]: + """ + Keyword Arguments: + concrete_first -- If True, concrete subclasses precede their superclass (default: {True}). + """ return sorted( exceptions, key=lambda exc: sum(issubclass(e, exc) for e in exceptions if e is not exc), From 3454d9794f314db5bd5bd1c353bd15981e40f6f1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 21:38:17 +0100 Subject: [PATCH 54/59] @sanderegg review: rename --- .../simcore_service_webserver/exception_handling_base.py | 4 ++-- .../tests/unit/isolated/test_exception_handling_base.py | 4 ++-- .../tests/unit/isolated/test_exception_handling_factory.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index 9d07ca218464..4ede48434ff7 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -117,7 +117,7 @@ async def __aexit__( return True # suppress return False # reraise - def get_response(self) -> web.Response | None: + def get_response_or_none(self) -> web.Response | None: """ Returns the response generated by the exception handler, if an exception was handled. Otherwise None """ @@ -143,7 +143,7 @@ async def _wrapper(request: web.Request): return await handler(request) # If an exception was handled, return the exception handler's return value - response = cm.get_response() + response = cm.get_response_or_none() assert response is not None # nosec return response diff --git a/services/web/server/tests/unit/isolated/test_exception_handling_base.py b/services/web/server/tests/unit/isolated/test_exception_handling_base.py index 0435893c5cec..455abe6cefb2 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling_base.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling_base.py @@ -93,11 +93,11 @@ async def _concrete_exc_handler(request, exception): ) async with cm: raise OneError - assert cm.get_response() == expected_response + assert cm.get_response_or_none() == expected_response async with cm: raise OtherError - assert cm.get_response() == expected_response + assert cm.get_response_or_none() == expected_response # reraises with pytest.raises(ArithmeticError): 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 e9ead8473dda..666ececc1aca 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 @@ -79,7 +79,7 @@ async def test_handling_different_exceptions_with_context( async with cm: raise OneError - response = cm.get_response() + 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( @@ -93,14 +93,14 @@ async def test_handling_different_exceptions_with_context( async with cm: raise err - assert cm.get_response() is None + assert cm.get_response_or_none() is None assert err_info.value == err # handles as 5XX and logs async with cm: raise OtherError - response = cm.get_response() + 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( From de74e8658e21a870b52b8459f9626ed68ad8eaba Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 21:45:43 +0100 Subject: [PATCH 55/59] @sanderegg @GitHK review: annotations --- .../exception_handling_base.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index 4ede48434ff7..5871e5fe9dc0 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -1,6 +1,6 @@ import functools import logging -from collections.abc import Iterable +from collections.abc import Callable, Iterable from contextlib import AbstractAsyncContextManager from types import TracebackType from typing import Protocol, TypeAlias @@ -19,7 +19,7 @@ async def __call__( self, request: web.Request, exception: BaseException, - ) -> web.Response: + ) -> web.StreamResponse: """ Callback that handles an exception produced during a request and transforms it into a response @@ -77,7 +77,7 @@ def __init__( list(self._exc_handlers_map.keys()), concrete_first=True ) self._request: web.Request = request - self._response: web.Response | None = None + self._response: web.StreamResponse | None = None def _get_exc_handler_or_none( self, exc_type: type[BaseException], exc_value: BaseException @@ -117,7 +117,7 @@ async def __aexit__( return True # suppress return False # reraise - def get_response_or_none(self) -> web.Response | None: + def get_response_or_none(self) -> web.StreamResponse | None: """ Returns the response generated by the exception handler, if an exception was handled. Otherwise None """ @@ -126,7 +126,7 @@ def get_response_or_none(self) -> web.Response | None: def exception_handling_decorator( exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] -): +) -> Callable[[WebHandler], WebHandler]: """Creates a decorator to manage exceptions raised in a given route handler. Ensures consistent exception management across decorated handlers. @@ -135,7 +135,7 @@ def exception_handling_decorator( def _decorator(handler: WebHandler): @functools.wraps(handler) - async def _wrapper(request: web.Request): + async def _wrapper(request: web.Request) -> web.StreamResponse: cm = ExceptionHandlingContextManager( exception_handlers_map, request=request ) From 429cbb8a40a80ed379ad6f124c8f42993863323e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 21:50:42 +0100 Subject: [PATCH 56/59] @sanderegg review: avoid BaseExceptions --- .../exception_handling_base.py | 18 +++++++++--------- .../exception_handling_factory.py | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling_base.py index 5871e5fe9dc0..a4dea61909d1 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling_base.py @@ -18,7 +18,7 @@ class AiohttpExceptionHandler(Protocol): async def __call__( self, request: web.Request, - exception: BaseException, + exception: Exception, ) -> web.StreamResponse: """ Callback that handles an exception produced during a request and transforms it into a response @@ -29,12 +29,12 @@ async def __call__( """ -ExceptionHandlersMap: TypeAlias = dict[type[BaseException], AiohttpExceptionHandler] +ExceptionHandlersMap: TypeAlias = dict[type[Exception], AiohttpExceptionHandler] def _sort_exceptions_by_specificity( - exceptions: Iterable[type[BaseException]], *, concrete_first: bool = True -) -> list[type[BaseException]]: + exceptions: Iterable[type[Exception]], *, concrete_first: bool = True +) -> list[type[Exception]]: """ Keyword Arguments: concrete_first -- If True, concrete subclasses precede their superclass (default: {True}). @@ -80,7 +80,7 @@ def __init__( self._response: web.StreamResponse | None = None def _get_exc_handler_or_none( - self, exc_type: type[BaseException], exc_value: BaseException + self, exc_type: type[Exception], exc_value: Exception ) -> AiohttpExceptionHandler | None: exc_handler = self._exc_handlers_map.get(exc_type) if not exc_handler and ( @@ -102,8 +102,8 @@ async def __aenter__(self): async def __aexit__( self, - exc_type: type[BaseException] | None, - exc_value: BaseException | None, + exc_type: type[Exception] | None, + exc_value: Exception | None, traceback: TracebackType | None, ) -> bool: if ( @@ -125,7 +125,7 @@ def get_response_or_none(self) -> web.StreamResponse | None: def exception_handling_decorator( - exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] + exception_handlers_map: dict[type[Exception], AiohttpExceptionHandler] ) -> Callable[[WebHandler], WebHandler]: """Creates a decorator to manage exceptions raised in a given route handler. Ensures consistent exception management across decorated handlers. @@ -153,7 +153,7 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: def exception_handling_middleware( - exception_handlers_map: dict[type[BaseException], AiohttpExceptionHandler] + exception_handlers_map: dict[type[Exception], AiohttpExceptionHandler] ) -> WebMiddleware: """Constructs middleware to handle exceptions raised across app routes 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 76a62e862871..4dced6d79298 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 @@ -31,7 +31,7 @@ class HttpErrorInfo(NamedTuple): msg_template: str # sets HTTPError.reason -ExceptionToHttpErrorMap: TypeAlias = dict[type[BaseException], HttpErrorInfo] +ExceptionToHttpErrorMap: TypeAlias = dict[type[Exception], HttpErrorInfo] def create_error_response(error: ErrorGet, status_code: int) -> web.Response: From aefd3842a067266c54e01b3d89b1d466949d9fb4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 22:01:15 +0100 Subject: [PATCH 57/59] @sanderegg review: folder --- .../exception_handling.py | 16 ---------------- .../exception_handling/__init__.py | 12 ++++++++++++ .../_base.py} | 0 .../_factory.py} | 2 +- .../unit/isolated/test_exception_handling.py | 4 ++-- .../isolated/test_exception_handling_base.py | 2 +- .../isolated/test_exception_handling_factory.py | 4 ++-- 7 files changed, 18 insertions(+), 22 deletions(-) delete mode 100644 services/web/server/src/simcore_service_webserver/exception_handling.py create mode 100644 services/web/server/src/simcore_service_webserver/exception_handling/__init__.py rename services/web/server/src/simcore_service_webserver/{exception_handling_base.py => exception_handling/_base.py} (100%) rename services/web/server/src/simcore_service_webserver/{exception_handling_factory.py => exception_handling/_factory.py} (98%) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling.py b/services/web/server/src/simcore_service_webserver/exception_handling.py deleted file mode 100644 index 98f7c3022491..000000000000 --- a/services/web/server/src/simcore_service_webserver/exception_handling.py +++ /dev/null @@ -1,16 +0,0 @@ -from .exception_handling_base import ExceptionHandlersMap, exception_handling_decorator -from .exception_handling_factory import ( - ExceptionToHttpErrorMap, - HttpErrorInfo, - to_exceptions_handlers_map, -) - -__all__: tuple[str, ...] = ( - "ExceptionHandlersMap", - "ExceptionToHttpErrorMap", - "HttpErrorInfo", - "exception_handling_decorator", - "to_exceptions_handlers_map", -) - -# nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/exception_handling/__init__.py b/services/web/server/src/simcore_service_webserver/exception_handling/__init__.py new file mode 100644 index 000000000000..a2f31a088619 --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/exception_handling/__init__.py @@ -0,0 +1,12 @@ +from ._base import ExceptionHandlersMap, exception_handling_decorator +from ._factory import ExceptionToHttpErrorMap, HttpErrorInfo, to_exceptions_handlers_map + +__all__: tuple[str, ...] = ( + "ExceptionHandlersMap", + "ExceptionToHttpErrorMap", + "HttpErrorInfo", + "exception_handling_decorator", + "to_exceptions_handlers_map", +) + +# nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/exception_handling_base.py b/services/web/server/src/simcore_service_webserver/exception_handling/_base.py similarity index 100% rename from services/web/server/src/simcore_service_webserver/exception_handling_base.py rename to services/web/server/src/simcore_service_webserver/exception_handling/_base.py 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 similarity index 98% rename from services/web/server/src/simcore_service_webserver/exception_handling_factory.py rename to services/web/server/src/simcore_service_webserver/exception_handling/_factory.py index 4dced6d79298..baae399f76b1 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 @@ -9,7 +9,7 @@ from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.status_codes_utils import is_5xx_server_error, is_error -from .exception_handling_base import AiohttpExceptionHandler, ExceptionHandlersMap +from ._base import AiohttpExceptionHandler, ExceptionHandlersMap _logger = logging.getLogger(__name__) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling.py b/services/web/server/tests/unit/isolated/test_exception_handling.py index 8da32c6f34c9..775fe452a213 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling.py @@ -19,10 +19,10 @@ exception_handling_decorator, to_exceptions_handlers_map, ) -from simcore_service_webserver.exception_handling_base import ( +from simcore_service_webserver.exception_handling._base import ( exception_handling_middleware, ) -from simcore_service_webserver.exception_handling_factory import ( +from simcore_service_webserver.exception_handling._factory import ( create_http_error_exception_handlers_map, ) diff --git a/services/web/server/tests/unit/isolated/test_exception_handling_base.py b/services/web/server/tests/unit/isolated/test_exception_handling_base.py index 455abe6cefb2..b9c3bc87f9d9 100644 --- a/services/web/server/tests/unit/isolated/test_exception_handling_base.py +++ b/services/web/server/tests/unit/isolated/test_exception_handling_base.py @@ -10,7 +10,7 @@ from aiohttp import web from aiohttp.test_utils import make_mocked_request from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exception_handling_base import ( +from simcore_service_webserver.exception_handling._base import ( AiohttpExceptionHandler, ExceptionHandlingContextManager, _sort_exceptions_by_specificity, 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 666ececc1aca..e87ef0b53c37 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 @@ -14,11 +14,11 @@ from servicelib.aiohttp import status from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from simcore_service_webserver.errors import WebServerBaseError -from simcore_service_webserver.exception_handling_base import ( +from simcore_service_webserver.exception_handling._base import ( ExceptionHandlingContextManager, exception_handling_decorator, ) -from simcore_service_webserver.exception_handling_factory import ( +from simcore_service_webserver.exception_handling._factory import ( ExceptionToHttpErrorMap, HttpErrorInfo, create_exception_handler_from_http_info, From eacd769d9abc71565ef510e1591ed7ad7192281a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 22:15:16 +0100 Subject: [PATCH 58/59] mypy --- .../simcore_service_webserver/exception_handling/_base.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling/_base.py b/services/web/server/src/simcore_service_webserver/exception_handling/_base.py index a4dea61909d1..aee9c4df0365 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling/_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling/_base.py @@ -102,13 +102,15 @@ async def __aenter__(self): async def __aexit__( self, - exc_type: type[Exception] | None, - exc_value: Exception | None, + exc_type: type[BaseException] | None, + exc_value: BaseException | None, traceback: TracebackType | None, ) -> bool: if ( exc_value is not None and exc_type is not None + and issubclass(exc_type, Exception) + and isinstance(exc_value, Exception) and (exc_handler := self._get_exc_handler_or_none(exc_type, exc_value)) ): self._response = await exc_handler( From 4705c813dbd197d452587dc3d4292de69d4fccfb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sun, 1 Dec 2024 22:16:36 +0100 Subject: [PATCH 59/59] cleanup --- .../src/simcore_service_webserver/exception_handling/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/exception_handling/_base.py b/services/web/server/src/simcore_service_webserver/exception_handling/_base.py index aee9c4df0365..0c9c123bbfb7 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling/_base.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling/_base.py @@ -109,8 +109,8 @@ async def __aexit__( if ( exc_value is not None and exc_type is not None - and issubclass(exc_type, Exception) and isinstance(exc_value, Exception) + and issubclass(exc_type, Exception) and (exc_handler := self._get_exc_handler_or_none(exc_type, exc_value)) ): self._response = await exc_handler(