Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improved error message clarity #38

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.MissingParameter(
"If you want to pass in additional task options please provide all three (max duration, priority, max retries)"
)
task_definition = TaskDefinition(
Expand Down
40 changes: 25 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,36 @@ 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)
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(error_details) from err
elif status_code == grpc.StatusCode.NOT_FOUND:
raise InternalArmoniKError(error_details) from err
elif status_code == grpc.StatusCode.ALREADY_EXISTS:
raise InternalArmoniKError(error_details) from err
elif status_code == grpc.StatusCode.DEADLINE_EXCEEDED:
raise InternalArmoniKError(error_details) from err
elif status_code == grpc.StatusCode.INTERNAL:
raise InternalArmoniKError(f"An internal exception has occurred:\n{error_details}")
raise InternalArmoniKError(error_details) from err
elif status_code == grpc.StatusCode.UNKNOWN:
raise InternalArmoniKError(f"An unknown exception has occurred:\n{error_details}")
raise InternalArmoniKError(error_details) from err
else:
raise InternalError("An internal fatal error occurred.")
except Exception:
if kwargs.get("debug", False):
raise InternalArmoniKError(error_details) from err

except Exception as e:
if debug_mode:
console.print_exception()
else:
raise InternalError("An internal fatal error occurred.")
raise InternalCliError(f"CLI errored with exception:\n{e}") from e

return wrapper

Expand Down
14 changes: 10 additions & 4 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,18 @@ 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."""

exit_code = 3


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."""
exit_code = 4
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,
3,
),
],
)
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 InternalArmoniKError, 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)],
[(InternalArmoniKError, 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
Loading