Adjust implementation to attempt to resume any step that has a FlowNode

Author: dwnusbaum

src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java

@@ -362,15 +362,14 @@ private static final class ParallelResumer {
                         nodes.put(n, se);
                     } else {
                         LOGGER.warning(() -> "Could not find FlowNode for " + se + " so it will not be resumed");
-                        // TODO: Should we call se.getContext().onFailure()?
                     }
                 } catch (IOException | InterruptedException x) {
                     LOGGER.log(Level.WARNING, "Could not look up FlowNode for " + se + " so it will not be resumed", x);
-                    // TODO: Should we call se.getContext().onFailure()?
                 }
             }
-            try {
-                for (FlowNode n : nodes.keySet()) {
+            for (Map.Entry<FlowNode, StepExecution> entry : nodes.entrySet()) {
+                FlowNode n = entry.getKey();
+                try {
                     LinearBlockHoppingScanner scanner = new LinearBlockHoppingScanner();
                     scanner.setup(n);
                     for (FlowNode parent : scanner) {
@@ -379,14 +378,9 @@ private static final class ParallelResumer {
                             break;
                         }
                     }
+                } catch (Exception x) {
+                    LOGGER.log(Level.WARNING, x, () -> "Unable to compute enclosing blocks for " + n + ", so " + entry.getValue() + " might not resume successfully");
                 }
-            } catch (Exception e) {
-                // TODO: Should we instead just try to resume steps with no respect to topological order?
-                // There is something wrong with the FlowGraph. Kill all of the steps so the build fails instead of hanging forever.
-                for (StepExecution se : executions) {
-                    se.getContext().onFailure(e);
-                }
-                onCompletion.run();
             }
         }
 

src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java

@@ -124,7 +124,7 @@ BlockStartNode bruteForceScanForEnclosingBlock(@NonNull final FlowNode node) {
         Set<String> visited = new HashSet<>();
         while (!(current instanceof FlowStartNode)) {  // Hunt back for enclosing blocks, a potentially expensive operation
             if (!visited.add(current.getId())) {
-                throw new IllegalStateException("Loop in flow graph for " + node.getExecution() + " involving " + current);
+                throw new IllegalStateException("Cycle in flow graph for " + node.getExecution() + " involving " + current);
             }
             if (current instanceof BlockEndNode) {
                 // Hop over the block to the start

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearBlockHoppingScanner.java

@@ -91,7 +91,7 @@ protected FlowNode jumpBlockScan(@CheckForNull FlowNode node, @NonNull Collectio
         // Find the first candidate node preceding a block... and filtering by blacklist
         while (candidate instanceof BlockEndNode) {
             if (!visited.add(candidate.getId())) {
-                throw new IllegalStateException("Loop in flow graph for " + candidate.getExecution() + " involving " + candidate);
+                throw new IllegalStateException("Cycle in flow graph for " + candidate.getExecution() + " involving " + candidate);
             }
             candidate = ((BlockEndNode) candidate).getStartNode();
             if (blacklistNodes.contains(candidate)) {

src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java

@@ -41,9 +41,12 @@
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collections;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
 import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import java.util.stream.Collectors;
 import org.hamcrest.Matcher;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
@@ -162,7 +165,7 @@ public class FlowExecutionListTest {
     }
 
     @LocalData
-    @Test public void resumeStepExecutionsWithCorruptFlowGraphWithLoop() throws Throwable {
+    @Test public void resumeStepExecutionsWithCorruptFlowGraphWithCycle() throws Throwable {
         // LocalData created using the following snippet while the build was waiting in the _second_ sleep, except
         // for build.xml, which was captured during the sleep step. The StepEndNode for the stage was then adjusted to
         // have its startId point to the timeout step's StepStartNode, creating a loop.
@@ -175,10 +178,18 @@ public class FlowExecutionListTest {
             r.waitForCompletion(b);
         });
         */
+        logging.capture(50);
         sessions.then(r -> {
             var p = r.jenkins.getItemByFullName("test0", WorkflowJob.class);
             var b = p.getBuildByNumber(1);
-            r.assertBuildStatus(Result.FAILURE, r.waitForCompletion(b));
+            r.waitForCompletion(b);
+            assertThat(logging.getMessages(), hasItem(containsString("Unable to compute enclosing blocks")));
+            var loggedExceptions = logging.getRecords().stream()
+                    .map(LogRecord::getThrown)
+                    .filter(Objects::nonNull)
+                    .map(Throwable::toString)
+                    .collect(Collectors.toList());
+            assertThat(loggedExceptions, hasItem(containsString("Cycle in flow graph")));
         });
     }