Merge pull request #264 from YunoHost/kill_error_see_log

[mod] trash this UX madness that is error_see_logs and improve error messages
This commit is contained in:
Alexandre Aubin 2021-01-01 16:21:15 +01:00 committed by GitHub
commit 201b033606
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 69 additions and 45 deletions

View file

@ -7,7 +7,6 @@
"deprecated_command": "'{prog} {command}' is deprecated and will be removed in the future", "deprecated_command": "'{prog} {command}' is deprecated and will be removed in the future",
"deprecated_command_alias": "'{prog} {old}' is deprecated and will be removed in the future, use '{prog} {new}' instead", "deprecated_command_alias": "'{prog} {old}' is deprecated and will be removed in the future, use '{prog} {new}' instead",
"error": "Error:", "error": "Error:",
"error_see_log": "An error occurred. Please see the logs for details, they are located in /var/log/yunohost/.",
"file_exists": "File already exists: '{path}'", "file_exists": "File already exists: '{path}'",
"file_not_exist": "File does not exist: '{path}'", "file_not_exist": "File does not exist: '{path}'",
"folder_exists": "Folder already exists: '{path}'", "folder_exists": "Folder already exists: '{path}'",

View file

@ -304,14 +304,12 @@ class ExtraArgumentParser(object):
# Validate parameter value # Validate parameter value
parameters[p] = klass.validate(v, arg_name) parameters[p] = klass.validate(v, arg_name)
except Exception as e: except Exception as e:
logger.error( error_message = (
"unable to validate extra parameter '%s' " "unable to validate extra parameter '%s' for argument '%s': %s"
"for argument '%s': %s", % (p, arg_name, e)
p,
arg_name,
e,
) )
raise MoulinetteError("error_see_log") logger.error(error_message)
raise MoulinetteError(error_message, raw_msg=True)
return parameters return parameters
@ -478,10 +476,12 @@ class ActionsMap(object):
try: try:
mod = import_module("moulinette.authenticators.%s" % auth_conf["vendor"]) mod = import_module("moulinette.authenticators.%s" % auth_conf["vendor"])
except ImportError: except ImportError:
logger.exception( error_message = (
"unable to load authenticator vendor '%s'", auth_conf["vendor"] "unable to load authenticator vendor module 'moulinette.authenticators.%s'"
% auth_conf["vendor"]
) )
raise MoulinetteError("error_see_log") logger.exception(error_message)
raise MoulinetteError(error_message, raw_msg=True)
else: else:
return mod.Authenticator(**auth_conf) return mod.Authenticator(**auth_conf)
@ -561,12 +561,17 @@ class ActionsMap(object):
time() - start, time() - start,
) )
func = getattr(mod, func_name) func = getattr(mod, func_name)
except (AttributeError, ImportError): except (AttributeError, ImportError) as e:
import traceback import traceback
traceback.print_exc() traceback.print_exc()
logger.exception("unable to load function %s.%s", namespace, func_name) error_message = "unable to load function %s.%s because: %s" % (
raise MoulinetteError("error_see_log") namespace,
func_name,
e,
)
logger.exception(error_message)
raise MoulinetteError(error_message, raw_msg=True)
else: else:
log_id = start_action_logging() log_id = start_action_logging()
if logger.isEnabledFor(logging.DEBUG): if logger.isEnabledFor(logging.DEBUG):

View file

@ -250,24 +250,24 @@ class BaseActionsMapParser(object):
# Store only if authentication is needed # Store only if authentication is needed
conf["authenticate"] = True if self.interface in ifaces else False conf["authenticate"] = True if self.interface in ifaces else False
else: else:
logger.error( error_message = (
"expecting 'all', 'False' or a list for " "expecting 'all', 'False' or a list for "
"configuration 'authenticate', got %r", "configuration 'authenticate', got %r" % ifaces,
ifaces,
) )
raise MoulinetteError("error_see_log") logger.error(error_message)
raise MoulinetteError(error_message, raw_msg=True)
# -- 'authenticator' # -- 'authenticator'
auth = configuration.get("authenticator", "default") auth = configuration.get("authenticator", "default")
if not is_global and isinstance(auth, str): if not is_global and isinstance(auth, str):
# Store needed authenticator profile # Store needed authenticator profile
if auth not in self.global_conf["authenticator"]: if auth not in self.global_conf["authenticator"]:
logger.error( error_message = (
"requesting profile '%s' which is undefined in " "requesting profile '%s' which is undefined in "
"global configuration of 'authenticator'", "global configuration of 'authenticator'" % auth,
auth,
) )
raise MoulinetteError("error_see_log") logger.error(error_message)
raise MoulinetteError(error_message, raw_msg=True)
else: else:
conf["authenticator"] = auth conf["authenticator"] = auth
elif is_global and isinstance(auth, dict): elif is_global and isinstance(auth, dict):
@ -286,12 +286,13 @@ class BaseActionsMapParser(object):
} }
conf["authenticator"] = auths conf["authenticator"] = auths
else: else:
logger.error( error_message = (
"expecting a dict of profile(s) or a profile name " "expecting a dict of profile(s) or a profile name "
"for configuration 'authenticator', got %r", "for configuration 'authenticator', got %r",
auth, auth,
) )
raise MoulinetteError("error_see_log") logger.error(error_message)
raise MoulinetteError(error_message, raw_msg=True)
return conf return conf
@ -371,12 +372,13 @@ class _CallbackAction(argparse.Action):
try: try:
# Execute callback and get returned value # Execute callback and get returned value
value = self.callback(namespace, values, **self.callback_kwargs) value = self.callback(namespace, values, **self.callback_kwargs)
except Exception: except Exception as e:
logger.exception( error_message = (
"cannot get value from callback method " "cannot get value from callback method "
"'{0}'".format(self.callback_method) "'{0}': {1}".format(self.callback_method, e)
) )
raise MoulinetteError("error_see_log") logger.exception(error_message)
raise MoulinetteError(error_message, raw_msg=True)
else: else:
if value: if value:
if self.callback_return: if self.callback_return:

View file

@ -675,9 +675,13 @@ class ActionsMapParser(BaseActionsMapParser):
try: try:
# Retrieve the tid for the route # Retrieve the tid for the route
tid, _ = self._parsers[kwargs.get("route")] tid, _ = self._parsers[kwargs.get("route")]
except KeyError: except KeyError as e:
logger.error("no argument parser found for route '%s'", kwargs.get("route")) error_message = "no argument parser found for route '%s': %s" % (
raise MoulinetteError("error_see_log") kwargs.get("route"),
e,
)
logger.error(error_message)
raise MoulinetteError(error_message, raw_msg=True)
if self.get_conf(tid, "authenticate"): if self.get_conf(tid, "authenticate"):
authenticator = self.get_conf(tid, "authenticator") authenticator = self.get_conf(tid, "authenticator")
@ -703,9 +707,10 @@ class ActionsMapParser(BaseActionsMapParser):
try: try:
# Retrieve the parser for the route # Retrieve the parser for the route
_, parser = self._parsers[route] _, parser = self._parsers[route]
except KeyError: except KeyError as e:
logger.error("no argument parser found for route '%s'", route) error_message = "no argument parser found for route '%s': %s" % (route, e)
raise MoulinetteError("error_see_log") logger.error(error_message)
raise MoulinetteError(error_message, raw_msg=True)
ret = argparse.Namespace() ret = argparse.Namespace()
# TODO: Catch errors? # TODO: Catch errors?
@ -825,10 +830,15 @@ class Interface(BaseInterface):
server = WSGIServer((host, port), self._app, handler_class=WebSocketHandler) server = WSGIServer((host, port), self._app, handler_class=WebSocketHandler)
server.serve_forever() server.serve_forever()
except IOError as e: except IOError as e:
logger.exception("unable to start the server instance on %s:%d", host, port) error_message = "unable to start the server instance on %s:%d: %s" % (
host,
port,
e,
)
logger.exception(error_message)
if e.args[0] == errno.EADDRINUSE: if e.args[0] == errno.EADDRINUSE:
raise MoulinetteError("server_already_running") raise MoulinetteError("server_already_running")
raise MoulinetteError("error_see_log") raise MoulinetteError(error_message)
# Routes handlers # Routes handlers

View file

@ -380,9 +380,13 @@ class ActionsMapParser(BaseActionsMapParser):
ret = self._parser.parse_args(args) ret = self._parser.parse_args(args)
except SystemExit: except SystemExit:
raise raise
except Exception: except Exception as e:
logger.exception("unable to parse arguments '%s'", " ".join(args)) error_message = "unable to parse arguments '%s' because: %s" % (
raise MoulinetteError("error_see_log") " ".join(args),
e,
)
logger.exception(error_message)
raise MoulinetteError(error_message, raw_msg=True)
tid = getattr(ret, "_tid", None) tid = getattr(ret, "_tid", None)
if self.get_conf(tid, "authenticate"): if self.get_conf(tid, "authenticate"):
@ -404,9 +408,13 @@ class ActionsMapParser(BaseActionsMapParser):
ret = self._parser.parse_args(args) ret = self._parser.parse_args(args)
except SystemExit: except SystemExit:
raise raise
except Exception: except Exception as e:
logger.exception("unable to parse arguments '%s'", " ".join(args)) error_message = "unable to parse arguments '%s' because: %s" % (
raise MoulinetteError("error_see_log") " ".join(args),
e,
)
logger.exception(error_message)
raise MoulinetteError(error_message, raw_msg=True)
else: else:
self.prepare_action_namespace(getattr(ret, "_tid", None), ret) self.prepare_action_namespace(getattr(ret, "_tid", None), ret)
self._parser.dequeue_callbacks(ret) self._parser.dequeue_callbacks(ret)

View file

@ -27,7 +27,7 @@ def patch_translate(moulinette):
def new_translate(self, key, *args, **kwargs): def new_translate(self, key, *args, **kwargs):
if key not in self._translations[self.default_locale].keys(): if key not in self._translations[self.default_locale].keys():
message = "Unable to retrieve key %s for default locale!" % key message = "Unable to retrieve key '%s' for default locale!" % key
raise KeyError(message) raise KeyError(message)
return old_translate(self, key, *args, **kwargs) return old_translate(self, key, *args, **kwargs)

View file

@ -188,7 +188,7 @@ def test_extra_argument_parser_add_argument_bad_arg(iface):
with pytest.raises(MoulinetteError) as exception: with pytest.raises(MoulinetteError) as exception:
extra_argument_parse.add_argument(GLOBAL_SECTION, "foo", {"ask": 1}) extra_argument_parse.add_argument(GLOBAL_SECTION, "foo", {"ask": 1})
translation = m18n.g("error_see_log") translation = m18n.g("error")
expected_msg = translation.format() expected_msg = translation.format()
assert expected_msg in str(exception) assert expected_msg in str(exception)
@ -265,7 +265,7 @@ def test_actions_map_import_error(mocker):
with pytest.raises(MoulinetteError) as exception: with pytest.raises(MoulinetteError) as exception:
amap.process({}, timeout=30, route=("GET", "/test-auth/none")) amap.process({}, timeout=30, route=("GET", "/test-auth/none"))
translation = m18n.g("error_see_log") translation = m18n.g("error")
expected_msg = translation.format() expected_msg = translation.format()
assert expected_msg in str(exception) assert expected_msg in str(exception)