Skip to content

Add null checks when fetching culprits in builds with SCM #26261

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

Open
prathamalwayscomeslast wants to merge 8 commits into
base: master
Choose a base branch
from
+98 −1
Open

Add null checks when fetching culprits in builds with SCM #26261

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
32f331f
Fix: #26256.
prathamalwayscomeslast Feb 4, 2026
d9cf838
New: RunWithSCMTest, handling culprit deserialization issues
prathamalwayscomeslast Feb 15, 2026
c50b632
Merge branch 'master' into culprit-npe-fix
prathamalwayscomeslast Feb 15, 2026
f2e1fa4
Resolved spotbugs issues in RunWithSCM.
prathamalwayscomeslast Feb 20, 2026
3692d14
Merge remote-tracking branch 'origin/culprit-npe-fix' into culprit-np…
prathamalwayscomeslast Feb 20, 2026
4a50e56
Merge branch 'master' into culprit-npe-fix
prathamalwayscomeslast Feb 20, 2026
7e0a510
Merge branch 'master' into culprit-npe-fix
MarkEWaite Feb 21, 2026
bce2f61
Merge branch 'master' into culprit-npe-fix
prathamalwayscomeslast Mar 9, 2026
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
15 changes: 14 additions & 1 deletion core/src/main/java/jenkins/scm/RunWithSCM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.model.Job;
import hudson.model.Result;
import hudson.model.Run;
Expand All @@ -34,6 +35,7 @@
import hudson.scm.SCM;
import hudson.util.AdaptedIterator;
import java.util.AbstractSet;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -87,14 +89,25 @@
* @return
* can be empty but never null.
*/
@SuppressFBWarnings(value = "DCN_NULLPOINTER_EXCEPTION", justification = "Handles corrupted UnmodifiableCollection from XStream deserialization of older configs")
@Exported
@NonNull default Set<User> getCulprits() {
if (shouldCalculateCulprits()) {
return calculateCulprits();
}

Set<String> ids = getCulpritIds();

return new AbstractSet<>() {

Check warning on line 101 in core/src/main/java/jenkins/scm/RunWithSCM.java

View check run for this annotation

ci.jenkins.io / SpotBugs

US_USELESS_SUPPRESSION_ON_METHOD 

NORMAL:
Suppressing annotation DCN_NULLPOINTER_EXCEPTION on the method jenkins.scm.RunWithSCM.getCulprits() is unnecessary
Raw output 
no message found
private Set<String> culpritIds = Set.copyOf(getCulpritIds());
private Set<String> culpritIds;

{
try {
culpritIds = (ids == null) ? Collections.emptySet() : Set.copyOf(ids);

Check warning on line 106 in core/src/main/java/jenkins/scm/RunWithSCM.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 106 is only partially covered, one branch is missing
} catch (NullPointerException e) {

Check warning on line 107 in core/src/main/java/jenkins/scm/RunWithSCM.java

View check run for this annotation

ci.jenkins.io / SpotBugs

DCN_NULLPOINTER_EXCEPTION 

NORMAL:
Do not catch NullPointerException like in new jenkins.scm.RunWithSCM$1(RunWithSCM, Set)
Raw output 
no message found
culpritIds = Collections.emptySet();

Check warning on line 108 in core/src/main/java/jenkins/scm/RunWithSCM.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 107-108 are not covered by tests
}
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The NullPointerException is caught but silently swallowed without any logging. This makes it difficult to diagnose issues with corrupted build configurations. Based on the codebase convention seen in other files (e.g., hudson/util/Scrambler.java:56 logs "Corrupted data" at WARNING level), exceptions related to data corruption should be logged. Consider adding logging at WARNING or INFO level to help users identify when builds have corrupted culprit data, which would aid in troubleshooting and allow administrators to take corrective action on affected builds.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@prathamalwayscomeslast prathamalwayscomeslast Feb 20, 2026

Choose a reason for hiding this comment

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

This catch block is removed now, so logging isn't needed

}

@Override
public Iterator<User> iterator() {
Expand Down
Loading