-
Notifications
You must be signed in to change notification settings - Fork 354
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.
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
Fix crash on continue after a missing package non-critical error #5942
Fix crash on continue after a missing package non-critical error #5942
Conversation
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. (cherry picked from commit 73b412f) Resolves: RHEL-57699 Resolves: INSTALLER-4045 Related: rhbz#2238045
/kickstart-test --testtype smoke |
1 similar comment
/kickstart-test --testtype smoke |
/kickstart-test --skip-testtypes whatever |
Wow, all 120 test passed with 5 retries - seems to be working, thanks @rvykydal! :) I've also checked the results c for retried tests both sets of results are there, so if we wanted to look for race conditions, we still can do that even with retries on. ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
# This results in a nice error dialog with "Exit Installer" button | ||
# being shown. | ||
raise PayloadInstallationError(str(e)) from e | ||
else: |
There was a problem hiding this comment.
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
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.
(cherry picked from commit 73b412f)
Resolves: RHEL-57699
Resolves: INSTALLER-4045
Related: rhbz#2238045
Backport of #5937 to the RHEL 10 branch.