🎨 Unified construction of error classes (#5359)

Author: pcrespov

packages/models-library/src/models_library/errors_classes.py

@@ -0,0 +1,23 @@
+from pydantic.errors import PydanticErrorMixin
+
+
+class OsparcErrorMixin(PydanticErrorMixin):
+    def __new__(cls, *args, **kwargs):
+        if not hasattr(cls, "code"):
+            cls.code = cls._get_full_class_name()
+        return super().__new__(cls, *args, **kwargs)
+
+    @classmethod
+    def _get_full_class_name(cls) -> str:
+        relevant_classes = [
+            c.__name__
+            for c in cls.__mro__[:-1]
+            if c.__name__
+            not in (
+                "PydanticErrorMixin",
+                "OsparcErrorMixin",
+                "Exception",
+                "BaseException",
+            )
+        ]
+        return ".".join(reversed(relevant_classes))

packages/models-library/tests/test_errors.py

@@ -8,16 +8,14 @@
 from pydantic import BaseModel, ValidationError, conint
 
 
-class B(BaseModel):
-    y: list[int]
-
-
-class A(BaseModel):
-    x: conint(ge=2)
-    b: B
+def test_pydantic_error_dict():
+    class B(BaseModel):
+        y: list[int]
 
+    class A(BaseModel):
+        x: conint(ge=2)
+        b: B
 
-def test_pydantic_error_dict():
     with pytest.raises(ValidationError) as exc_info:
         A(x=-1, b={"y": [0, "wrong"]})
 

packages/models-library/tests/test_errors_classes.py

@@ -0,0 +1,136 @@
+# pylint: disable=protected-access
+# pylint: disable=redefined-outer-name
+# pylint: disable=unused-argument
+# pylint: disable=unused-variable
+# pylint: disable=no-member
+
+
+from datetime import datetime
+from typing import Any
+
+import pytest
+from models_library.errors_classes import OsparcErrorMixin
+
+
+def test_get_full_class_name():
+    class A(OsparcErrorMixin):
+        ...
+
+    class B1(A):
+        ...
+
+    class B2(A):
+        ...
+
+    class C(B2):
+        ...
+
+    class B12(B1, ValueError):
+        ...
+
+    assert B1._get_full_class_name() == "A.B1"
+    assert C._get_full_class_name() == "A.B2.C"
+    assert A._get_full_class_name() == "A"
+
+    # diamond inheritance (not usual but supported)
+    assert B12._get_full_class_name() == "ValueError.A.B1.B12"
+
+
+def test_error_codes_and_msg_template():
+    class MyBaseError(OsparcErrorMixin, Exception):
+        def __init__(self, **ctx: Any) -> None:
+            super().__init__(**ctx)  # Do not forget this for base exceptions!
+
+    class MyValueError(MyBaseError, ValueError):
+        msg_template = "Wrong value {value}"
+
+    error = MyValueError(value=42)
+
+    assert error.code == "ValueError.MyBaseError.MyValueError"
+    assert f"{error}" == "Wrong value 42"
+
+    class MyTypeError(MyBaseError, TypeError):
+        code = "i_want_this"
+        msg_template = "Wrong type {type}"
+
+    error = MyTypeError(type="int")
+
+    assert error.code == "i_want_this"
+    assert f"{error}" == "Wrong type int"
+
+
+def test_error_msg_template_override():
+    class MyError(OsparcErrorMixin, Exception):
+        msg_template = "Wrong value {value}"
+
+    error_override_msg = MyError(msg_template="I want this message")
+    assert str(error_override_msg) == "I want this message"
+
+    error = MyError(value=42)
+    assert hasattr(error, "value")
+    assert str(error) == f"Wrong value {error.value}"
+
+
+def test_error_msg_template_nicer_override():
+    class MyError(OsparcErrorMixin, Exception):
+        msg_template = "Wrong value {value}"
+
+        def __init__(self, msg=None, **ctx: Any) -> None:
+            super().__init__(**ctx)
+            # positional argument msg (if defined) overrides the msg_template
+            if msg:
+                self.msg_template = msg
+
+    error_override_msg = MyError("I want this message")
+    assert str(error_override_msg) == "I want this message"
+
+    error = MyError(value=42)
+    assert hasattr(error, "value")
+    assert str(error) == f"Wrong value {error.value}"
+
+
+def test_error_with_constructor():
+    class MyError(OsparcErrorMixin, ValueError):
+        msg_template = "Wrong value {value}"
+
+        # handy e.g. autocompletion
+        def __init__(self, *, my_value: int = 42, **extra):
+            super().__init__(**extra)
+            self.value = my_value
+
+    error = MyError(my_value=33, something_else="yes")
+    assert error.value == 33
+    assert str(error) == "Wrong value 33"
+    assert not hasattr(error, "my_value")
+
+    # the autocompletion does not see this
+    assert error.something_else == "yes"
+
+
+@pytest.mark.parametrize(
+    "str_format,ctx,expected",
+    [
+        pytest.param("{value:10}", {"value": "Python"}, "Python    ", id="left-align"),
+        pytest.param(
+            "{value:>10}", {"value": "Python"}, "    Python", id="right-align"
+        ),
+        pytest.param(
+            "{value:^10}", {"value": "Python"}, "  Python  ", id="center-align"
+        ),
+        pytest.param("{v:.2f}", {"v": 3.1415926}, "3.14", id="decimals"),
+        pytest.param(
+            "{dt:%Y-%m-%d %H:%M}",
+            {"dt": datetime(2020, 5, 17, 18, 45)},
+            "2020-05-17 18:45",
+            id="datetime",
+        ),
+    ],
+)
+def test_msg_template_with_different_formats(
+    str_format: str, ctx: dict[str, Any], expected: str
+):
+    class MyError(OsparcErrorMixin, ValueError):
+        msg_template = str_format
+
+    error = MyError(**ctx)
+    assert str(error) == expected

services/web/server/Makefile

@@ -6,7 +6,7 @@ include ../../../scripts/common-service.Makefile
 
 # overrides since it does not has same directory name
 APP_NAME := webserver
-
+PY_PACKAGE_NAME := simcore_service_webserver
 
 .PHONY: requirements
 requirements: ## compiles pip requirements (.in -> .txt)

services/web/server/src/simcore_service_webserver/db_listener/_db_comp_tasks_listening_task.py

@@ -35,7 +35,7 @@ async def _get_project_owner(conn: SAConnection, project_uuid: str) -> PositiveI
         select(projects.c.prj_owner).where(projects.c.uuid == project_uuid)
     )
     if not the_project_owner:
-        raise exceptions.ProjectOwnerNotFoundError(project_uuid)
+        raise exceptions.ProjectOwnerNotFoundError(project_uuid=project_uuid)
     return the_project_owner
 
 

services/web/server/src/simcore_service_webserver/director_v2/exceptions.py

@@ -2,43 +2,43 @@
 
 from typing import Any
 
-from pydantic.errors import PydanticErrorMixin
+from ..errors import WebServerBaseError
 
 
-class DirectorServiceError(PydanticErrorMixin, RuntimeError):
+class DirectorServiceError(WebServerBaseError, RuntimeError):
     """Basic exception for errors raised by director-v2"""
 
-    msg_template = "Unexpected error: director-v2 returned {status!r}, reason {reason!r} after calling {url!r}"
+    msg_template = "Unexpected error: director-v2 returned '{status}', reason '{reason}' after calling '{url}'"
 
     def __init__(self, *, status: int, reason: str, **ctx: Any) -> None:
+        super().__init__(**ctx)
         self.status = status
         self.reason = reason
-        super().__init__(status=status, reason=reason, **ctx)
 
 
 class ClusterNotFoundError(DirectorServiceError):
     """Cluster was not found in director-v2"""
 
-    msg_template = "Cluster {cluster_id!r} not found"
+    msg_template = "Cluster '{cluster_id}' not found"
 
 
 class ClusterAccessForbidden(DirectorServiceError):
     """Cluster access is forbidden"""
 
-    msg_template = "Cluster {cluster_id!r} access forbidden!"
+    msg_template = "Cluster '{cluster_id}' access forbidden!"
 
 
 class ClusterPingError(DirectorServiceError):
     """Cluster ping failed"""
 
-    msg_template = "Connection to cluster in {endpoint!r} failed, received {reason!r}"
+    msg_template = "Connection to cluster in '{endpoint}' failed, received '{reason}'"
 
 
 class ClusterDefinedPingError(DirectorServiceError):
     """Cluster ping failed"""
 
-    msg_template = "Connection to cluster {cluster_id!r} failed, received {reason!r}"
+    msg_template = "Connection to cluster '{cluster_id}' failed, received '{reason}'"
 
 
 class ServiceWaitingForManualIntervention(DirectorServiceError):
-    msg_template = "Service {service_uuid} is waiting for user manual intervention"
+    msg_template = "Service '{service_uuid}' is waiting for user manual intervention"

services/web/server/src/simcore_service_webserver/errors.py

@@ -0,0 +1,8 @@
+from typing import Any
+
+from models_library.errors_classes import OsparcErrorMixin
+
+
+class WebServerBaseError(OsparcErrorMixin, Exception):
+    def __init__(self, **ctx: Any) -> None:
+        super().__init__(**ctx)

services/web/server/src/simcore_service_webserver/groups/_handlers.py

@@ -26,7 +26,7 @@
 from ..login.decorators import login_required
 from ..products.api import Product, get_current_product
 from ..scicrunch.db import ResearchResourceRepository
-from ..scicrunch.errors import InvalidRRID, ScicrunchError
+from ..scicrunch.errors import InvalidRRIDError, ScicrunchError
 from ..scicrunch.models import ResearchResource, ResourceHit
 from ..scicrunch.service_client import SciCrunch
 from ..security.decorators import permission_required
@@ -295,8 +295,8 @@ async def wrapper(request: web.Request) -> web.Response:
         try:
             return await handler(request)
 
-        except InvalidRRID as err:
-            raise web.HTTPBadRequest(reason=err.reason) from err
+        except InvalidRRIDError as err:
+            raise web.HTTPBadRequest(reason=f"{err}") from err
 
         except ScicrunchError as err:
             user_msg = "Cannot get RRID since scicrunch.org service is not reachable."

services/web/server/src/simcore_service_webserver/groups/exceptions.py

@@ -1,32 +1,31 @@
 """Defines the different exceptions that may arise in the projects subpackage"""
 
+from ..errors import WebServerBaseError
 
-class GroupsError(Exception):
-    """Basic exception for errors raised in projects"""
+
+class GroupsError(WebServerBaseError):
+    msg_template = "{msg}"
 
     def __init__(self, msg: str = None):
-        super().__init__(msg or "Unexpected error occured in projects subpackage")
+        super().__init__(msg=msg or "Unexpected error occured in projects subpackage")
 
 
 class GroupNotFoundError(GroupsError):
-    """Group was not found in DB"""
+    msg_template = "Group with id {gid} not found"
 
-    def __init__(self, gid: int):
-        super().__init__(f"Group with id {gid} not found")
+    def __init__(self, gid, **extras):
+        super().__init__(**extras)
         self.gid = gid
 
 
 class UserInsufficientRightsError(GroupsError):
-    """User has not sufficient rights"""
-
-    def __init__(self, msg: str):
-        super().__init__(msg)
+    ...
 
 
 class UserInGroupNotFoundError(GroupsError):
-    """User in group was not found in DB"""
+    msg_template = "User id {uid} in Group {gid} not found"
 
-    def __init__(self, gid: int, uid: int):
-        super().__init__(f"User id {uid} in Group {gid} not found")
-        self.gid = gid
+    def __init__(self, uid, gid, **extras):
+        super().__init__(**extras)
         self.uid = uid
+        self.gid = gid