From 8f1c4e32237351928f4f2040abb701417e2d5b73 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:16:41 +0100 Subject: [PATCH 1/3] improves error --- .../studies_dispatcher/_constants.py | 8 +++++--- .../studies_dispatcher/_redirects_handlers.py | 4 ++-- .../studies_dispatcher/_studies_access.py | 6 +++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py index 440941ccb5d5..3be267f73c28 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py @@ -27,6 +27,8 @@ "Please try later or login with a registered account." ) -MSG_UNEXPECTED_ERROR: Final[ - str -] = "Opps this is embarrasing! Something went really wrong {hint}" +MSG_UNEXPECTED_DISPATCH_ERROR: Final[str] = ( + "Sorry, but looks like something unexpected went wrong!
" + "We track these errors automatically, but if the problem persists feel free to contact us. " + "In the meantime, try refreshing." +) diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py index 3fc24965f3f3..e5985e6cefed 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py @@ -23,7 +23,7 @@ from ..utils import compose_support_error_msg from ..utils_aiohttp import create_redirect_to_page_response from ._catalog import ValidService, validate_requested_service -from ._constants import MSG_UNEXPECTED_ERROR +from ._constants import MSG_UNEXPECTED_DISPATCH_ERROR from ._core import validate_requested_file, validate_requested_viewer from ._errors import InvalidRedirectionParams, StudyDispatcherError from ._models import FileParams, ServiceInfo, ServiceParams, ViewerInfo @@ -127,7 +127,7 @@ async def wrapper(request: web.Request) -> web.StreamResponse: error_code = create_error_code(err) user_error_msg = compose_support_error_msg( - msg=MSG_UNEXPECTED_ERROR.format(hint=""), error_code=error_code + msg=MSG_UNEXPECTED_DISPATCH_ERROR, error_code=error_code ) _logger.exception( **create_troubleshotting_log_kwargs( diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py index 10f1d8b66749..812e03f961ed 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py @@ -48,7 +48,7 @@ MSG_PROJECT_NOT_PUBLISHED, MSG_PUBLIC_PROJECT_NOT_PUBLISHED, MSG_TOO_MANY_GUESTS, - MSG_UNEXPECTED_ERROR, + MSG_UNEXPECTED_DISPATCH_ERROR, ) from ._errors import GuestUsersLimitError from ._users import create_temporary_guest_user, get_authorized_user @@ -260,7 +260,7 @@ async def wrapper(request: web.Request) -> web.StreamResponse: except Exception as err: error_code = create_error_code(err) user_error_msg = compose_support_error_msg( - msg=MSG_UNEXPECTED_ERROR.format(hint=""), error_code=error_code + msg=MSG_UNEXPECTED_DISPATCH_ERROR, error_code=error_code ) _logger.exception( **create_troubleshotting_log_kwargs( @@ -366,7 +366,7 @@ async def get_redirection_to_study_page(request: web.Request) -> web.Response: except Exception as exc: # pylint: disable=broad-except error_code = create_error_code(exc) - user_error_msg = MSG_UNEXPECTED_ERROR.format(hint="while copying your study") + user_error_msg = MSG_UNEXPECTED_DISPATCH_ERROR _logger.exception( **create_troubleshotting_log_kwargs( user_error_msg, From 5127137c351d1d0c7786fbe9544db3dfc25a34b1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:35:40 +0100 Subject: [PATCH 2/3] cleanup --- .../src/simcore_service_webserver/_constants.py | 2 +- .../studies_dispatcher/_catalog.py | 2 +- .../studies_dispatcher/_constants.py | 4 ++-- .../studies_dispatcher/_core.py | 8 +++----- .../studies_dispatcher/_projects.py | 6 ++---- .../studies_dispatcher/_projects_permalinks.py | 10 ++++++---- .../studies_dispatcher/_redirects_handlers.py | 14 +++++++------- .../server/src/simcore_service_webserver/utils.py | 12 ++++++------ .../web/server/tests/unit/isolated/test_utils.py | 2 +- 9 files changed, 29 insertions(+), 31 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/_constants.py b/services/web/server/src/simcore_service_webserver/_constants.py index aafa109b47da..6590592afafd 100644 --- a/services/web/server/src/simcore_service_webserver/_constants.py +++ b/services/web/server/src/simcore_service_webserver/_constants.py @@ -29,8 +29,8 @@ __all__: tuple[str, ...] = ( - "APP_CONFIG_KEY", "APP_AIOPG_ENGINE_KEY", + "APP_CONFIG_KEY", "APP_FIRE_AND_FORGET_TASKS_KEY", "APP_SETTINGS_KEY", "RQT_USERID_KEY", diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_catalog.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_catalog.py index 5f1c8d486e2a..cac54fc484bd 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_catalog.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_catalog.py @@ -1,7 +1,7 @@ import logging +from collections.abc import AsyncIterator from contextlib import suppress from dataclasses import dataclass -from typing import AsyncIterator import sqlalchemy as sa from aiohttp import web diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py index 3be267f73c28..ed596b9f1fa8 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_constants.py @@ -28,7 +28,7 @@ ) MSG_UNEXPECTED_DISPATCH_ERROR: Final[str] = ( - "Sorry, but looks like something unexpected went wrong!
" - "We track these errors automatically, but if the problem persists feel free to contact us. " + "Sorry, but looks like something unexpected went wrong!" + "We track these errors automatically, but if the problem persists feel free to contact us." "In the meantime, try refreshing." ) diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_core.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_core.py index fe76e1a28550..f0fa876bf0d2 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_core.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_core.py @@ -28,8 +28,7 @@ @lru_cache def compose_uuid_from(*values) -> uuid.UUID: composition: str = "/".join(map(str, values)) - new_uuid = uuid.uuid5(_BASE_UUID, composition) - return new_uuid + return uuid.uuid5(_BASE_UUID, composition) async def list_viewers_info( @@ -58,9 +57,8 @@ async def list_viewers_info( async for row in await conn.execute(query): try: # TODO: filter in database (see test_list_default_compatible_services ) - if only_default: - if row["filetype"] in listed_filetype: - continue + if only_default and row["filetype"] in listed_filetype: + continue listed_filetype.add(row["filetype"]) consumer = ViewerInfo.create_from_db(row) consumers.append(consumer) diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects.py index 20d208daf077..3c80c8322462 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects.py @@ -92,7 +92,7 @@ def _create_project( access_rights.write = access_rights.delete = False # Assambles project instance - project = Project( + return Project( uuid=project_id, name=name, description=description, @@ -105,8 +105,6 @@ def _create_project( ui=StudyUI(workbench=workbench_ui), # type: ignore[arg-type] ) - return project - def _create_project_with_service( project_id: ProjectID, @@ -275,7 +273,7 @@ async def get_or_create_project_with_file_and_service( download_link, ) # FIXME: CANNOT GUARANTEE!!, DELETE?? ERROR?? and cannot be viewed until verified? - raise web.HTTPInternalServerError() + raise web.HTTPInternalServerError except (ProjectNotFoundError, ProjectInvalidRightsError): exists = False diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects_permalinks.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects_permalinks.py index a3ba8113c7fe..406982190ec9 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects_permalinks.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects_permalinks.py @@ -31,6 +31,7 @@ class _GroupAccessRightsDict(TypedDict): def create_permalink_for_study( request: web.Request, + *, project_uuid: ProjectID | ProjectIDStr, project_type: ProjectType, project_access_rights: dict[_GroupID, _GroupAccessRightsDict], @@ -44,19 +45,21 @@ def create_permalink_for_study( # check: criterias/conditions on a project to have a permalink if project_type != ProjectType.TEMPLATE: - raise PermalinkNotAllowedError( + msg = ( "Can only create permalink from a template project. " f"Got {project_uuid=} with {project_type=}" ) + raise PermalinkNotAllowedError(msg) project_access_rights_group_1_or_empty: _GroupAccessRightsDict | dict = ( project_access_rights.get("1", {}) ) if not project_access_rights_group_1_or_empty.get("read", False): - raise PermalinkNotAllowedError( + msg = ( "Cannot create permalink if not shared with everyone. " f"Got {project_uuid=} with {project_access_rights=}" ) + raise PermalinkNotAllowedError(msg) # create url_for = create_url_for_function(request) @@ -114,14 +117,13 @@ async def permalink_factory( if not row: raise ProjectNotFoundError(project_uuid=project_uuid) - permalink_info = create_permalink_for_study( + return create_permalink_for_study( request, project_uuid=row.uuid, project_type=row.type, project_access_rights=row.access_rights, project_is_public=row.published, ) - return permalink_info def setup_projects_permalinks( diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py index e5985e6cefed..060cea75a4d3 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py @@ -73,12 +73,12 @@ def _create_redirect_response_to_error_page( def _create_service_info_from(service: ValidService) -> ServiceInfo: - values_map = dict( - key=service.key, - version=service.version, - label=service.title, - is_guest_allowed=service.is_public, - ) + values_map = { + "key": service.key, + "version": service.version, + "label": service.title, + "is_guest_allowed": service.is_public, + } if service.thumbnail: values_map["thumbnail"] = service.thumbnail return ServiceInfo.model_construct(_fields_set=set(values_map.keys()), **values_map) @@ -335,7 +335,7 @@ async def get_redirection_to_viewer(request: web.Request): else: # NOTE: if query is done right, this should never happen - raise InvalidRedirectionParams() + raise InvalidRedirectionParams # Adds auth cookies (login) await ensure_authentication(user, request, response) diff --git a/services/web/server/src/simcore_service_webserver/utils.py b/services/web/server/src/simcore_service_webserver/utils.py index c6eade6345d7..d7b50e7fb5e4 100644 --- a/services/web/server/src/simcore_service_webserver/utils.py +++ b/services/web/server/src/simcore_service_webserver/utils.py @@ -156,13 +156,13 @@ def get_tracemalloc_info(top=10) -> list[str]: def compose_support_error_msg( msg: str, error_code: ErrorCodeStr, support_email: str = "support" ) -> str: - sentences = [] - for line in msg.split("\n"): - if sentence := line.strip(" ."): - sentences.append(sentence[0].upper() + sentence[1:]) - + sentences = [ + sentence[0].upper() + sentence[1:] + for line in msg.split("\n") + if (sentence := line.strip(" .")) + ] sentences.append( - f"For more information please forward this message to {support_email} [{error_code}]" + f"For more information please forward this message to {support_email} (supportID={error_code})" ) return ". ".join(sentences) diff --git a/services/web/server/tests/unit/isolated/test_utils.py b/services/web/server/tests/unit/isolated/test_utils.py index aa02a1a8b299..1bcb69532694 100644 --- a/services/web/server/tests/unit/isolated/test_utils.py +++ b/services/web/server/tests/unit/isolated/test_utils.py @@ -128,5 +128,5 @@ def test_compose_support_error_msg(): ) assert ( msg == "First sentence for Mr.X. Second sentence." - " For more information please forward this message to support@email.com [OEC:139641204989600]" + " For more information please forward this message to support@email.com (supportID=OEC:139641204989600)" ) From 8296cf4e5aaaab55245f7ee624aeea19fe173818 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 7 Jan 2025 13:18:06 +0100 Subject: [PATCH 3/3] fixes error handling issues found in logs --- .../src/servicelib/aiohttp/rest_middlewares.py | 2 +- .../simcore_service_webserver/projects/_crud_handlers.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py index e1d1848406e0..5bdaf3bf6cff 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py @@ -32,7 +32,7 @@ DEFAULT_API_VERSION = "v0" _FMSG_INTERNAL_ERROR_USER_FRIENDLY_WITH_OEC = ( "We apologize for the inconvenience." - " Our team has recorded the issue [{error_code}] and is working to resolve it as quickly as possible." + " Our team has recorded the issue [SupportID={error_code}] and is working to resolve it as quickly as possible." " Thank you for your patience" ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 91f43f8a94cb..5d1874753234 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -113,6 +113,7 @@ async def _wrapper(request: web.Request) -> web.StreamResponse: @login_required @permission_required("project.create") @permission_required("services.pipeline.*") # due to update_pipeline_db +@_handle_projects_exceptions async def create_project(request: web.Request): # # - Create https://google.aip.dev/133 @@ -263,6 +264,7 @@ async def list_projects_full_search(request: web.Request): @routes.get(f"/{VTAG}/projects/active", name="get_active_project") @login_required @permission_required("project.read") +@_handle_projects_exceptions async def get_active_project(request: web.Request) -> web.Response: # # - Get https://google.aip.dev/131 @@ -310,6 +312,7 @@ async def get_active_project(request: web.Request) -> web.Response: @routes.get(f"/{VTAG}/projects/{{project_id}}", name="get_project") @login_required @permission_required("project.read") +@_handle_projects_exceptions async def get_project(request: web.Request): """ @@ -373,6 +376,7 @@ async def get_project(request: web.Request): ) @login_required @permission_required("project.read") +@_handle_projects_exceptions async def get_project_inactivity(request: web.Request): path_params = parse_request_path_parameters_as(ProjectPathParams, request) @@ -409,6 +413,7 @@ async def patch_project(request: web.Request): @routes.delete(f"/{VTAG}/projects/{{project_id}}", name="delete_project") @login_required @permission_required("project.delete") +@_handle_projects_exceptions async def delete_project(request: web.Request): # Delete https://google.aip.dev/135 """ @@ -498,6 +503,7 @@ async def delete_project(request: web.Request): @login_required @permission_required("project.create") @permission_required("services.pipeline.*") # due to update_pipeline_db +@_handle_projects_exceptions async def clone_project(request: web.Request): req_ctx = RequestContext.model_validate(request) path_params = parse_request_path_parameters_as(ProjectPathParams, request)