From 73b412fa4025b10cb1f33450556d040ba07d5b5d Mon Sep 17 00:00:00 2001 From: Martin Kolman Date: Mon, 14 Oct 2024 15:01:13 +0200 Subject: [PATCH] Fix crash on continue after a missing package non-critical error Due to the way how exceptions propagate from the Anaconda installation tasks we ended up with a non-critical error exception interrupting the iteration of an important task iteration loop installing the payload. Due to not being handled "deep enough" but instead "bubbling up" to a top level error handler the loop apparently gets interrupted & remaining tasks skipped. This resulted in an unrelated crash ("kernel version list no available") due to all the important installation tasks being skipped, packages not being installed and installation related data not being populated. The end result was that a non-critical error (such as a missing package) would trigger a dialog asking the user to quit or continue - but clicking "continue" would result in a weird crash. So move the error handler check closer to the task execution, to prevent the loop from being interrupted. That way the loop will resume its iteration if "continue" is clicked in the UI. Also convert the non-critical error if it gets raised (user deciding *not* to continue after the non-critical error) to a fatal one. This is necessary, as otherwise the top level error handler would get triggered, asking the user again to quit or continue. NOTE: Longer term we really should clean this up & have all installation tasks gathered, ordered and executed from a single place. Then all the error handling could be in a single place, making things much simpler. Resolves: INSTALLER-4045 Related: rhbz#2238045 Related: RHEL-57699 --- pyanaconda/payload/migrated.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pyanaconda/payload/migrated.py b/pyanaconda/payload/migrated.py index 9affb6a32ca..17684f31b17 100644 --- a/pyanaconda/payload/migrated.py +++ b/pyanaconda/payload/migrated.py @@ -23,6 +23,9 @@ from pyanaconda.modules.common.task import sync_run_task from pyanaconda.payload.base import Payload from pyanaconda.ui.lib.payload import get_payload, get_source, set_up_sources +from pyanaconda.errors import errorHandler, ERROR_RAISE +from pyanaconda.modules.common.errors.installation import PayloadInstallationError, \ + NonCriticalInstallationError log = get_module_logger(__name__) @@ -102,7 +105,30 @@ def _run_tasks(self, task_paths, progress_cb=None): if progress_cb: task_proxy.ProgressChanged.connect(progress_cb) - sync_run_task(task_proxy) + # Wrap the task call with error handler here, or else + # the exception will propagate up to the top level error handler. + # It will still show the error/continnue dialog, but would result + # in the rest of the loop being skipped if continue is selected. + # By running the error handler from here, we can correctly continue + # the installation on non-critical errors. + try: + sync_run_task(task_proxy) + except Exception as e: # pylint: disable=broad-except + if errorHandler.cb(e) == ERROR_RAISE: + # check if the error we are raising is a non-critical error + if isinstance(e, NonCriticalInstallationError): + # This means the user has selected "No" on the continue dialog + # for non-critical errors. Unfortunatelly, there is still the top-level + # error handler and if we just raise the non-critical error, + # the user will be asked *again*. That would be a rather bad UX, + # so lets turn the non-ciritcal error into a fatal one. + # This results in a nice error dialog with "Exit Installer" button + # being shown. + raise PayloadInstallationError(str(e)) from e + else: + # raise all other errors as usual & let them to propagate + # to the top level error handler + raise class ActiveDBusPayload(MigratedDBusPayload):