[JENKINS-50407] Better diagnosis for certain fatal loading errors (#809)

* [JENKINS-50407] Better diagnosis for certain fatal loading errors * Abandon elaborate logic in `loadProgramFailed` * `Supplier`-based overload Co-authored-by: Devin Nusbaum <dwnusbaum@users.noreply.github.com> --------- Co-authored-by: Devin Nusbaum <dwnusbaum@users.noreply.github.com>
jenkinsci · Oct 23, 2023 · 769bb74 · 769bb74
1 parent 5d695de
commit 769bb74
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 43 deletions.
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
@@ -28,8 +28,6 @@
 import com.cloudbees.groovy.cps.Env;
 import com.cloudbees.groovy.cps.Envs;
 import com.cloudbees.groovy.cps.Outcome;
-import com.cloudbees.groovy.cps.impl.ConstantBlock;
-import com.cloudbees.groovy.cps.impl.ThrowBlock;
 import com.cloudbees.groovy.cps.sandbox.Invoker;
 import com.cloudbees.jenkins.support.api.Component;
 import com.cloudbees.jenkins.support.api.Container;
@@ -107,6 +105,7 @@
 import hudson.AbortException;
 import hudson.BulkChange;
 import hudson.Extension;
+import hudson.Functions;
 import hudson.init.Terminator;
 import hudson.model.Item;
 import hudson.model.Job;
@@ -840,51 +839,17 @@ public void onFailure(Throwable t) {
     }
 
     /**
-     * Used by {@link #loadProgramAsync(File)} to propagate a failure to load the persisted execution state.
-     * <p>
-     * Let the workflow interrupt by throwing an exception that indicates how it failed.
+     * Used to propagate a failure to load the persisted execution state.
      * @param promise same as {@link #programPromise} but more strongly typed
      */
     private void loadProgramFailed(final Throwable problem, SettableFuture<CpsThreadGroup> promise) {
-        FlowHead head;
-
-        synchronized(this) {
-            if (heads == null || heads.isEmpty()) {
-                head = null;
-            } else {
-                head = getFirstHead();
-            }
-        }
-
-        if (head==null) {
-            // something went catastrophically wrong and there's no live head. fake one
-            head = new FlowHead(this);
-            try {
-                head.newStartNode(new FlowStartNode(this, iotaStr()));
-            } catch (IOException e) {
-                LOGGER.log(Level.FINE, "Failed to persist", e);
-            }
+        try {
+            Functions.printStackTrace(problem, owner.getListener().getLogger());
+        } catch (Exception x) {
+            LOGGER.log(Level.WARNING, x, () -> "failed to log problem to " + owner);
         }
-
-
-        CpsThreadGroup g = new CpsThreadGroup(this);
-        final FlowHead head_ = head;
-
-        promise.set(g);
-        runInCpsVmThread(new FutureCallback<>() {
-            @Override public void onSuccess(CpsThreadGroup g) {
-                CpsThread t = g.addThread(
-                        new Continuable(new ThrowBlock(new ConstantBlock(
-                            problem instanceof AbortException || problem instanceof FlowInterruptedException ? problem : new IOException("Failed to load build state", problem)))),
-                        head_, null
-                );
-                t.resume(new Outcome(null,null));
-            }
-            @Override public void onFailure(Throwable t) {
-                LOGGER.log(Level.WARNING, "Failed to set program failure on " + owner, t);
-                croak(t);
-            }
-        });
+        promise.setException(problem);
+        croak(new AbortException("Failed to load program"));
     }
 
     /** Report a fatal error in the VM. */

diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionTest.java
@@ -608,6 +608,29 @@ private void trustedShell(final boolean pos) throws Throwable {
         });
     }
 
+    @Issue("JENKINS-50407")
+    @Test public void shellLoadingError() throws Throwable {
+        sessions.then(r -> {
+            WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+            p.setDefinition(new CpsFlowDefinition("semaphore 'wait'", true));
+            SemaphoreStep.waitForStart("wait/1", p.scheduleBuild2(0).waitForStart());
+        });
+        sessions.then(r -> {
+            r.assertLogContains("IllegalStateException: decorator problem here",
+                r.assertBuildStatus(Result.FAILURE,
+                    r.waitForCompletion(r.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild())));
+        });
+    }
+    @TestExtension("shellLoadingError") public static final class BrokenDecorator extends GroovyShellDecorator {
+        static int count;
+        @Override
+        public void configureShell(CpsFlowExecution context, GroovyShell shell) {
+            if (count++ == 1) {
+                throw new IllegalStateException("decorator problem here");
+            }
+        }
+    }
+
     /**
      * This field shouldn't be visible to regular script.
      */