Raise a new ClientError instead of AioRpcError

We want to avoid exposing the underlying details of the client, so in
the future we can change the libraries used to connect to the service,
or even the protocol.

This also removes the re-building of the exceptions, which was pretty
unnecessary and needed some `type: ignore`s due to typing issues in
`grpcio`.

We should probably create a separate error for timeouts in the future.

Signed-off-by: Leandro Lucarella <[email protected]>

Author: llucax

RELEASE_NOTES.md

@@ -6,7 +6,8 @@
 
 ## Upgrading
 
-- The client is now using [`grpclib`](https://pypi.org/project/grpclib/) to connect to the server instead of [`grpcio`](https://pypi.org/project/grpcio/). You might need to adapt the way you connect to the server in your code, using `grpcio.client.Channel` and catching `grpcio.GRPCError` exceptions.
+- The client is now using [`grpclib`](https://pypi.org/project/grpclib/) to connect to the server instead of [`grpcio`](https://pypi.org/project/grpcio/). You might need to adapt the way you connect to the server in your code, using `grpcio.client.Channel`.
+- The client now doesn't raise `grpc.aio.RpcError` exceptions anymore. Instead, it raises `ClientError` exceptions that have the `grpc.aio.RpcError` as their `__cause__`. You might need to adapt your error handling code to catch `ClientError` exceptions instead of `grpc.aio.RpcError` exceptions.
 
 ## New Features
 

src/frequenz/client/microgrid/__init__.py

@@ -27,11 +27,13 @@
 )
 from ._component_states import EVChargerCableState, EVChargerComponentState
 from ._connection import Connection
+from ._exception import ClientError
 from ._metadata import Location, Metadata
 
 __all__ = [
     "ApiClient",
     "BatteryData",
+    "ClientError",
     "Component",
     "ComponentCategory",
     "ComponentData",

src/frequenz/client/microgrid/_client.py

@@ -50,6 +50,7 @@
 )
 from ._connection import Connection
 from ._constants import RECEIVER_MAX_SIZE
+from ._exception import ClientError
 from ._metadata import Location, Metadata
 
 DEFAULT_GRPC_CALL_TIMEOUT = 60.0
@@ -96,8 +97,8 @@ async def components(self) -> Iterable[Component]:
             Iterator whose elements are all the components in the microgrid.
 
         Raises:
-            AioRpcError: if connection to Microgrid API cannot be established or
-                when the api call exceeded timeout
+            ClientError: If the connection to the Microgrid API cannot be established or
+                when the api call exceeded the timeout.
         """
         try:
             # grpc.aio is missing types and mypy thinks this is not awaitable,
@@ -111,21 +112,9 @@ async def components(self) -> Iterable[Component]:
             )
 
         except grpc.aio.AioRpcError as err:
-            msg = f"Failed to list components. Microgrid API: {self.target}. Err: {err.details()}"
-            raise grpc.aio.AioRpcError(
-                code=err.code(),
-                initial_metadata=err.initial_metadata(),
-                # We need to ignore these errors for some reason, otherwise we get this
-                # mypy error:
-                #   Argument "trailing_metadata" to "AioRpcError" has incompatible type
-                #   "tuple[_Metadatum, ...]"; expected "Metadata"
-                # According to grpc.aio documentation, both should have the type
-                # Metadata.
-                # https://grpc.github.io/grpc/python/grpc_asyncio.html#grpc.aio.AioRpcError
-                trailing_metadata=err.trailing_metadata(),  # type: ignore[arg-type]
-                details=msg,
-                debug_error_string=err.debug_error_string(),
-            )
+            raise ClientError(
+                f"Failed to list components. Microgrid API: {self.target}. Err: {err.details()}"
+            ) from err
         components_only = filter(
             lambda c: c.category is not PbComponentCategory.COMPONENT_CATEGORY_SENSOR,
             component_list.components,
@@ -192,8 +181,8 @@ async def connections(
             Microgrid connections matching the provided start and end filters.
 
         Raises:
-            AioRpcError: if connection to Microgrid API cannot be established or
-                when the api call exceeded timeout
+            ClientError: If the connection to the Microgrid API cannot be established or
+                when the api call exceeded the timeout.
         """
         connection_filter = PbConnectionFilter(starts=starts, ends=ends)
         try:
@@ -210,15 +199,9 @@ async def connections(
                 ),
             )
         except grpc.aio.AioRpcError as err:
-            msg = f"Failed to list connections. Microgrid API: {self.target}. Err: {err.details()}"
-            raise grpc.aio.AioRpcError(
-                code=err.code(),
-                initial_metadata=err.initial_metadata(),
-                # See the comment in def components() for why we need to ignore
-                trailing_metadata=err.trailing_metadata(),  # type: ignore[arg-type]
-                details=msg,
-                debug_error_string=err.debug_error_string(),
-            )
+            raise ClientError(
+                f"Failed to list connections. Microgrid API: {self.target}. Err: {err.details()}"
+            ) from err
         # Filter out the components filtered in `components` method.
         # id=0 is an exception indicating grid component.
         valid_ids = {c.component_id for c in valid_components}
@@ -422,8 +405,8 @@ async def set_power(self, component_id: int, power_w: float) -> None:
             power_w: power to set for the component.
 
         Raises:
-            AioRpcError: if connection to Microgrid API cannot be established or
-                when the api call exceeded timeout
+            ClientError: If the connection to the Microgrid API cannot be established or
+                when the api call exceeded the timeout.
         """
         try:
             await cast(
@@ -434,15 +417,9 @@ async def set_power(self, component_id: int, power_w: float) -> None:
                 ),
             )
         except grpc.aio.AioRpcError as err:
-            msg = f"Failed to set power. Microgrid API: {self.target}. Err: {err.details()}"
-            raise grpc.aio.AioRpcError(
-                code=err.code(),
-                initial_metadata=err.initial_metadata(),
-                # See the comment in def components() for why we need to ignore
-                trailing_metadata=err.trailing_metadata(),  # type: ignore[arg-type]
-                details=msg,
-                debug_error_string=err.debug_error_string(),
-            )
+            raise ClientError(
+                f"Failed to set power. Microgrid API: {self.target}. Err: {err.details()}"
+            ) from err
 
     async def set_bounds(
         self,
@@ -460,8 +437,8 @@ async def set_bounds(
         Raises:
             ValueError: when upper bound is less than 0, or when lower bound is
                 greater than 0.
-            grpc.aio.AioRpcError: if connection to Microgrid API cannot be established
-                or when the api call exceeded timeout
+            ClientError: If the connection to the Microgrid API cannot be established or
+                when the api call exceeded the timeout.
         """
         api_details = f"Microgrid API: {self.target}."
         if upper < 0:
@@ -490,4 +467,7 @@ async def set_bounds(
                 api_details,
                 err.details(),
             )
-            raise
+            raise ClientError(
+                f"Failed to set inclusion bounds. Microgrid API: {self.target}. "
+                f"Err: {err.details()}"
+            ) from err

src/frequenz/client/microgrid/_exception.py

@@ -0,0 +1,8 @@
+# License: MIT
+# Copyright © 2022 Frequenz Energy-as-a-Service GmbH
+
+"""Exceptions raised by the microgrid API client."""
+
+
+class ClientError(Exception):
+    """There was an error in the microgrid API client."""

tests/test_timeout.py

@@ -25,7 +25,7 @@
 # pylint: enable=no-name-in-module
 from pytest_mock import MockerFixture
 
-from frequenz.client.microgrid import ApiClient
+from frequenz.client.microgrid import ApiClient, ClientError
 
 from .mock_api import MockGrpcServer, MockMicrogridServicer
 
@@ -38,7 +38,7 @@
 def fake_grpc_call_timeout() -> Iterator[float]:
     """Patch the default gRPC call timeout."""
     # Timeout applied to all gRPC calls under test. It is expected after that the gRPC
-    # calls will raise an AioRpcError with status code equal to DEADLINE_EXCEEDED.
+    # calls will raise an ClientError with status code equal to DEADLINE_EXCEEDED.
     grpc_call_timeout: float = 0.1
 
     with patch(
@@ -49,7 +49,7 @@ def fake_grpc_call_timeout() -> Iterator[float]:
 
 
 async def test_components_timeout(mocker: MockerFixture) -> None:
-    """Test if the components() method properly raises a timeeout AioRpcError."""
+    """Test if the components() method properly raises a timeeout ClientError."""
     servicer = MockMicrogridServicer()
 
     def mock_list_components(
@@ -66,14 +66,16 @@ def mock_list_components(
     grpc_channel = grpc.aio.insecure_channel(target)
     client = ApiClient(grpc_channel=grpc_channel, target=target)
 
-    with pytest.raises(grpc.aio.AioRpcError) as err_ctx:
+    with pytest.raises(ClientError) as err_ctx:
         _ = await client.components()
-    assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
+    cause = err_ctx.value.__cause__
+    assert isinstance(cause, grpc.aio.AioRpcError)
+    assert cause.code() == grpc.StatusCode.DEADLINE_EXCEEDED
     assert await server.graceful_shutdown()
 
 
 async def test_connections_timeout(mocker: MockerFixture) -> None:
-    """Test if the connections() method properly raises a timeout AioRpcError."""
+    """Test if the connections() method properly raises a timeout ClientError."""
     servicer = MockMicrogridServicer()
 
     def mock_list_connections(
@@ -90,14 +92,16 @@ def mock_list_connections(
     grpc_channel = grpc.aio.insecure_channel(target)
     client = ApiClient(grpc_channel=grpc_channel, target=target)
 
-    with pytest.raises(grpc.aio.AioRpcError) as err_ctx:
+    with pytest.raises(ClientError) as err_ctx:
         _ = await client.connections()
-    assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
+    cause = err_ctx.value.__cause__
+    assert isinstance(cause, grpc.aio.AioRpcError)
+    assert cause.code() == grpc.StatusCode.DEADLINE_EXCEEDED
     assert await server.graceful_shutdown()
 
 
 async def test_set_power_timeout(mocker: MockerFixture) -> None:
-    """Test if the set_power() method properly raises a timeout AioRpcError."""
+    """Test if the set_power() method properly raises a timeout ClientError."""
     servicer = MockMicrogridServicer()
 
     def mock_set_power(
@@ -116,9 +120,11 @@ def mock_set_power(
 
     power_values = [-100, 100]
     for power_w in power_values:
-        with pytest.raises(grpc.aio.AioRpcError) as err_ctx:
+        with pytest.raises(ClientError) as err_ctx:
             await client.set_power(component_id=1, power_w=power_w)
-        assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
+        cause = err_ctx.value.__cause__
+        assert isinstance(cause, grpc.aio.AioRpcError)
+        assert cause.code() == grpc.StatusCode.DEADLINE_EXCEEDED
 
     assert await server.graceful_shutdown()
 
@@ -144,8 +150,10 @@ def mock_add_inclusion_bounds(
     bounds_values = [{"lower": 0.0, "upper": 100.0}, {"lower": -10.0, "upper": 1.0}]
 
     for bounds in bounds_values:
-        with pytest.raises(grpc.aio.AioRpcError) as err_ctx:
+        with pytest.raises(ClientError) as err_ctx:
             await client.set_bounds(component_id=1, **bounds)
-        assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
+        cause = err_ctx.value.__cause__
+        assert isinstance(cause, grpc.aio.AioRpcError)
+        assert cause.code() == grpc.StatusCode.DEADLINE_EXCEEDED
 
     assert await server.graceful_shutdown()