github-branch-source-plugin
github-branch-source-plugin copied to clipboard
Return null for unmergable heads rather than excepting
Description
Triggered by https://github.com/jenkinsci/checks-api-plugin/pull/83.
I have zombie jobs (job is aborted, but agents remain busy executing a dead job) seemingly triggered by the following stack trace:
00:32:24.679 hudson.AbortException: Pull request 5852 : Not mergeable at xxxxxxxx+xxxxxxxxxxxx (NOT_MERGEABLE)
00:32:24.679 at org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision.validateMergeHash(PullRequestSCMRevision.java:106)
00:32:24.679 at org.jenkinsci.plugins.github_branch_source.GitHubSCMSource.retrieve(GitHubSCMSource.java:1599)
00:32:24.679 at jenkins.scm.api.SCMSource.fetch(SCMSource.java:582)
00:32:24.679 at io.jenkins.plugins.checks.github.SCMFacade.findRevision(SCMFacade.java:156)
00:32:24.679 Caused: java.lang.IllegalStateException: Could not fetch revision from repository: xxxxxxxxxxxx
00:32:24.679 at io.jenkins.plugins.checks.github.SCMFacade.findRevision(SCMFacade.java:159)
00:32:24.679 at io.jenkins.plugins.checks.github.GitHubSCMSourceChecksContext.resolveHeadSha(GitHubSCMSourceChecksContext.java:131)
00:32:24.679 at io.jenkins.plugins.checks.github.GitHubSCMSourceChecksContext.<init>(GitHubSCMSourceChecksContext.java:46)
00:32:24.679 at io.jenkins.plugins.checks.github.GitHubSCMSourceChecksContext.fromRun(GitHubSCMSourceChecksContext.java:24)
00:32:24.679 at io.jenkins.plugins.checks.github.GitHubChecksPublisherFactory.createPublisher(GitHubChecksPublisherFactory.java:42)
00:32:24.679 at io.jenkins.plugins.checks.api.ChecksPublisherFactory.lambda$fromRun$0(ChecksPublisherFactory.java:89)
...
00:32:24.679 at io.jenkins.plugins.checks.api.ChecksPublisherFactory.fromRun(ChecksPublisherFactory.java:92)
00:32:24.679 at io.jenkins.plugins.checks.api.ChecksPublisherFactory.fromRun(ChecksPublisherFactory.java:69)
00:32:24.679 at io.jenkins.plugins.checks.steps.WithChecksStep$WithChecksStepExecution.publish(WithChecksStep.java:150)
00:32:24.679 at io.jenkins.plugins.checks.steps.WithChecksStep$WithChecksStepExecution.access$100(WithChecksStep.java:83)
00:32:24.679 at io.jenkins.plugins.checks.steps.WithChecksStep$WithChecksStepExecution$WithChecksCallBack.onStart(WithChecksStep.java:169)
The environmental behaviour that lead to this I believe was my force-pushing to the PR.
Now while there are separate issues in many different plugins that lead to this, the root exception in validateMergeHash
I believe should in fact be handled in its direct caller.
This PR updates https://github.com/jenkinsci/github-branch-source-plugin/blob/ea3b723b4f12fbc3f06d985c9cc81dccea1ae2ae/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java#L1590 to now return null
if given an unmergeable PR head rather than excepting.
This should not affect downstream consumers as this API is marked as @CheckForNull
.
Note that this would bring it line with
https://github.com/jenkinsci/github-branch-source-plugin/blob/ea3b723b4f12fbc3f06d985c9cc81dccea1ae2ae/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java#L1328
which does return null
for unmergeable heads (despite the API not being marked @CheckForNull
).
I'm not certain that this is the best/correct way to solve this problem; I did look at fixing it in the github-checks-api
plugin, but I did not want to leak the checking of this specific and ignorable issue across modules; as validateMergeHash
only throws a generic AbortException
it is indistinguishable from the many other uses of AbortExceptions
throughout this plugin.
Submitter checklist
- [ ] Link to JIRA ticket in description, if appropriate.
- [ ] Change is code complete and matches issue description
- [ ] Automated tests have been added to exercise the changes
- [ ] Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.
Reviewer checklist
- [ ] Run the changes and verify that the change matches the issue description
- [ ] Reviewed the code
- [ ] Verified that the appropriate tests have been written or valid explanation given
Documentation changes
- [ ] Link to jenkins.io PR, or an explanation for why no doc changes are needed
Users/aliases to notify
cc: @timja
Having another look through the code, since the simplifications in https://github.com/jenkinsci/github-branch-source-plugin/pull/226 it may be that validateMergeHash
could just return a bool
.