Merged
Conversation
Member
Author
|
(This is a |
MarkEWaite
added a commit
to MarkEWaite/bom
that referenced
this pull request
Aug 6, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While playing with a CloudBees CI setup configured as code, I noticed to my alarm that when a sample Git repository was offline, Jenkins startup failed:
It turns out that there was nothing really wrong in the proprietary plugin:
GlobalTemplateCatalogConfigurator.configurewas catching anIOExceptionthrown fromSCMSource.fetchand proceeding. UnfortunatelyAbstractGitSCMSource.retrievewas passing along aGitExceptionthrown from aGitClientmethod as is, without wrapping in a declared exception permitted by the interface inscm-api.While I could have fixed merely this call site, once I started looking I found that the problem was fairly general in this plugin: in a number of places it is implementing APIs from core or
scm-apiwhich permitIOExceptionbut throwing this unchecked exception. So I decided to fix the general issue. If desired, this PR could be broken up and fixes to just those methods filed separately, though for legibility I find it helpful to be documenting whereGitExceptionis thrown.Initially developed by manually looking up usages of
GitClientmethods throwingGitExceptionand checking their call sites, but it proved much easier to use jenkinsci/git-client-plugin#1166 withGitExceptionchecked and fix all compiler errors. My general principle is to just keeping addingthrows GitExceptionas needed until coming up to an interface defined in a dependency which does not allow it, and at that point wrapping it. Note that a number of places in the plugin already caughtGitException, though generally printing an error to aTaskListenerand then signaling some sort of failure separately rather than using it as acause.