-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
Recover views after error in Jenkins.load
#10023
Conversation
….primaryView` is null
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.
FWIW, there was a similar issue with views in folders caused by HashiCorp Vault plugin, fixed by jenkinsci/hashicorp-vault-plugin#336 and made more robust in core by #9653, leading to the same recursion as in jenkinsci/cloudbees-folder-plugin#447.
Do you have an OldDataMonitor
warning for an observed loading failure? I wonder if any other XStream robustness improvements are possible here.
No, |
/label ready-for-merge This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. Thanks! |
I had occasionally seen mysterious errors in CloudBees CI which I finally tracked down by observing that they came after a warning about reloading
$JENKINS_HOME/config.xml
from disk. #8413 made this somewhat safer but not enough.(CloudBees-internal reference)
Testing done
Without the fix, the assertions about
.primaryView
and.views
fail. More interestingly, while browsing the dashboard (goTo("")
) simply fails with a 404 (Stapler trace is pretty obvious: looking up the primary view fails since it is null), browsing some other pages produces twoStackOverflowError
s from Stapler/Jelly:and
which seems to be caused by an unfortunate default logic in
IncludeTag
resulting in an infinite recursion whenit=null
.Proposed changelog entries
Proposed upgrade guidelines
N/A
Desired reviewers
@Vlatombe
Before the changes are marked as
ready-for-merge
:Maintainer checklist