Rename GrpcStatusError to GrpcError

Also improve the docs for the class slightly to be more clear about which errors should be more protocol-independent and safer to catch. Signed-off-by: Leandro Lucarella <[email protected]>
frequenz-floss · May 29, 2024 · 2c90bd4 · 2c90bd4
1 parent 7fe7066
commit 2c90bd4
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 29 deletions.
diff --git a/src/frequenz/client/microgrid/__init__.py b/src/frequenz/client/microgrid/__init__.py
@@ -32,7 +32,7 @@
     DataLoss,
     EntityAlreadyExists,
     EntityNotFound,
-    GrpcStatusError,
+    GrpcError,
     InternalError,
     InvalidArgument,
     OperationAborted,
@@ -69,7 +69,7 @@
     "EntityNotFound",
     "Fuse",
     "GridMetadata",
-    "GrpcStatusError",
+    "GrpcError",
     "InternalError",
     "InvalidArgument",
     "InverterData",

diff --git a/src/frequenz/client/microgrid/_exception.py b/src/frequenz/client/microgrid/_exception.py
@@ -58,7 +58,7 @@ def from_grpc_error(
         server_url: str,
         operation: str,
         grpc_error: grpclib.GRPCError,
-    ) -> GrpcStatusError:
+    ) -> GrpcError:
         """Create an instance of the appropriate subclass from a gRPC error.
 
         Args:
@@ -77,7 +77,7 @@ class Ctor(Protocol):
 
             def __call__(
                 self, *, server_url: str, operation: str, grpc_error: grpclib.GRPCError
-            ) -> GrpcStatusError: ...
+            ) -> GrpcError: ...
 
         status_map: dict[grpclib.Status, Ctor] = {
             grpclib.Status.CANCELLED: OperationCancelled,
@@ -109,11 +109,12 @@ def __call__(
         )
 
 
-class GrpcStatusError(ClientError):
-    """The gRPC server returned an error status code.
+class GrpcError(ClientError):
+    """The gRPC server returned an error with a status code.
 
     These errors are specific to gRPC. If you want to use the client in
-    a protocol-independent way, you should avoid catching this exception.
+    a protocol-independent way, you should avoid catching this exception. Catching
+    subclasses that don't have *grpc* in their name should be protocol-independent.
 
     References:
         * [gRPC status
@@ -152,7 +153,7 @@ def __init__(  # pylint: disable=too-many-arguments
         """The original gRPC error."""
 
 
-class UnrecognizedGrpcStatus(GrpcStatusError):
+class UnrecognizedGrpcStatus(GrpcError):
     """The gRPC server returned an unrecognized status code."""
 
     def __init__(
@@ -174,7 +175,7 @@ def __init__(
         )
 
 
-class OperationCancelled(GrpcStatusError):
+class OperationCancelled(GrpcError):
     """The operation was cancelled."""
 
     def __init__(
@@ -196,7 +197,7 @@ def __init__(
         )
 
 
-class UnknownError(GrpcStatusError):
+class UnknownError(GrpcError):
     """There was an error that can't be described using other statuses."""
 
     def __init__(
@@ -218,7 +219,7 @@ def __init__(
         )
 
 
-class InvalidArgument(GrpcStatusError, ValueError):
+class InvalidArgument(GrpcError, ValueError):
     """The client specified an invalid argument.
 
     Note that this error differs from
@@ -246,7 +247,7 @@ def __init__(
         )
 
 
-class OperationTimedOut(GrpcStatusError):
+class OperationTimedOut(GrpcError):
     """The time limit was exceeded while waiting for the operationt o complete.
 
     For operations that change the state of the system, this error may be returned even
@@ -274,7 +275,7 @@ def __init__(
         )
 
 
-class EntityNotFound(GrpcStatusError):
+class EntityNotFound(GrpcError):
     """The requested entity was not found.
 
     Note that this error differs from
@@ -301,7 +302,7 @@ def __init__(
         )
 
 
-class EntityAlreadyExists(GrpcStatusError):
+class EntityAlreadyExists(GrpcError):
     """The entity that we attempted to create already exists."""
 
     def __init__(
@@ -323,7 +324,7 @@ def __init__(
         )
 
 
-class PermissionDenied(GrpcStatusError):
+class PermissionDenied(GrpcError):
     """The caller does not have permission to execute the specified operation.
 
     Note that when the operation is rejected due to other reasons, such as the resources
@@ -354,7 +355,7 @@ def __init__(
         )
 
 
-class ResourceExhausted(GrpcStatusError):
+class ResourceExhausted(GrpcError):
     """Some resource has been exhausted (for example per-user quota, disk space, etc.)."""
 
     def __init__(
@@ -377,7 +378,7 @@ def __init__(
         )
 
 
-class OperationPreconditionFailed(GrpcStatusError):
+class OperationPreconditionFailed(GrpcError):
     """The operation was rejected because the system is not in a required state.
 
     For example, the directory to be deleted is non-empty, an rmdir operation is applied
@@ -405,7 +406,7 @@ def __init__(
         )
 
 
-class OperationAborted(GrpcStatusError):
+class OperationAborted(GrpcError):
     """The operation was aborted.
 
     Typically due to a concurrency issue or transaction abort.
@@ -430,7 +431,7 @@ def __init__(
         )
 
 
-class OperationOutOfRange(GrpcStatusError):
+class OperationOutOfRange(GrpcError):
     """The operation was attempted past the valid range.
 
     Unlike [InvalidArgument][frequenz.client.microgrid.InvalidArgument], this
@@ -461,7 +462,7 @@ def __init__(
         )
 
 
-class OperationNotImplemented(GrpcStatusError):
+class OperationNotImplemented(GrpcError):
     """The operation is not implemented or not supported/enabled in this service."""
 
     def __init__(
@@ -484,7 +485,7 @@ def __init__(
         )
 
 
-class InternalError(GrpcStatusError):
+class InternalError(GrpcError):
     """Some invariants expected by the underlying system have been broken.
 
     This error code is reserved for serious errors.
@@ -510,7 +511,7 @@ def __init__(
         )
 
 
-class ServiceUnavailable(GrpcStatusError):
+class ServiceUnavailable(GrpcError):
     """The service is currently unavailable.
 
     This is most likely a transient condition, which can be corrected by retrying with
@@ -536,7 +537,7 @@ def __init__(
         )
 
 
-class DataLoss(GrpcStatusError):
+class DataLoss(GrpcError):
     """Unrecoverable data loss or corruption."""
 
     def __init__(
@@ -558,7 +559,7 @@ def __init__(
         )
 
 
-class OperationUnauthenticated(GrpcStatusError):
+class OperationUnauthenticated(GrpcError):
     """The request does not have valid authentication credentials for the operation."""
 
     def __init__(

diff --git a/tests/test_exception.py b/tests/test_exception.py
@@ -15,7 +15,7 @@
     DataLoss,
     EntityAlreadyExists,
     EntityNotFound,
-    GrpcStatusError,
+    GrpcError,
     InternalError,
     InvalidArgument,
     OperationAborted,
@@ -38,10 +38,10 @@ class _GrpcStatusErrorCtor(Protocol):
 
     def __call__(
         self, *, server_url: str, operation: str, grpc_error: grpclib.GRPCError
-    ) -> GrpcStatusError: ...
+    ) -> GrpcError: ...
 
 
-ERROR_TUPLES: list[tuple[type[GrpcStatusError], grpclib.Status, str, bool]] = [
+ERROR_TUPLES: list[tuple[type[GrpcError], grpclib.Status, str, bool]] = [
     (
         UnrecognizedGrpcStatus,
         mock.MagicMock(name="unknown_status"),
@@ -176,7 +176,7 @@ def test_grpc_unknown_status_error() -> None:
         "grpc error message",
         "grpc error details",
     )
-    exception = GrpcStatusError(
+    exception = GrpcError(
         server_url="http://testserver",
         operation="test_operation",
         description=expected_description,
@@ -210,7 +210,7 @@ def test_client_error() -> None:
     "exception_class, grpc_status, expected_description, retryable", ERROR_TUPLES
 )
 def test_from_grpc_error(
-    exception_class: type[GrpcStatusError],
+    exception_class: type[GrpcError],
     grpc_status: grpclib.Status,
     expected_description: str,
     retryable: bool,