Handle jmp-client exceptions and introduce a exception handling module

packages/jumpstarter-cli-client/jumpstarter_cli_client/__init__.py

@@ -5,9 +5,9 @@
 from jumpstarter_cli_common import AliasedGroup, opt_log_level, version
 
 from .client_config import create_client_config, delete_client_config, list_client_configs, use_client_config
+from .client_lease import client_lease
 from .client_login import client_login
 from .client_shell import client_shell
-from .lease import lease
 from jumpstarter.common.utils import env
 
 
@@ -30,7 +30,7 @@ def j():
 client.add_command(delete_client_config)
 client.add_command(list_client_configs)
 client.add_command(use_client_config)
-client.add_command(lease)
+client.add_command(client_lease)
 client.add_command(client_login)
 client.add_command(client_shell)
 client.add_command(version)

packages/jumpstarter-cli-client/jumpstarter_cli_client/client_config.py

@@ -2,6 +2,7 @@
 
 import asyncclick as click
 from jumpstarter_cli_common import make_table
+from jumpstarter_cli_common.exceptions import handle_exceptions
 
 from jumpstarter.config import ClientConfigV1Alpha1, ClientConfigV1Alpha1Drivers, ObjectMeta, UserConfigV1Alpha1
 
@@ -50,6 +51,7 @@
     default="",
 )
 @click.option("--unsafe", is_flag=True, help="Should all driver client packages be allowed to load (UNSAFE!).")
+@handle_exceptions
 def create_client_config(
     alias: str,
     namespace: str,
@@ -98,13 +100,15 @@ def set_next_client(name: str):
 
 @click.command("delete-config", short_help="Delete a client config.")
 @click.argument("name", type=str)
+@handle_exceptions
 def delete_client_config(name: str):
     """Delete a Jumpstarter client configuration."""
     set_next_client(name)
     ClientConfigV1Alpha1.delete(name)
 
 
 @click.command("list-configs", short_help="List available client configurations.")
+@handle_exceptions
 def list_client_configs():
     # Allow listing if there is no user config defined
     current_name = None
@@ -130,6 +134,7 @@ def make_row(c: ClientConfigV1Alpha1):
 
 @click.command("use-config", short_help="Select the current client config.")
 @click.argument("name", type=str)
+@handle_exceptions
 def use_client_config(name: str):
     """Select the current Jumpstarter client configuration to use."""
     user_config = UserConfigV1Alpha1.load_or_create()

packages/jumpstarter-cli-client/jumpstarter_cli_client/lease.py → packages/jumpstarter-cli-client/jumpstarter_cli_client/client_lease.py

@@ -1,5 +1,6 @@
 import asyncclick as click
 from jumpstarter_cli_common import AliasedGroup
+from jumpstarter_cli_common.exceptions import handle_exceptions
 
 from jumpstarter.common import MetadataFilter
 from jumpstarter.config import (
@@ -8,14 +9,15 @@
 )
 
 
-@click.group(cls=AliasedGroup, short_help="")
-def lease():
+@click.group(name="lease", cls=AliasedGroup, short_help="")
+def client_lease():
     """Manage leases held by the current client"""
     pass
 
 
-@lease.command("list")
+@client_lease.command("list")
 @click.argument("name", type=str, default="")
+@handle_exceptions
 def lease_list(name):
     if name:
         config = ClientConfigV1Alpha1.load(name)
@@ -28,30 +30,34 @@ def lease_list(name):
         print(lease)
 
 
-@lease.command("release")
+@client_lease.command("release")
 @click.argument("name", type=str, default="")
 @click.option("-l", "--lease", "lease", type=str, default="")
 @click.option("--all", "all_leases", is_flag=True)
+@handle_exceptions
 def lease_release(name, lease, all_leases):
     if name:
         config = ClientConfigV1Alpha1.load(name)
     else:
         config = UserConfigV1Alpha1.load_or_create().config.current_client
     if not config:
-        raise ValueError("no client specified")
+        raise click.BadParameter("no client specified, and no default client set:" +
+                                 "specify a client name, or use jmp client use-config ", param_hint="name")
 
     if all_leases:
         for lease in config.list_leases():
             config.release_lease(lease)
     else:
         if not lease:
-            raise ValueError("no lease specified")
+            raise click.BadParameter("no lease specified, provide one or use --all to release all leases",
+                                    param_hint="lease")
         config.release_lease(lease)
 
 
-@lease.command("request")
+@client_lease.command("request")
 @click.option("-l", "--label", "labels", type=(str, str), multiple=True)
 @click.argument("name", type=str, default="")
+@handle_exceptions
 def lease_request(name, labels):
     """Request an exporter lease from the jumpstarter controller.
 
@@ -74,16 +80,13 @@ def lease_request(name, labels):
         $ jmp lease release -l "${JMP_LEASE}"
 
     """
-    try:
-        if name:
-            config = ClientConfigV1Alpha1.load(name)
-        else:
-            config = UserConfigV1Alpha1.load_or_create().config.current_client
-        if not config:
-            raise ValueError("No client specified")
-        lease = config.request_lease(metadata_filter=MetadataFilter(labels=dict(labels)))
-        print(lease.name)
-    except ValueError as e:
-        raise click.ClickException(str(e)) from e
-    except Exception as e:
-        raise e
+    if name:
+        config = ClientConfigV1Alpha1.load(name)
+    else:
+        config = UserConfigV1Alpha1.load_or_create().config.current_client
+    if not config:
+        raise click.BadParameter("no client specified, and no default client set:" +
+                                 "specify a client name, or use jmp client use-config ", param_hint="name")
+    lease = config.request_lease(metadata_filter=MetadataFilter(labels=dict(labels)))
+    print(lease.name)
+

packages/jumpstarter-cli-client/jumpstarter_cli_client/client_login.py

@@ -1,6 +1,8 @@
 import asyncclick as click
+from jumpstarter_cli_common.exceptions import async_handle_exceptions
 from jumpstarter_cli_common.oidc import Config, decode_jwt_issuer, opt_client_id
 
+from jumpstarter.common.exceptions import FileNotFoundError
 from jumpstarter.config import ClientConfigV1Alpha1, ClientConfigV1Alpha1Drivers, ObjectMeta, UserConfigV1Alpha1
 
 
@@ -43,6 +45,7 @@
     "--unsafe", is_flag=True, help="Should all driver client packages be allowed to load (UNSAFE!).", default=None
 )
 @opt_client_id
+@async_handle_exceptions
 async def client_login(  # noqa: C901
     alias: str,
     endpoint: str,

packages/jumpstarter-cli-client/jumpstarter_cli_client/client_shell.py

@@ -1,4 +1,5 @@
 import asyncclick as click
+from jumpstarter_cli_common.exceptions import handle_exceptions
 
 from jumpstarter.common import MetadataFilter
 from jumpstarter.common.utils import launch_shell
@@ -12,14 +13,17 @@
 @click.argument("name", type=str, default="")
 @click.option("-l", "--label", "labels", type=(str, str), multiple=True)
 @click.option("-n", "--lease", "lease_name", type=str)
+@handle_exceptions
 def client_shell(name: str, labels, lease_name):
     """Spawns a shell connecting to a leased remote exporter"""
     if name:
         config = ClientConfigV1Alpha1.load(name)
     else:
         config = UserConfigV1Alpha1.load_or_create().config.current_client
     if not config:
-        raise ValueError("no client specified")
+       raise click.BadParameter("no client specified, and no default client set:" +
+                                 "specify a client name, or use jmp client use-config ", param_hint="name")
+
 
     with config.lease(metadata_filter=MetadataFilter(labels=dict(labels)), lease_name=lease_name) as lease:
         with lease.serve_unix() as path:

packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py

@@ -0,0 +1,36 @@
+import asyncclick as click
+
+from jumpstarter.common.exceptions import JumpstarterException
+
+
+class ClickExceptionRed(click.ClickException):
+    def format_message(self) -> str:
+        return click.style(self.message, fg='red')
+
+def async_handle_exceptions(func):
+    """Decorator to handle exceptions in async functions."""
+    async def wrapped(*args, **kwargs):
+        try:
+            return await func(*args, **kwargs)
+        except JumpstarterException as e:
+            raise ClickExceptionRed(str(e)) from None
+        except click.ClickException:
+            raise # if it was already a click exception from the cli commands, just re-raise it
+        except Exception:
+            raise
+
+    return wrapped
+
+def handle_exceptions(func):
+    """Decorator to handle exceptions in blocking functions."""
+    def wrapped(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except JumpstarterException as e:
+            raise ClickExceptionRed(str(e)) from None
+        except click.ClickException:
+            raise # if it was already a click exception from the cli commands, just re-raise it
+        except Exception:
+            raise
+
+    return wrapped

packages/jumpstarter/jumpstarter/client/exceptions.py

@@ -0,0 +1,5 @@
+from jumpstarter.common import exceptions
+
+
+class LeaseError(exceptions.JumpstarterException):
+    """Raised when a lease operation fails."""

packages/jumpstarter/jumpstarter/client/lease.py

@@ -8,9 +8,11 @@
 from grpc.aio import Channel
 from jumpstarter_protocol import jumpstarter_pb2, jumpstarter_pb2_grpc, kubernetes_pb2
 
+from .exceptions import LeaseError
 from jumpstarter.client import client_from_path
 from jumpstarter.common import MetadataFilter, TemporaryUnixListener
 from jumpstarter.common.condition import condition_false, condition_message, condition_present_and_equal, condition_true
+from jumpstarter.common.grpc import translate_grpc_exceptions
 from jumpstarter.common.streams import connect_router_stream
 from jumpstarter.config.tls import TLSConfigV1Alpha1
 
@@ -43,26 +45,35 @@ async def _create(self):
         duration_str = f"{duration.seconds}s"
 
         logger.debug("Creating lease request for labels %s for %s", self.metadata_filter.labels, duration_str)
-        self.name = (
-            await self.controller.RequestLease(
-                jumpstarter_pb2.RequestLeaseRequest(
-                    duration=duration,
-                    selector=kubernetes_pb2.LabelSelector(match_labels=self.metadata_filter.labels),
+        with translate_grpc_exceptions():
+            self.name = (
+                await self.controller.RequestLease(
+                    jumpstarter_pb2.RequestLeaseRequest(
+                        duration=duration,
+                        selector=kubernetes_pb2.LabelSelector(match_labels=self.metadata_filter.labels),
+                    )
                 )
-            )
-        ).name
+            ).name
         logger.info("Created lease request for labels %s for %s", self.metadata_filter.labels, duration_str)
 
     def request(self):
+        """Request a lease, or verifies a lease which was already created.
+
+        :return: lease
+        :rtype: Lease
+        :raises LeaseError: if lease is unsatisfiable
+        :raises LeaseError: if lease is not pending
+        :raises TimeoutError: if lease is not ready after timeout
+        """
         return self.portal.call(self.request_async)
 
     async def request_async(self):
         """Request a lease, or verifies a lease which was already created.
 
         :return: lease
         :rtype: Lease
-        :raises ValueError: if lease is unsatisfiable
-        :raises ValueError: if lease is not pending
+        :raises LeaseError: if lease is unsatisfiable
+        :raises LeaseError: if lease is not pending
         :raises TimeoutError: if lease is not ready after timeout
         """
         if self.name:
@@ -79,26 +90,28 @@ async def _acquire(self):
         with fail_after(300):  # TODO: configurable timeout
             while True:
                 logger.debug("Polling Lease %s", self.name)
-                result = await self.controller.GetLease(jumpstarter_pb2.GetLeaseRequest(name=self.name))
+                with translate_grpc_exceptions():
+                    result = await self.controller.GetLease(jumpstarter_pb2.GetLeaseRequest(name=self.name))
 
                 # lease ready
                 if condition_true(result.conditions, "Ready"):
-                    logger.info("Lease %s acquired", self.name)
+                    logger.debug("Lease %s acquired", self.name)
                     return self
                 # lease unsatisfiable
                 if condition_true(result.conditions, "Unsatisfiable"):
-                    logger.error("Lease %s cannot be satisfied: %s", self.name,
+                    message = condition_message(result.conditions, "Unsatisfiable")
+                    logger.debug("Lease %s cannot be satisfied: %s", self.name,
                                  condition_message(result.conditions, "Unsatisfiable"))
-                    raise ValueError("lease unsatisfiable")
+                    raise LeaseError(f"the lease cannot be satisfied: {message}")
+
                 # lease not pending
                 if condition_false(result.conditions, "Pending"):
-                    logger.Error("Lease %s is not in pending, but it isn't in Ready or Unsatisfiable state either",
-                                 self.name)
-                    raise ValueError("lease not pending")
+                    raise LeaseError(
+                        f"Lease {self.name} is not in pending, but it isn't in Ready or Unsatisfiable state either")
+
                 # lease released
                 if condition_present_and_equal(result.conditions, "Ready", "False", "Released"):
-                    logger.error("The lease %s was released", self.name)
-                    raise ValueError("lease released")
+                    raise LeaseError(f"lease {self.name} released")
 
                 await sleep(1)
 

packages/jumpstarter/jumpstarter/common/exceptions.py

@@ -0,0 +1,45 @@
+import sys
+
+
+class JumpstarterException(Exception):
+    """Base class for jumpstarter-specific errors.
+
+    This class should not be raised directly, but should be used as a base
+    class for all jumpstarter-specific errors.
+    It handles the __cause__ attribute so the jumpstarter errors could be raised as
+
+    raise SomeError("message") from original_exception
+    """
+    def __init__(self, message:str):
+        super().__init__(message)
+        self.message = message
+
+    def __str__(self):
+        if self.__cause__:
+            return f"{self.message} (Caused by: {self.__cause__})"
+        return f"{self.message}"
+
+    def print(self, message:str|None = None):
+        ANSI_RED = "\033[91m"
+        ANSI_CLEAR = "\033[0m"
+        print(f"{ANSI_RED}{self}{ANSI_CLEAR}", file=sys.stderr)
+
+class ConnectionError(JumpstarterException):
+    """Raised when a connection to a jumpstarter server fails."""
+    pass
+
+class ConfigurationError(JumpstarterException):
+    """Raised when a configuration error exists."""
+    pass
+
+class ArgumentError(JumpstarterException):
+    """Raised when a cli argument is not valid."""
+    pass
+
+class FileAccessError(JumpstarterException):
+    """Raised when a file access error occurs."""
+    pass
+
+class FileNotFoundError(JumpstarterException):
+    """Raised when a file is not found."""
+    pass