If a project action fails to load we should log the project action and...

If a project action fails to load we should log the project action and continue to load the project.

core/src/main/java/hudson/model/AbstractProject.java

@@ -751,8 +751,13 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
         for (JobProperty<? super P> p : Util.fixNull(properties))
             ta.addAll(p.getJobActions((P)this));
 
-        for (TransientProjectActionFactory tpaf : TransientProjectActionFactory.all())
-            ta.addAll(Util.fixNull(tpaf.createFor(this))); // be defensive against null
+        for (TransientProjectActionFactory tpaf : TransientProjectActionFactory.all()) {
+            try {
+                ta.addAll(Util.fixNull(tpaf.createFor(this))); // be defensive against null
+            } catch (Exception e) {
+                LOGGER.log(Level.SEVERE, "Error loading transient project action factory.");
+            }
+        }
         return ta;
     }
 

core/src/main/java/hudson/model/Actionable.java

@@ -30,6 +30,9 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 import jenkins.model.ModelObjectWithContextMenu;
 import jenkins.model.TransientActionFactory;
 import org.kohsuke.stapler.StaplerRequest;
@@ -91,7 +94,11 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj
         List<Action> _actions = new ArrayList<Action>(getActions());
         for (TransientActionFactory<?> taf : ExtensionList.lookup(TransientActionFactory.class)) {
             if (taf.type().isInstance(this)) {
-                _actions.addAll(createFor(taf));
+                try {
+                    _actions.addAll(createFor(taf));
+                } catch (Exception e) {
+                    LOGGER.log(Level.SEVERE, "Error loading action.", e);
+                }
             }
         }
         return Collections.unmodifiableList(_actions);
@@ -177,4 +184,6 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj
     @Override public ContextMenu doContextMenu(StaplerRequest request, StaplerResponse response) throws Exception {
         return new ContextMenu().from(this,request,response);
     }
+
+    private static final Logger LOGGER = Logger.getLogger(Actionable.class.getName());
 }

core/src/main/java/hudson/model/Project.java

@@ -52,6 +52,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 import jenkins.triggers.SCMTriggerItem;
 
 /**
@@ -234,15 +237,37 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     protected List<Action> createTransientActions() {
         List<Action> r = super.createTransientActions();
 
-        for (BuildStep step : getBuildersList())
-            r.addAll(step.getProjectActions(this));
-        for (BuildStep step : getPublishersList())
-            r.addAll(step.getProjectActions(this));
-        for (BuildWrapper step : getBuildWrappers().values())
-            r.addAll(step.getProjectActions(this));
-        for (Trigger trigger : triggers())
-            r.addAll(trigger.getProjectActions());
+        for (BuildStep step : getBuildersList()) {
+            try {
+                r.addAll(step.getProjectActions(this));
+            } catch (Exception e) {
+                LOGGER.log(Level.SEVERE, "Error loading build step.", e);
+            }
+        }
+        for (BuildStep step : getPublishersList()) {
+            try {
+                r.addAll(step.getProjectActions(this));
+            } catch (Exception e) {
+                LOGGER.log(Level.SEVERE, "Error loading publisher.", e);
+            }
+        }
+        for (BuildWrapper step : getBuildWrappers().values()) {
+            try {
+                r.addAll(step.getProjectActions(this));
+            } catch (Exception e) {
+                LOGGER.log(Level.SEVERE, "Error loading build wrapper.", e);
+            }
+        }
+        for (Trigger trigger : triggers()) {
+            try {
+                r.addAll(trigger.getProjectActions());
+            } catch (Exception e) {
+                LOGGER.log(Level.SEVERE, "Error loading trigger.", e);
+            }
+        }
 
         return r;
     }
+
+    private static final Logger LOGGER = Logger.getLogger(Project.class.getName());
 }

test/src/test/java/hudson/model/ItemGroupMixInTest.java

@@ -24,11 +24,15 @@
 
 package hudson.model;
 
-import java.util.Collection;
-import static org.junit.Assert.*;
 import hudson.Extension;
+import hudson.tasks.BuildStepDescriptor;
+import hudson.tasks.BuildStepMonitor;
+import hudson.tasks.BuildTrigger;
 import hudson.tasks.BuildWrapper;
 import hudson.tasks.BuildWrapperDescriptor;
+import hudson.tasks.Builder;
+import hudson.tasks.Publisher;
+import hudson.triggers.Trigger;
 import org.apache.commons.io.FileUtils;
 import org.junit.Rule;
 import org.junit.Test;
@@ -39,8 +43,14 @@ import org.jvnet.hudson.test.TestExtension;
 import org.jvnet.hudson.test.recipes.LocalData;
 
 import java.io.File;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
 public class ItemGroupMixInTest {
 
     @Rule public JenkinsRule r = new JenkinsRule();
@@ -103,12 +113,17 @@ public class ItemGroupMixInTest {
     MockFolder d = r.jenkins.getItemByFullName("d", MockFolder.class);
     assertNotNull(d);
     Collection<TopLevelItem> items = d.getItems();
-    assertEquals(1, items.size());
-    assertEquals("valid", items.iterator().next().getName());
+    assertEquals(5, items.size());
+    Iterator<TopLevelItem> iterator = items.iterator();
+    assertEquals("badBuildStep", iterator.next().getName());
+    assertEquals("badBuildTrigger", iterator.next().getName());
+    assertEquals("badBuildWrapper", iterator.next().getName());
+    assertEquals("badPublisher", iterator.next().getName());
+    assertEquals("valid", iterator.next().getName());
   }
 
   @TestExtension
-  public static class MockBuilderThrowsError extends BuildWrapper {
+  public static class MockBuildWrapperThrowsError extends BuildWrapper {
     @Override
     public Collection<? extends Action> getProjectActions(AbstractProject project){
       throw new NullPointerException();
@@ -127,4 +142,62 @@ public class ItemGroupMixInTest {
       }
     }
   }
+
+  @TestExtension
+  public static class MockBuilderThrowsError extends Builder {
+    @Override
+    public Collection<? extends Action> getProjectActions(AbstractProject project){
+      throw new NullPointerException();
+    }
+    @Extension public static final Descriptor DESCRIPTOR = new DescriptorImpl();
+
+    public static class DescriptorImpl extends BuildStepDescriptor {
+      @Override
+      public boolean isApplicable(Class jobType) {
+        return false;
+      }
+
+      @Override
+      public String getDisplayName() {
+        return null;
+      }
+    }
+  }
+
+  @TestExtension
+  public static class MockBuildTriggerThrowsError extends Trigger {
+    @Override
+    public Collection<? extends Action> getProjectActions() {
+      throw new NullPointerException();
+    }
+
+    @Extension public static final Descriptor DESCRIPTOR = new BuildTrigger.DescriptorImpl();
+  }
+
+  @TestExtension
+  public static class MockPublisherThrowsError extends Publisher {
+    @Override
+    public Collection<? extends Action> getProjectActions(AbstractProject project) {
+      throw new NullPointerException();
+    }
+
+    @Override
+    public BuildStepMonitor getRequiredMonitorService() {
+      return null;
+    }
+
+    @Extension public static final Descriptor DESCRIPTOR = new DescriptorImpl();
+
+    public static class DescriptorImpl extends BuildStepDescriptor {
+      @Override
+      public boolean isApplicable(Class jobType) {
+        return false;
+      }
+
+      @Override
+      public String getDisplayName() {
+        return null;
+      }
+    }
+  }
 }