Improve error management for app restore, similar to what's done in app install

locales/ca.json

@@ -327,7 +327,7 @@
     "regenconf_failed": "No s'ha pogut regenerar la configuració per la/les categoria/es : {categories}",
     "regenconf_pending_applying": "Aplicació de la configuració pendent per la categoria «{category}»...",
     "restore_already_installed_app": "Una aplicació amb la ID «{app:s}» ja està instal·lada",
-    "restore_app_failed": "No s'ha pogut restaurar {app:s}",
+    "app_restore_failed": "No s'ha pogut restaurar {app:s}: {error:s}",
     "restore_cleaning_failed": "No s'ha pogut netejar el directori temporal de restauració",
     "restore_complete": "Restauració completada",
     "restore_confirm_yunohost_installed": "Esteu segur de voler restaurar un sistema ja instal·lat? [{answers:s}]",

locales/de.json

@@ -96,7 +96,7 @@
     "port_already_closed": "Der Port {port:d} wurde bereits für {ip_version:s} Verbindungen geschlossen",
     "port_already_opened": "Der Port {port:d} wird bereits von {ip_version:s} benutzt",
     "restore_already_installed_app": "Es ist bereits eine App mit der ID '{app:s}' installiet",
-    "restore_app_failed": "App '{app:s}' konnte nicht wiederhergestellt werden",
+    "app_restore_failed": "App '{app:s}' konnte nicht wiederhergestellt werden: {error:s}",
     "restore_cleaning_failed": "Das temporäre Wiederherstellungsverzeichnis konnte nicht geleert werden",
     "restore_complete": "Wiederherstellung abgeschlossen",
     "restore_confirm_yunohost_installed": "Möchtest du die Wiederherstellung wirklich starten? [{answers:s}]",

locales/en.json

@@ -523,7 +523,7 @@
     "regex_with_only_domain": "You can't use a regex for domain, only for path",
     "restore_already_installed_app": "An app with the ID '{app:s}' is already installed",
     "restore_already_installed_apps": "The following apps can't be restored because they are already installed: {apps}",
-    "restore_app_failed": "Could not restore {app:s}",
+    "restore_app_failed": "Could not restore {app:s}: {error: s}",
     "restore_backup_too_old": "This backup archive can not be restored because it comes from a too-old YunoHost version.",
     "restore_cleaning_failed": "Could not clean up the temporary restoration directory",
     "restore_complete": "Restoration completed",

locales/eo.json

@@ -479,7 +479,7 @@
     "service_restarted": "Servo '{service:s}' rekomencis",
     "pattern_username": "Devas esti minuskulaj literoj kaj minuskloj nur",
     "extracting": "Eltirante…",
-    "restore_app_failed": "Ne povis restarigi la programon '{app:s}'",
+    "app_restore_failed": "Ne povis restarigi la programon '{app:s}': {error:s}",
     "yunohost_configured": "YunoHost nun estas agordita",
     "certmanager_self_ca_conf_file_not_found": "Ne povis trovi agorddosieron por mem-subskriba aŭtoritato (dosiero: {file:s})",
     "log_app_remove": "Forigu la aplikon '{}'",

locales/es.json

@@ -107,7 +107,7 @@
     "port_already_closed": "El puerto {port:d} ya está cerrado para las conexiones {ip_version:s}",
     "port_already_opened": "El puerto {port:d} ya está abierto para las conexiones {ip_version:s}",
     "restore_already_installed_app": "Una aplicación con el ID «{app:s}» ya está instalada",
-    "restore_app_failed": "No se pudo restaurar la aplicación «{app:s}»",
+    "app_restore_failed": "No se pudo restaurar la aplicación «{app:s}»: {error:s}",
     "restore_cleaning_failed": "No se pudo limpiar el directorio temporal de restauración",
     "restore_complete": "Restaurada",
     "restore_confirm_yunohost_installed": "¿Realmente desea restaurar un sistema ya instalado? [{answers:s}]",

locales/fr.json

@@ -107,7 +107,7 @@
     "port_already_closed": "Le port {port:d} est déjà fermé pour les connexions {ip_version:s}",
     "port_already_opened": "Le port {port:d} est déjà ouvert pour les connexions {ip_version:s}",
     "restore_already_installed_app": "Une application est déjà installée avec l’identifiant '{app:s}'",
-    "restore_app_failed": "Impossible de restaurer '{app:s}'",
+    "app_restore_failed": "Impossible de restaurer '{app:s}': {error:s}",
     "restore_cleaning_failed": "Impossible de nettoyer le dossier temporaire de restauration",
     "restore_complete": "Restauration terminée",
     "restore_confirm_yunohost_installed": "Voulez-vous vraiment restaurer un système déjà installé ? [{answers:s}]",

locales/it.json

@@ -120,7 +120,7 @@
     "pattern_username": "Caratteri minuscoli alfanumerici o trattini bassi soli",
     "port_already_closed": "La porta {port:d} è già chiusa per le connessioni {ip_version:s}",
     "restore_already_installed_app": "Un'applicazione con l'ID '{app:s}' è già installata",
-    "restore_app_failed": "Impossibile ripristinare l'applicazione '{app:s}'",
+    "app_restore_failed": "Impossibile ripristinare l'applicazione '{app:s}': {error:s}",
     "restore_cleaning_failed": "Impossibile pulire la directory temporanea di ripristino",
     "restore_complete": "Ripristino completo",
     "restore_confirm_yunohost_installed": "Sei sicuro di volere ripristinare un sistema già installato? {answers:s}",

locales/nl.json

@@ -54,7 +54,7 @@
     "pattern_password": "Wachtwoord moet tenminste 3 karakters lang zijn",
     "port_already_closed": "Poort {port:d} is al gesloten voor {ip_version:s} verbindingen",
     "port_already_opened": "Poort {port:d} is al open voor {ip_version:s} verbindingen",
-    "restore_app_failed": "De app '{app:s}' kon niet worden terug gezet",
+    "app_restore_failed": "De app '{app:s}' kon niet worden terug gezet: {error:s}",
     "restore_hook_unavailable": "De herstel-hook '{part:s}' is niet beschikbaar op dit systeem",
     "service_add_failed": "Kan service '{service:s}' niet toevoegen",
     "service_already_started": "Service '{service:s}' draait al",

locales/oc.json

@@ -170,7 +170,7 @@
     "port_already_closed": "Lo pòrt {port:d} es ja tampat per las connexions {ip_version:s}",
     "port_already_opened": "Lo pòrt {port:d} es ja dubèrt per las connexions {ip_version:s}",
     "restore_already_installed_app": "Una aplicacion es ja installada amb l’id « {app:s} »",
-    "restore_app_failed": "Impossible de restaurar l’aplicacion « {app:s} »",
+    "app_restore_failed": "Impossible de restaurar l’aplicacion « {app:s} »: {error:s}",
     "backup_ask_for_copying_if_needed": "Volètz far una salvagarda en utilizant {size:s} Mo temporàriament ? (Aqueste biais de far es emplegat perque unes fichièrs an pas pogut èsser preparats amb un metòde mai eficaç.)",
     "yunohost_not_installed": "YunoHost es pas installat o corrèctament installat. Mercés d’executar « yunohost tools postinstall »",
     "backup_output_directory_forbidden": "Causissètz un repertòri de destinacion deferent. Las salvagardas pòdon pas se realizar dins los repertòris bin, /boot, /dev, /etc, /lib, /root, /run, /sbin, /sys, /usr, /var ou /home/yunohost.backup/archives",

src/yunohost/backup.py

@@ -1392,7 +1392,6 @@ class RestoreManager:
             self.targets.set_result("apps", app_instance_name, "Warning")
             return
 
-        logger.debug(m18n.n("restore_running_app_script", app=app_instance_name))
         try:
             # Restore app settings
             app_settings_new_path = os.path.join(
@@ -1401,7 +1400,7 @@ class RestoreManager:
             app_scripts_new_path = os.path.join(app_settings_new_path, "scripts")
             shutil.copytree(app_settings_in_archive, app_settings_new_path)
             filesystem.chmod(app_settings_new_path, 0o400, 0o400, True)
-            filesystem.chown(app_scripts_new_path, "admin", None, True)
+            filesystem.chown(app_scripts_new_path, "root", None, True)
 
             # Copy the app scripts to a writable temporary folder
             # FIXME : use 'install -Dm555' or something similar to what's done
@@ -1409,7 +1408,7 @@ class RestoreManager:
             tmp_folder_for_app_restore = tempfile.mkdtemp(prefix="restore")
             copytree(app_scripts_in_archive, tmp_folder_for_app_restore)
             filesystem.chmod(tmp_folder_for_app_restore, 0o550, 0o550, True)
-            filesystem.chown(tmp_folder_for_app_restore, "admin", None, True)
+            filesystem.chown(tmp_folder_for_app_restore, "root", None, True)
             restore_script = os.path.join(tmp_folder_for_app_restore, "restore")
 
             # Restore permissions
@@ -1454,81 +1453,111 @@ class RestoreManager:
             os.remove("%s/permissions.yml" % app_settings_new_path)
 
             _tools_migrations_run_before_app_restore(backup_version=self.info["from_yunohost_version"], app_id=app_instance_name)
-
-            # Prepare env. var. to pass to script
-            env_dict = _make_environment_for_app_script(app_instance_name)
-            env_dict.update(
-                {
-                    "YNH_BACKUP_DIR": self.work_dir,
-                    "YNH_BACKUP_CSV": os.path.join(self.work_dir, "backup.csv"),
-                    "YNH_APP_BACKUP_DIR": os.path.join(
-                        self.work_dir, "apps", app_instance_name, "backup"
-                    ),
-                }
-            )
-
-            operation_logger.extra["env"] = env_dict
-            operation_logger.flush()
-
-            # Execute app restore script
-            hook_exec(
-                restore_script,
-                chdir=app_backup_in_archive,
-                raise_on_error=True,
-                env=env_dict,
-            )[0]
         except Exception:
-            msg = m18n.n("restore_app_failed", app=app_instance_name)
+            import traceback
+            error = m18n.n("unexpected_error", error="\n" + traceback.format_exc())
+            msg = m18n.n("app_restore_failed", app=app_instance_name, error=error)
             logger.error(msg)
             operation_logger.error(msg)
 
-            if msettings.get("interface") != "api":
-                dump_app_log_extract_for_debugging(operation_logger)
-
             self.targets.set_result("apps", app_instance_name, "Error")
 
-            remove_script = os.path.join(app_scripts_in_archive, "remove")
+            # Cleanup
-
-            # Setup environment for remove script
-            env_dict_remove = _make_environment_for_app_script(app_instance_name)
-
-            operation_logger = OperationLogger(
-                "remove_on_failed_restore",
-                [("app", app_instance_name)],
-                env=env_dict_remove,
-            )
-            operation_logger.start()
-
-            # Execute remove script
-            if hook_exec(remove_script, env=env_dict_remove)[0] != 0:
-                msg = m18n.n("app_not_properly_removed", app=app_instance_name)
-                logger.warning(msg)
-                operation_logger.error(msg)
-            else:
-                operation_logger.success()
-
-            # Cleaning app directory
             shutil.rmtree(app_settings_new_path, ignore_errors=True)
+            shutil.rmtree(tmp_folder_for_app_restore, ignore_errors=True)
 
-            # Remove all permission in LDAP for this app
+            return
-            for permission_name in user_permission_list()["permissions"].keys():
-                if permission_name.startswith(app_instance_name + "."):
-                    permission_delete(permission_name, force=True)
 
-            # TODO Cleaning app hooks
+        logger.debug(m18n.n("restore_running_app_script", app=app_instance_name))
-        else:
+
-            self.targets.set_result("apps", app_instance_name, "Success")
+        # Prepare env. var. to pass to script
-            operation_logger.success()
+        env_dict = _make_environment_for_app_script(app_instance_name)
+        env_dict.update(
+            {
+                "YNH_BACKUP_DIR": self.work_dir,
+                "YNH_BACKUP_CSV": os.path.join(self.work_dir, "backup.csv"),
+                "YNH_APP_BACKUP_DIR": os.path.join(
+                    self.work_dir, "apps", app_instance_name, "backup"
+                ),
+            }
+        )
+
+        operation_logger.extra["env"] = env_dict
+        operation_logger.flush()
+
+        # Execute the app install script
+        restore_failed = True
+        try:
+            restore_retcode = hook_exec(
+                restore_script,
+                chdir=app_backup_in_archive,
+                env=env_dict,
+            )[0]
+            # "Common" app restore failure : the script failed and returned exit code != 0
+            restore_failed = True if restore_retcode != 0 else False
+            if restore_failed:
+                error = m18n.n("app_restore_script_failed")
+                logger.error(m18n.n("app_restore_failed", app=app_instance_name, error=error))
+                failure_message_with_debug_instructions = operation_logger.error(error)
+                if msettings.get("interface") != "api":
+                    dump_app_log_extract_for_debugging(operation_logger)
+        # Script got manually interrupted ... N.B. : KeyboardInterrupt does not inherit from Exception
+        except (KeyboardInterrupt, EOFError):
+            error = m18n.n("operation_interrupted")
+            logger.error(m18n.n("app_restore_failed", app=app_instance_name, error=error))
+            failure_message_with_debug_instructions = operation_logger.error(error)
+        # Something wrong happened in Yunohost's code (most probably hook_exec)
+        except Exception:
+            import traceback
+            error = m18n.n("unexpected_error", error="\n" + traceback.format_exc())
+            logger.error(m18n.n("app_restore_failed", app=app_instance_name, error=error))
+            failure_message_with_debug_instructions = operation_logger.error(error)
         finally:
             # Cleaning temporary scripts directory
             shutil.rmtree(tmp_folder_for_app_restore, ignore_errors=True)
 
+            if not restore_failed:
+                self.targets.set_result("apps", app_instance_name, "Success")
+                operation_logger.success()
+            else:
+
+                self.targets.set_result("apps", app_instance_name, "Error")
+
+                remove_script = os.path.join(app_scripts_in_archive, "remove")
+
+                # Setup environment for remove script
+                env_dict_remove = _make_environment_for_app_script(app_instance_name)
+
+                remove_operation_logger = OperationLogger(
+                    "remove_on_failed_restore",
+                    [("app", app_instance_name)],
+                    env=env_dict_remove,
+                )
+                remove_operation_logger.start()
+
+                # Execute remove script
+                if hook_exec(remove_script, env=env_dict_remove)[0] != 0:
+                    msg = m18n.n("app_not_properly_removed", app=app_instance_name)
+                    logger.warning(msg)
+                    remove_operation_logger.error(msg)
+                else:
+                    remove_operation_logger.success()
+
+                # Cleaning app directory
+                shutil.rmtree(app_settings_new_path, ignore_errors=True)
+
+                # Remove all permission in LDAP for this app
+                for permission_name in user_permission_list()["permissions"].keys():
+                    if permission_name.startswith(app_instance_name + "."):
+                        permission_delete(permission_name, force=True)
+
+                # TODO Cleaning app hooks
+
+                logger.error(failure_message_with_debug_instructions)
 
 #
 # Backup methods                                                            #
 #
-
-
 class BackupMethod(object):
 
     """

src/yunohost/tests/test_backuprestore.py

@@ -473,7 +473,7 @@ def test_restore_app_script_failure_handling(monkeypatch, mocker):
 
     assert not _is_installed("wordpress")
 
-    with message(mocker, "restore_app_failed", app="wordpress"):
+    with message(mocker, "app_restore_failed", app="wordpress"):
         with raiseYunohostError(mocker, "restore_nothings_done"):
             backup_restore(
                 system=None, name=backup_list()["archives"][0], apps=["wordpress"]