diff --git a/.github/instructions/general.instructions.md b/.github/instructions/general.instructions.md index 481c3462815a..bc8950340b68 100644 --- a/.github/instructions/general.instructions.md +++ b/.github/instructions/general.instructions.md @@ -1,11 +1,9 @@ --- applyTo: '**' --- -Provide project context and coding guidelines that AI should follow when generating code, answering questions, or reviewing changes. - ## General Guidelines 1. **Test-Driven Development**: Write unit tests for all new functions and features. Use `pytest` for Python and appropriate testing frameworks for Node.js. 2. **Environment Variables**: Use [Environment Variables Guide](../../docs/env-vars.md) for configuration. Avoid hardcoding sensitive information. -3. **Documentation**: Prefer self-explanatory code; add documentation only if explicitly requested by the developer. Be concise. +3. **Documentation**: Prefer self-explanatory code and add it **only when neeeded** in order to clarify non-obvious rationale or exceptions/errors. 4. **Code Reviews**: Participate in code reviews and provide constructive feedback. diff --git a/.github/instructions/python.instructions.md b/.github/instructions/python.instructions.md index 31dfa431a3e9..3cc5c3cc97f9 100644 --- a/.github/instructions/python.instructions.md +++ b/.github/instructions/python.instructions.md @@ -1,28 +1,61 @@ --- applyTo: '**/*.py' --- -Provide project context and coding guidelines that AI should follow when generating code, answering questions, or reviewing changes. ## 🛠️Coding Instructions for Python in This Repository Follow these rules **strictly** when generating Python code: -### 1. Python Version -* Use Python 3.11: Ensure all code uses features and syntax compatible with Python 3.11. - -### 2. **Type Annotations** +### Python Version -* Always use full type annotations for all functions and class attributes. -* ❗ **Exception**: Do **not** add return type annotations in `test_*` functions. +* Use Python 3.11: Ensure all code uses features and syntax compatible with Python 3.11. -### 3. **Code Style & Formatting** +### General Coding Practices & Formatting * Follow [Python Coding Conventions](../../docs/coding-conventions.md) **strictly**. * Format code with `black` and `ruff`. * Lint code with `ruff` and `pylint`. +* Place **all imports at the top** of the file. +* Use `f-string` formatting for all string interpolation except for logging message strings. +* Use **relative imports** within the same package/module. + - For imports within the same repository/project, always use relative imports (e.g., `from ..constants import APP_SETTINGS_KEY` instead of `from simcore_service_webserver.constants import APP_SETTINGS_KEY`) + - Use absolute imports only for external packages AND main.py-like modules used in entrypoints + + +### Documentation Guidelines (docstrings) + +```python +# ✅ DO THIS +async def fetch_data(user_id: UUID) -> Data: + """Raises: + NotFoundError: If user doesn't exist""" + +def process_items(items: Sequence[Item]) -> Result: + """Complex reordering using Knuth's algorithm + + Raises: + ValueError: If items contains duplicates""" + +# ❌ NOT THIS +def add(a: int, b: int): + """Adds two numbers""" # Redundant +``` -### 4. **Library Compatibility** +**Rules:** +1. Clear names + type hints first +2. Document exceptions (always) +3. Document complex logic or rationale only if not clear from context +4. Skip obvious behavior. Prefer no docstring if nothing above applies + + +### Type Annotations + +* Always use full type annotations for all functions and class attributes. +* ❗ **Exception**: Do **not** add return type annotations in `test_*` functions. + + +### Library Compatibility Ensure compatibility with the following library versions: @@ -31,22 +64,12 @@ Ensure compatibility with the following library versions: * `fastapi` ≥ 0.100 -### 5. **Code Practices** - -* Use `f-string` formatting for all string interpolation except for logging message strings. -* Use **relative imports** within the same package/module. - - For imports within the same repository/project, always use relative imports (e.g., `from ..constants import APP_SETTINGS_KEY` instead of `from simcore_service_webserver.constants import APP_SETTINGS_KEY`) - - Use absolute imports only for external libraries and packages -* Place **all imports at the top** of the file. -* Document functions when the code is not self-explanatory or if asked explicitly. - - -### 6. **JSON Serialization** +### JSON Serialization * Prefer `json_dumps` / `json_loads` from `common_library.json_serialization` instead of the built-in `json.dumps` / `json.loads`. * When using Pydantic models, prefer methods like `model.model_dump_json()` for serialization. -### 7. **aiohttp Framework** +### aiohttp Framework * **Application Keys**: Always use `web.AppKey` for type-safe application storage instead of string keys - Define keys with specific types: `APP_MY_KEY: Final = web.AppKey("APP_MY_KEY", MySpecificType)` @@ -58,6 +81,6 @@ Ensure compatibility with the following library versions: * **Error Handling**: Use the established exception handling decorators and patterns * **Route Definitions**: Use `web.RouteTableDef()` and organize routes logically within modules -### 8. **Running tests** +### Running tests * Use `--keep-docker-up` flag when testing to keep docker containers up between sessions. * Always activate the python virtual environment before running pytest. diff --git a/packages/common-library/src/common_library/error_codes.py b/packages/common-library/src/common_library/error_codes.py index 70829a059ca3..4d1b4c2c908b 100644 --- a/packages/common-library/src/common_library/error_codes.py +++ b/packages/common-library/src/common_library/error_codes.py @@ -56,7 +56,10 @@ def create_error_code(exception: BaseException) -> ErrorCodeStr: """ Generates a unique error code for the given exception. - The error code follows the format: `OEC:{traceback}-{timestamp}`. + The error code follows the format: `OEC:{traceback}-{timestamp}`, i.e. + error with the same traceback fingerprint will have the same first part + and only the timestamp will differ. + This code is intended to be shared with the front-end as a `SupportID` for debugging and support purposes. """ diff --git a/packages/common-library/src/common_library/error_messages.py b/packages/common-library/src/common_library/error_messages.py new file mode 100644 index 000000000000..dbeaeb191947 --- /dev/null +++ b/packages/common-library/src/common_library/error_messages.py @@ -0,0 +1,7 @@ +from typing import Final + +from .user_messages import user_message + +MSG_TRY_AGAIN_OR_SUPPORT: Final[str] = user_message( + "Please try again shortly. If the issue persists, contact support.", _version=1 +) diff --git a/packages/service-library/src/servicelib/aiohttp/requests_validation.py b/packages/service-library/src/servicelib/aiohttp/requests_validation.py index 366e2f90cc0f..3348b5f4d641 100644 --- a/packages/service-library/src/servicelib/aiohttp/requests_validation.py +++ b/packages/service-library/src/servicelib/aiohttp/requests_validation.py @@ -8,17 +8,24 @@ """ import json.decoder +import logging from collections.abc import Iterator from contextlib import contextmanager from typing import Final, TypeVar from aiohttp import web +from common_library.error_codes import create_error_code +from common_library.error_messages import MSG_TRY_AGAIN_OR_SUPPORT +from common_library.logging.logging_errors import create_troubleshooting_log_kwargs from common_library.user_messages import user_message -from models_library.rest_error import EnvelopedError +from models_library.basic_types import IDStr +from models_library.rest_error import EnvelopedError, ErrorGet from pydantic import BaseModel, TypeAdapter, ValidationError from ..mimetype_constants import MIMETYPE_APPLICATION_JSON +from ..rest_constants import RESPONSE_MODEL_POLICY from . import status +from .web_exceptions_handling import create_error_context_from_request ModelClass = TypeVar("ModelClass", bound=BaseModel) ModelOrListOrDictType = TypeVar("ModelOrListOrDictType", bound=BaseModel | list | dict) @@ -26,6 +33,7 @@ APP_JSON_SCHEMA_SPECS_KEY: Final = web.AppKey( "APP_JSON_SCHEMA_SPECS_KEY", dict[str, object] ) +_logger = logging.getLogger(__name__) @contextmanager @@ -85,6 +93,49 @@ def handle_validation_as_http_error( ) from err +@contextmanager +def _handle_json_decode_as_http_error(request: web.Request) -> Iterator[None]: + """Context manager to handle JSONDecodeError and reraise them as HTTPBadRequest error + + Arguments: + request -- web request with JSON body + + Raises: + web.HTTPBadRequest: (400) raised from a JSONDecodeError including a SupportID + """ + try: + yield + + except json.decoder.JSONDecodeError as err: + # This error is really unusual so we create an OEC to help troubleshooting it + error_code = create_error_code(err) + user_error_msg = user_message( + "The request contains data that is not in a valid format. " + + MSG_TRY_AGAIN_OR_SUPPORT, + _version=1, + ) + _logger.exception( + **create_troubleshooting_log_kwargs( + user_error_msg, + error=err, + error_code=error_code, + error_context=create_error_context_from_request(request), + ) + ) + error_model = EnvelopedError( + error=ErrorGet( + message=user_error_msg, + support_id=IDStr(error_code), + status=status.HTTP_400_BAD_REQUEST, + ) + ) + # 400 with a support ID! + raise web.HTTPBadRequest( + text=error_model.model_dump_json(**RESPONSE_MODEL_POLICY), + content_type=MIMETYPE_APPLICATION_JSON, + ) from err + + # NOTE: # # - parameters in the path are part of the resource name and therefore are required @@ -109,7 +160,7 @@ def parse_request_path_parameters_as( with handle_validation_as_http_error( error_msg_template=user_message( - "Invalid parameter/s '{failed}' in request path" + "The parameters '{failed}' in the request path are not valid.", _version=1 ), resource_name=request.rel_url.path, ): @@ -133,7 +184,7 @@ def parse_request_query_parameters_as( with handle_validation_as_http_error( error_msg_template=user_message( - "Invalid parameter/s '{failed}' in request query" + "The parameters '{failed}' in the request query are not valid.", _version=1 ), resource_name=request.rel_url.path, ): @@ -153,7 +204,8 @@ def parse_request_headers_as( ) -> ModelClass: with handle_validation_as_http_error( error_msg_template=user_message( - "Invalid parameter/s '{failed}' in request headers" + "The parameters '{failed}' in the request headers are not valid.", + _version=1, ), resource_name=request.rel_url.path, ): @@ -178,17 +230,18 @@ async def parse_request_body_as( Validated model of request body """ with handle_validation_as_http_error( - error_msg_template=user_message("Invalid field/s '{failed}' in request body"), + error_msg_template=user_message( + "The fields '{failed}' in the request contain values that are not valid.", + _version=1, + ), resource_name=request.rel_url.path, ): if not request.can_read_body: # requests w/o body e.g. when model-schema is fully optional body = {} else: - try: + with _handle_json_decode_as_http_error(request): body = await request.json() - except json.decoder.JSONDecodeError as err: - raise web.HTTPBadRequest(text=f"Invalid json in body: {err}") from err if hasattr(model_schema_cls, "model_validate"): # NOTE: model_schema can be 'list[T]' or 'dict[T]' which raise TypeError diff --git a/packages/service-library/src/servicelib/aiohttp/web_exceptions_handling.py b/packages/service-library/src/servicelib/aiohttp/web_exceptions_handling.py new file mode 100644 index 000000000000..206374586f14 --- /dev/null +++ b/packages/service-library/src/servicelib/aiohttp/web_exceptions_handling.py @@ -0,0 +1,34 @@ +from typing import Any + +from aiohttp import web +from common_library.json_serialization import json_dumps +from models_library.rest_error import ErrorGet + +from ..status_codes_utils import ( + get_code_display_name, + is_error, +) +from .rest_responses import safe_status_message + + +def create_error_context_from_request(request: web.Request) -> dict[str, Any]: + return { + "request": request, + "request.remote": f"{request.remote}", + "request.method": f"{request.method}", + "request.path": f"{request.path}", + } + + +# TODO: HttpJsonError(error = ErrorGet) + + +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=safe_status_message(get_code_display_name(status_code)), + status=status_code, + ) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_constants.py b/services/web/server/src/simcore_service_webserver/catalog/_constants.py index ba8059239478..90e9b1eb06ab 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_constants.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_constants.py @@ -1,9 +1,8 @@ from typing import Final +from common_library.error_messages import MSG_TRY_AGAIN_OR_SUPPORT from common_library.user_messages import user_message -from ..constants import MSG_TRY_AGAIN_OR_SUPPORT - MSG_CATALOG_SERVICE_UNAVAILABLE: Final[str] = user_message( # Most likely the director service is down or misconfigured so the user is asked to try again later. "The catalog service is currently unavailable. This issue has been logged and will be investigated. " diff --git a/services/web/server/src/simcore_service_webserver/constants.py b/services/web/server/src/simcore_service_webserver/constants.py index 1eb80e42d7c0..32248a98d6e0 100644 --- a/services/web/server/src/simcore_service_webserver/constants.py +++ b/services/web/server/src/simcore_service_webserver/constants.py @@ -1,8 +1,7 @@ # pylint:disable=unused-import -from typing import TYPE_CHECKING, Final +from typing import Final -from aiohttp import web from common_library.user_messages import user_message from servicelib.aiohttp.application_keys import ( APP_AIOPG_ENGINE_KEY, @@ -50,16 +49,12 @@ RQ_PRODUCT_KEY: Final[str] = f"{__name__}.RQ_PRODUCT_KEY" -MSG_TRY_AGAIN_OR_SUPPORT: Final[str] = user_message( - "Please try again shortly. If the issue persists, contact support.", _version=1 -) - - __all__: tuple[str, ...] = ( "APP_AIOPG_ENGINE_KEY", "APP_CLIENT_SESSION_KEY", "APP_CONFIG_KEY", "APP_FIRE_AND_FORGET_TASKS_KEY", + "APP_NAME", "FRONTEND_APPS_AVAILABLE", "FRONTEND_APP_DEFAULT", "RQT_USERID_KEY", diff --git a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py index 89888cb31757..6efed90f2de4 100644 --- a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py @@ -2,13 +2,13 @@ from aiohttp import web from common_library.error_codes import create_error_code +from common_library.error_messages import MSG_TRY_AGAIN_OR_SUPPORT from common_library.logging.logging_errors import create_troubleshooting_log_kwargs from common_library.user_messages import user_message from models_library.rest_error import ErrorGet from servicelib import status_codes_utils from servicelib.aiohttp import status -from ...constants import MSG_TRY_AGAIN_OR_SUPPORT from ...exception_handling import ( ExceptionHandlersMap, ExceptionToHttpErrorMap, 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 index fd4f6992a1b2..642628c658e4 100644 --- a/services/web/server/src/simcore_service_webserver/exception_handling/__init__.py +++ b/services/web/server/src/simcore_service_webserver/exception_handling/__init__.py @@ -1,9 +1,12 @@ +from servicelib.aiohttp.web_exceptions_handling import ( + create_error_context_from_request, + create_error_response, +) + from ._base import ExceptionHandlersMap, exception_handling_decorator from ._factory import ( ExceptionToHttpErrorMap, HttpErrorInfo, - create_error_context_from_request, - create_error_response, to_exceptions_handlers_map, ) 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 49118ad8b8de..3890b4a679d0 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 @@ -1,15 +1,16 @@ import logging -from typing import Any, NamedTuple, TypeAlias +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 common_library.logging.logging_errors import create_troubleshooting_log_kwargs from models_library.rest_error import ErrorGet -from servicelib.aiohttp.rest_responses import safe_status_message from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions +from servicelib.aiohttp.web_exceptions_handling import ( + create_error_context_from_request, + create_error_response, +) from servicelib.status_codes_utils import ( - get_code_display_name, is_5xx_server_error, is_error, ) @@ -39,26 +40,6 @@ class HttpErrorInfo(NamedTuple): ExceptionToHttpErrorMap: TypeAlias = dict[type[Exception], HttpErrorInfo] -def create_error_context_from_request(request: web.Request) -> dict[str, Any]: - return { - "request": request, - "request.remote": f"{request.remote}", - "request.method": f"{request.method}", - "request.path": f"{request.path}", - } - - -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=safe_status_message(get_code_display_name(status_code)), - status=status_code, - ) - - def create_exception_handler_from_http_info( status_code: int, msg_template: str, diff --git a/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py b/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py index 78635508bd95..9f7ccc0d2c1c 100644 --- a/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py @@ -1,7 +1,7 @@ +from common_library.error_messages import MSG_TRY_AGAIN_OR_SUPPORT from common_library.user_messages import user_message from servicelib.aiohttp import status -from ...constants import MSG_TRY_AGAIN_OR_SUPPORT from ...exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo,