Skip to content

Commit

Permalink
Only log exceptions when caught (#350)
Browse files Browse the repository at this point in the history
  • Loading branch information
pederhan authored Nov 30, 2024
1 parent 14e54fc commit 8392aa7
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 30 deletions.
16 changes: 4 additions & 12 deletions ci/testsuite-result.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"command_issued": "zone create example.org hostmaster@example.org ns1.example.org # should require force because ns1 is unknown",
"ok": [],
"warning": [
"Host ns1.example.org not found.",
"ns1.example.org is not in mreg, must force"
],
"error": [],
Expand Down Expand Up @@ -41,9 +40,7 @@
"ok": [
"Created zone example.org"
],
"warning": [
"Host ns1.example.org not found."
],
"warning": [],
"error": [],
"output": [],
"api_requests": [
Expand Down Expand Up @@ -210,10 +207,9 @@
"command": "zone set_ns example.org ns2.example.org",
"command_filter": null,
"command_filter_negate": false,
"command_issued": "zone set_ns example.org ns2.example.org # requires force because ns2 is unknown",
"command_issued": "zone set_ns example.org ns2.example.org # requires force because ns2 is unknown",
"ok": [],
"warning": [
"Host ns2.example.org not found.",
"ns2.example.org is not in mreg, must force"
],
"error": [],
Expand Down Expand Up @@ -271,9 +267,7 @@
"ok": [
"Updated nameservers for example.org"
],
"warning": [
"Host ns2.example.org not found."
],
"warning": [],
"error": [],
"output": [],
"api_requests": [
Expand Down Expand Up @@ -15283,9 +15277,7 @@
"ok": [
"Created zone delegation wut.example.org"
],
"warning": [
"Host ns2.example.org not found."
],
"warning": [],
"error": [],
"output": [],
"api_requests": [
Expand Down
4 changes: 2 additions & 2 deletions mreg_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def parse(self, command: str) -> None:
self.last_errno = e.code

except (CliWarning, CliError) as exc:
exc.print_self()
exc.print_and_log()

except CliExit:
# If we have active recordings going on, save them before exiting
Expand Down Expand Up @@ -263,7 +263,7 @@ def process_command_line(self, line: str) -> None:
try:
cmd = output.from_command(line)
except (CliWarning, CliError) as exc:
exc.print_self()
exc.print_and_log()
return
# Create and set the corrolation id, using the cleaned command
# as the suffix. This is used to track the command in the logs
Expand Down
30 changes: 18 additions & 12 deletions mreg_cli/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from prompt_toolkit.formatted_text.html import html_escape


logger = logging.getLogger(__name__)


class CliExit(Exception):
"""Exception used to exit the CLI."""

Expand All @@ -37,10 +40,22 @@ def formatted_exception(self) -> str:
# NOTE: override this in subclasses to provide custom formatting.
return self.escape()

def log(self):
"""Log the exception."""
from mreg_cli.outputmanager import OutputManager

logger.error(str(self))
OutputManager().add_error(str(self))

def print_self(self):
"""Print the exception with appropriate formatting."""
print_formatted_text(HTML(self.formatted_exception()), file=sys.stdout)

def print_and_log(self):
"""Print the exception and log it."""
self.log()
self.print_self()


class CliError(CliException):
"""Exception class for CLI errors.
Expand All @@ -49,14 +64,6 @@ class CliError(CliException):
the user cannot be expected to resolve.
"""

def __init__(self, *args: Any, **kwargs: Any):
"""Initialize the error."""
super().__init__(*args, **kwargs)
logging.getLogger(__name__).error(str(self))
from mreg_cli.outputmanager import OutputManager

OutputManager().add_error(str(self))

def formatted_exception(self) -> str:
"""Return a string formatted with HTML red tag for the error message.
Expand All @@ -71,12 +78,11 @@ class CliWarning(CliException):
Warnings should be recoverable by changing the user input.
"""

def __init__(self, *args: Any, **kwargs: Any):
"""Initialize the warning."""
def log(self):
"""Log the exception."""
from mreg_cli.outputmanager import OutputManager

super().__init__(*args, **kwargs)
logging.getLogger(__name__).warning(str(self))
logger.warning(str(self))
OutputManager().add_warning(str(self))

def formatted_exception(self) -> str:
Expand Down
4 changes: 2 additions & 2 deletions mreg_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def main():
)
except (EOFError, KeyboardInterrupt, LoginFailedError) as e:
if isinstance(e, LoginFailedError):
e.print_self()
e.print_and_log()
else:
print(e)
raise SystemExit() from None
Expand Down Expand Up @@ -204,7 +204,7 @@ def main():
except EOFError:
raise SystemExit() from None
except CliException as e:
e.print_self()
e.print_and_log()
raise SystemExit() from None
else:
try:
Expand Down
4 changes: 2 additions & 2 deletions mreg_cli/utilities/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def prompt_for_password_and_login(user: str, url: str, catch_exception: bool = T
auth_and_update_token(user, password)
except CliError as e:
if catch_exception:
e.print_self()
e.print_and_log()
else:
raise LoginFailedError("Updating token failed.") from e

Expand All @@ -169,7 +169,7 @@ def prompt_for_password_and_try_update_token() -> None:
raise LoginFailedError("Unable to determine username.")
auth_and_update_token(user, password)
except CliError as e:
e.print_self()
e.print_and_log()


def auth_and_update_token(username: str, password: str) -> None:
Expand Down

0 comments on commit 8392aa7

Please sign in to comment.