diff --git a/src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java b/src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java index fe8effbf..a6203355 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java +++ b/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 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(); } } diff --git a/src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java b/src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java index 8f4f0f04..98de089c 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java @@ -124,7 +124,7 @@ BlockStartNode bruteForceScanForEnclosingBlock(@NonNull final FlowNode node) { Set 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 diff --git a/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearBlockHoppingScanner.java b/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearBlockHoppingScanner.java index 3e4209dd..73720a4e 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearBlockHoppingScanner.java +++ b/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)) { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java b/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java index 47a15707..5981d714 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java +++ b/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"))); }); } diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/build.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/build.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/build.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/build.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/log b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/log similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/log rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/log diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/log-index b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/log-index similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/log-index rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/log-index diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/program.dat b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/program.dat similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/program.dat rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/program.dat diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/10.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/10.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/10.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/10.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/2.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/2.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/2.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/2.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/3.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/3.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/3.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/3.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/4.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/4.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/4.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/4.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/5.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/5.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/5.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/5.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/6.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/6.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/6.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/6.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/7.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/7.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/7.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/7.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/8.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/8.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/8.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/8.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/9.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/9.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/1/workflow/9.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/1/workflow/9.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/legacyIds b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/legacyIds similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/builds/legacyIds rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/builds/legacyIds diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/config.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/config.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/config.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/config.xml diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/nextBuildNumber b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/nextBuildNumber similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/jobs/test0/nextBuildNumber rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/jobs/test0/nextBuildNumber diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml b/src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithLoop/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml rename to src/test/resources/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest/resumeStepExecutionsWithCorruptFlowGraphWithCycle/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml