Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix crash on continue after a missing package non-critical error #5942

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Fix crash on continue after a missing package non-critical error #5942

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion pyanaconda/payload/migrated.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I dont get why we need to check fro ERROR_RAISE, handle broad Exception and raise again.
If we dont handle it it will be just propagate up. What am I missing here?

I mean convert this to::

try:
    sync_run_task(task_proxy)
except NonCriticalInstallationError as e:
    # Handle the non-critical error by raising it as a fatal PayloadInstallationError
    raise PayloadInstallationError(str(e)) from e

# raise all other errors as usual & let them to propagate
# to the top level error handler
raise


class ActiveDBusPayload(MigratedDBusPayload):
Expand Down
Loading