Skip to content

Commit

Permalink
Issue 6472 - CLI - Improve error message format
Browse files Browse the repository at this point in the history
Description:

For non-json/non-verbose generic exceptions format the message into
something more readable and friendly (desc, result, and info)

Relates: #6472

Reviewed by: tbordaz & spichugi (Thanks!!)
  • Loading branch information
mreynolds389 committed Jan 6, 2025
1 parent 52ebef2 commit 3bb9fcc
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 15 deletions.
5 changes: 4 additions & 1 deletion src/lib389/cli/dsconf
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ from lib389.cli_base.dsrc import dsrc_to_ldap, dsrc_arg_concat
from lib389.cli_base import setup_script_logger
from lib389.cli_base import format_error_to_dict
from lib389.cli_base import parent_argparser
from lib389.cli_base import format_pretty_error
from lib389.utils import instance_choices

parser = argparse.ArgumentParser(allow_abbrev=True, parents=[parent_argparser])
Expand Down Expand Up @@ -133,14 +134,16 @@ if __name__ == '__main__':
if args.verbose:
log.info("Command successful.")
except Exception as e:
result = False
log.debug(e, exc_info=True)
msg = format_error_to_dict(e)

if args and args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))
result = False

disconnect_instance(inst)

Expand Down
4 changes: 3 additions & 1 deletion src/lib389/cli/dscreate
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ from textwrap import dedent
from lib389 import DirSrv
from lib389.cli_ctl import instance as cli_instance
from lib389.cli_base import setup_script_logger
from lib389.cli_base import format_error_to_dict
from lib389.cli_base import format_error_to_dict, format_pretty_error


epilog = """
Expand Down Expand Up @@ -103,6 +103,8 @@ if __name__ == '__main__':
if args and args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))
result = False

Expand Down
7 changes: 7 additions & 0 deletions src/lib389/cli/dsctl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ from lib389.cli_base import (
disconnect_instance,
setup_script_logger,
format_error_to_dict,
format_pretty_error,
parent_argparser
)
from lib389._constants import DSRC_CONTAINER
Expand Down Expand Up @@ -129,13 +130,17 @@ if __name__ == '__main__':
if args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(msg.values()))
sys.exit(1)
except Exception as e:
msg = format_error_to_dict(e)
if args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(msg.values()))
sys.exit(1)
if len(insts) != 1:
Expand All @@ -158,6 +163,8 @@ if __name__ == '__main__':
if args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))
result = False
disconnect_instance(inst)
Expand Down
9 changes: 4 additions & 5 deletions src/lib389/cli/dsidm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ from lib389.cli_idm import role as cli_role
from lib389.cli_idm import service as cli_service
from lib389.cli_base import connect_instance, disconnect_instance, setup_script_logger
from lib389.cli_base.dsrc import dsrc_to_ldap, dsrc_arg_concat
from lib389.cli_base import format_error_to_dict
from lib389.cli_base import format_error_to_dict, format_pretty_error
from lib389.cli_base import parent_argparser

parser = argparse.ArgumentParser(allow_abbrev=True, parents=[parent_argparser])
Expand Down Expand Up @@ -145,10 +145,9 @@ if __name__ == '__main__':
if args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if 'desc' in msg:
log.error(f"Error: {msg['desc']}")
else:
log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))
result = False

disconnect_instance(inst)
Expand Down
6 changes: 5 additions & 1 deletion src/lib389/cli/openldap_to_ds
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@

import argparse
import argcomplete
import json
import signal
import sys
from lib389 import DirSrv
from lib389.cli_base import (
connect_instance,
disconnect_instance,
setup_script_logger,
format_error_to_dict)
format_error_to_dict,
format_pretty_error)
from lib389.cli_base.dsrc import dsrc_to_ldap, dsrc_arg_concat
from lib389._constants import DSRC_HOME

Expand Down Expand Up @@ -254,6 +256,8 @@ if __name__ == '__main__':
if args.json:
sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")
else:
if not args.verbose:
msg = format_pretty_error(msg)
log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))
result = False
disconnect_instance(inst)
Expand Down
9 changes: 5 additions & 4 deletions src/lib389/lib389/_mapped_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def _ldap_op_s(inst, f, fname, *args, **kwargs):
try:
return f(*args, **kwargs)
except ldap.LDAPError as e:
new_desc = f"{fname}({args},{kwargs}) on instance {inst.serverid}";
new_desc = f"{fname}({args},{kwargs}) on instance {inst.serverid}"
if len(e.args) >= 1:
e.args[0]['ldap_request'] = new_desc
logging.getLogger().debug(f"args={e.args}")
Expand Down Expand Up @@ -488,9 +488,10 @@ def set(self, key, value, action=ldap.MOD_REPLACE):
elif value is not None:
value = [ensure_bytes(value)]

return _modify_ext_s(self._instance,self._dn, [(action, key, value)],
serverctrls=self._server_controls, clientctrls=self._client_controls,
escapehatch='i am sure')
return _modify_ext_s(self._instance, self._dn, [(action, key, value)],
serverctrls=self._server_controls,
clientctrls=self._client_controls,
escapehatch='i am sure')

def apply_mods(self, mods):
"""Perform modification operation using several mods at once
Expand Down
21 changes: 18 additions & 3 deletions src/lib389/lib389/cli_base/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def add_arguments(self, actions):
if len(actions) > 0 and actions[0].option_strings:
actions = parent_arguments + actions
super(CustomHelpFormatter, self).add_arguments(actions)

def _format_usage(self, usage, actions, groups, prefix):
usage = super(CustomHelpFormatter, self)._format_usage(usage, actions, groups, prefix)
formatted_options = self._format_actions_usage(parent_arguments, [])
Expand Down Expand Up @@ -504,9 +504,24 @@ def format_error_to_dict(exception):
# We should fix the code here after the issue is fixed
errmsg = str(exception)
try:
# The function literal_eval parses the string and returns only if it's a literal.
# Also, the code is never executed. So there is no reason for a security risk.
# The function literal_eval parses the string and returns only if it's
# a literal. Also, the code is never executed. So there is no reason
# for a security risk.
msg = ast.literal_eval(errmsg)
except Exception:
msg = {'desc': errmsg}

return msg


def format_pretty_error(msg_dict):
"""
Take a raw exception dict and make a pretty message for the user
"""
msg = f"{msg_dict['desc']}"
if 'result' in msg_dict:
msg += f"({msg_dict['result']})"
if 'info' in msg_dict:
msg += f" - {msg_dict['info']}"

return {'desc': msg}

0 comments on commit 3bb9fcc

Please sign in to comment.