diff --git a/locales/en.json b/locales/en.json index 3b51403c8..1d4d37d92 100644 --- a/locales/en.json +++ b/locales/en.json @@ -27,6 +27,7 @@ "app_full_domain_unavailable": "Sorry, this app must be installed on a domain of its own, but other apps are already installed on the domain '{domain}'. You could use a subdomain dedicated to this app instead.", "app_id_invalid": "Invalid app ID", "app_install_failed": "Unable to install {app}: {error}", + "app_resource_failed": "Provisioning, deprovisioning, or updating resources for {app} failed: {error}", "app_install_files_invalid": "These files cannot be installed", "app_install_script_failed": "An error occurred inside the app installation script", "app_label_deprecated": "This command is deprecated! Please use the new command 'yunohost user permission update' to manage the app label.", diff --git a/src/app.py b/src/app.py index 5463fe357..8281f6d43 100644 --- a/src/app.py +++ b/src/app.py @@ -675,13 +675,9 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False if manifest["packaging_format"] >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager( - app_instance_name, wanted=manifest, current=app_dict["manifest"] - ).apply(rollback_if_failure=True) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager( + app_instance_name, wanted=manifest, current=app_dict["manifest"] + ).apply(rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger) # Execute the app upgrade script upgrade_failed = True @@ -1038,13 +1034,9 @@ def app_install( if packaging_format >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager(app_instance_name, wanted=manifest, current={}).apply( - rollback_if_failure=True - ) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager(app_instance_name, wanted=manifest, current={}).apply( + rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger + ) else: # Initialize the main permission for the app # The permission is initialized with no url associated, and with tile disabled @@ -1108,6 +1100,9 @@ def app_install( "Packagers /!\\ This app manually modified some system configuration files! This should not happen! If you need to do so, you should implement a proper conf_regen hook. Those configuration were affected:\n - " + "\n -".join(manually_modified_files_by_app) ) + # Actually forbid this for app packaging >= 2 + if packaging_format >= 2: + broke_the_system = True # If the install failed or broke the system, we remove it if install_failed or broke_the_system: @@ -1157,13 +1152,9 @@ def app_install( if packaging_format >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager( - app_instance_name, wanted={}, current=manifest - ).apply(rollback_if_failure=False) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager( + app_instance_name, wanted={}, current=manifest + ).apply(rollback_and_raise_exception_if_failure=False) else: # Remove all permission in LDAP for permission_name in user_permission_list()["permissions"].keys(): @@ -1288,15 +1279,11 @@ def app_remove(operation_logger, app, purge=False): packaging_format = manifest["packaging_format"] if packaging_format >= 2: - try: - from yunohost.utils.resources import AppResourceManager + from yunohost.utils.resources import AppResourceManager - AppResourceManager(app, wanted={}, current=manifest).apply( - rollback_if_failure=False - ) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager(app, wanted={}, current=manifest).apply( + rollback_and_raise_exception_if_failure=False + ) else: # Remove all permission in LDAP for permission_name in user_permission_list(apps=[app])["permissions"].keys(): diff --git a/src/backup.py b/src/backup.py index 78d52210b..21d499eaf 100644 --- a/src/backup.py +++ b/src/backup.py @@ -1518,13 +1518,9 @@ class RestoreManager: if manifest["packaging_format"] >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager( - app_instance_name, wanted=manifest, current={} - ).apply(rollback_if_failure=True) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager( + app_instance_name, wanted=manifest, current={} + ).apply(rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger) # Execute the app install script restore_failed = True diff --git a/src/utils/resources.py b/src/utils/resources.py index f48722236..4e8388e61 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -36,9 +36,6 @@ logger = getActionLogger("yunohost.app_resources") class AppResourceManager: - # FIXME : add some sort of documentation mechanism - # to create a have a detailed description of each resource behavior - def __init__(self, app: str, current: Dict, wanted: Dict): self.app = app @@ -50,7 +47,7 @@ class AppResourceManager: if "resources" not in self.wanted: self.wanted["resources"] = {} - def apply(self, rollback_if_failure, **context): + def apply(self, rollback_and_raise_exception_if_failure, operation_logger=None, **context): todos = list(self.compute_todos()) completed = [] @@ -69,12 +66,13 @@ class AppResourceManager: elif todo == "update": logger.info(f"Updating {name} ...") new.provision_or_update(context=context) - # FIXME FIXME FIXME : this exception doesnt catch Ctrl+C ?!?! - except Exception as e: + except (KeyboardInterrupt, Exception) as e: exception = e - # FIXME: better error handling ? display stacktrace ? - logger.warning(f"Failed to {todo} for {name} : {e}") - if rollback_if_failure: + if isinstance(e, KeyboardInterrupt): + logger.error(m18n.n("operation_interrupted")) + else: + logger.warning(f"Failed to {todo} {name} : {e}") + if rollback_and_raise_exception_if_failure: rollback = True completed.append((todo, name, old, new)) break @@ -97,12 +95,24 @@ class AppResourceManager: elif todo == "update": logger.info(f"Reverting {name} ...") old.provision_or_update(context=context) - except Exception as e: - # FIXME: better error handling ? display stacktrace ? - logger.error(f"Failed to rollback {name} : {e}") + except (KeyboardInterrupt, Exception) as e: + if isinstance(e, KeyboardInterrupt): + logger.error(m18n.n("operation_interrupted")) + else: + logger.error(f"Failed to rollback {name} : {e}") if exception: - raise exception + if rollback_and_raise_exception_if_failure: + logger.error(m18n.n("app_resource_failed", app=self.app, error=exception)) + if operation_logger: + failure_message_with_debug_instructions = operation_logger.error(str(exception)) + raise YunohostError( + failure_message_with_debug_instructions, raw_msg=True + ) + else: + raise YunohostError(str(exception), raw_msg=True0 + else: + logger.error(exception) def compute_todos(self):