Skip to content

Commit

Permalink
Rewrote the error handler for better error message clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
AncientPatata committed Jan 30, 2025
1 parent 9f21556 commit 8f5806d
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 27 deletions.
3 changes: 1 addition & 2 deletions src/armonik_cli/commands/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

from armonik_cli.core import console, base_command, base_group
from armonik_cli.core.params import KeyValuePairParam, TimeDeltaParam, FilterParam, FieldParam
from armonik_cli.exceptions import InternalError

TASKS_TABLE_COLS = [("ID", "Id"), ("Status", "Status"), ("CreatedAt", "CreatedAt")]

Expand Down Expand Up @@ -238,7 +237,7 @@ def tasks_create(
"red",
)
)
raise InternalError(
raise click.BadParameter(
"If you want to pass in additional task options please provide all three (max duration, priority, max retries)"
)
task_definition = TaskDefinition(
Expand Down
55 changes: 40 additions & 15 deletions src/armonik_cli/core/decorators.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import grpc
import rich_click as click

from functools import wraps, partial
from typing import Callable, Optional, Any

from armonik_cli.core.console import console
from armonik_cli.core.common import global_cluster_config_options, global_common_options
from armonik_cli.exceptions import NotFoundError, InternalError, InternalArmoniKError
from armonik_cli.exceptions import (
InternalCliError,
InternalArmoniKError,
)


def error_handler(func: Optional[Callable[..., Any]] = None) -> Callable[..., Any]:
Expand All @@ -24,28 +26,51 @@ def error_handler(func: Optional[Callable[..., Any]] = None) -> Callable[..., An
return partial(error_handler)

@wraps(func)
def wrapper(*args: Any, **kwargs: Any) -> Any:
def wrapper(*args, **kwargs):
debug_mode = kwargs.get("debug", False)
enriched_message = f"when executing {func.__name__}"
try:
return func(*args, **kwargs)
except click.ClickException:
raise
except grpc.RpcError as err:
status_code = err.code()
error_details = f"{err.details()}."
error_details = f"{err.details()} (gRPC Code: {status_code.name})."

if debug_mode:
console.print_exception()

if status_code == grpc.StatusCode.NOT_FOUND:
raise NotFoundError(error_details)
if status_code == grpc.StatusCode.INVALID_ARGUMENT:
raise InternalCliError(
f"Invalid argument provided {enriched_message}: \n{error_details}"
) from err
elif status_code == grpc.StatusCode.NOT_FOUND:
raise InternalArmoniKError(
f"Resource Not Found {enriched_message}:\n{error_details}"
) from err
elif status_code == grpc.StatusCode.ALREADY_EXISTS:
raise InternalArmoniKError(
f"Resource already exists {enriched_message}:\n{error_details}"
) from err
elif status_code == grpc.StatusCode.DEADLINE_EXCEEDED:
raise InternalArmoniKError(
f"Request deadline exceeded {enriched_message}:\n{error_details}"
) from err
elif status_code == grpc.StatusCode.INTERNAL:
raise InternalArmoniKError(f"An internal exception has occurred:\n{error_details}")
raise InternalArmoniKError(
f"An internal exception occurred {enriched_message}:\n{error_details}"
) from err
elif status_code == grpc.StatusCode.UNKNOWN:
raise InternalArmoniKError(f"An unknown exception has occurred:\n{error_details}")
raise InternalArmoniKError(
f"An unknown exception occurred {enriched_message}:\n{error_details}"
) from err
else:
raise InternalError("An internal fatal error occurred.")
except Exception:
if kwargs.get("debug", False):
raise InternalArmoniKError(
f"An unhandled gRPC error occurred: {error_details}"
) from err

except:
if debug_mode:
console.print_exception()
else:
raise InternalError("An internal fatal error occurred.")
raise

return wrapper

Expand Down
12 changes: 7 additions & 5 deletions src/armonik_cli/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import rich_click as click
from rich.panel import Panel
from rich import print


class ArmoniKCLIError(click.ClickException):
Expand All @@ -7,14 +9,14 @@ class ArmoniKCLIError(click.ClickException):
def __init__(self, message: str) -> None:
super().__init__(message)

def show(self, file=None):
"""Override to display errors in a cleaner format."""
print(Panel(self.format_message(), title="Error", style="red"))

class InternalError(ArmoniKCLIError):

class InternalCliError(ArmoniKCLIError):
"""Error raised when an unknown internal error occured."""


class InternalArmoniKError(ArmoniKCLIError):
"""Error raised when there's an error in ArmoniK, you need to check the logs there for more information."""


class NotFoundError(ArmoniKCLIError):
"""Error raised when a given object of the API is not found."""
2 changes: 1 addition & 1 deletion tests/commands/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def test_task_cancel(mocker, cmd):
),
(
f"task create --endpoint {ENDPOINT} --session-id sessionid --payload-id payloadid --expected-outputs 1 --expected-outputs 2 --data-dependencies 3 --max-duration 00:00:15.00 --priority 0",
1,
2,
),
],
)
Expand Down
9 changes: 5 additions & 4 deletions tests/core/test_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from grpc import RpcError, StatusCode

from armonik_cli.core.decorators import error_handler, base_command, base_group
from armonik_cli.exceptions import NotFoundError, InternalError
from armonik_cli.exceptions import InternalCliError


class DummyRpcError(RpcError):
Expand All @@ -20,7 +20,7 @@ def details(self):

@pytest.mark.parametrize(
("exception", "code"),
[(NotFoundError, StatusCode.NOT_FOUND), (InternalError, StatusCode.UNAVAILABLE)],
[(InternalCliError, StatusCode.UNAVAILABLE)],
)
def test_error_handler_rpc_error(exception, code):
@error_handler
Expand All @@ -37,7 +37,7 @@ def test_error_handler_other_no_debug(decorator):
def raise_error():
raise ValueError()

with pytest.raises(InternalError):
with pytest.raises(InternalCliError):
raise_error()


Expand All @@ -47,7 +47,8 @@ def test_error_handler_other_debug(decorator):
def raise_error(debug=None):
raise ValueError()

raise_error(debug=True)
with pytest.raises(InternalCliError):
raise_error(debug=True)


@pytest.mark.parametrize("decorator", [base_group, base_group()])
Expand Down

0 comments on commit 8f5806d

Please sign in to comment.