Skip to content

Commit

Permalink
Merge pull request #6 from jglick/ParallelStepException-JENKINS-25894
Browse files Browse the repository at this point in the history 
[JENKINS-25894] Fix reporting of errors from parallel step
  • Loading branch information
@jglick
jglick committed Apr 13, 2016
2 parents 32c5626 + a07839f commit 4fc3310
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 50 deletions.
15 changes: 13 additions & 2 deletions src/main/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStep.java
View file
Expand Up @@ -5,6 +5,7 @@
import groovy.lang.Closure;
import hudson.Extension;
import hudson.model.TaskListener;
import java.io.IOException;

import org.jenkinsci.plugins.workflow.cps.CpsVmThreadOnly;
import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
Expand All @@ -23,6 +24,8 @@
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;
import java.util.logging.Level;
import java.util.logging.Logger;

import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.*;

Expand All @@ -35,6 +38,8 @@
*/
public class ParallelStep extends Step {

private static final Logger LOGGER = Logger.getLogger(ParallelStep.class.getName());

/** should a failure in a parallel branch terminate other still executing branches. */
private final boolean failFast;

Expand Down Expand Up @@ -113,8 +118,15 @@ public void onSuccess(StepContext context, Object result) {
@Override
public void onFailure(StepContext context, Throwable t) {
handler.outcomes.put(name, new Outcome(null, t));
try {
context.get(TaskListener.class).getLogger().println("Failed in branch " + name);
} catch (IOException | InterruptedException x) {
LOGGER.log(Level.WARNING, null, x);
}
if (handler.originalFailure == null) {
handler.originalFailure = new SimpleEntry<String, Throwable>(name, t);
} else {
handler.originalFailure.getValue().addSuppressed(t);
}
checkAllDone(true);
}
Expand Down Expand Up @@ -149,8 +161,7 @@ private void checkAllDone(boolean stepFailed) {
}
// all done
if (handler.originalFailure!=null) {
// wrap the exception so that the call stack leading up to parallel is visible
handler.context.onFailure(new ParallelStepException(handler.originalFailure.getKey(), handler.originalFailure.getValue()));
handler.context.onFailure(handler.originalFailure.getValue());
} else {
handler.context.onSuccess(success);
}
Expand Down
34 changes: 0 additions & 34 deletions src/main/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepException.java
View file

This file was deleted.

85 changes: 71 additions & 14 deletions src/test/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepTest.java
View file
Expand Up @@ -11,10 +11,9 @@
import org.jenkinsci.plugins.workflow.SingleJobTestBase;
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.cps.steps.ParallelStep;
import org.jenkinsci.plugins.workflow.cps.steps.ParallelStepException;
import org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.jenkinsci.plugins.workflow.steps.EchoStep;
import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
import org.jenkinsci.plugins.workflow.steps.durable_task.BatchScriptStep;
Expand Down Expand Up @@ -72,7 +71,6 @@ public void failure_in_subflow_will_cause_join_to_fail() throws Exception {
p = jenkins().createProject(WorkflowJob.class, "demo");
p.setDefinition(new CpsFlowDefinition(join(
"import "+AbortException.class.getName(),
"import "+ParallelStepException.class.getName(),

"node {",
" try {",
Expand All @@ -83,9 +81,8 @@ public void failure_in_subflow_will_cause_join_to_fail() throws Exception {
" a: { sleep 3; writeFile text: '', file: 'a.done' }",
" )",
" assert false;",
" } catch (ParallelStepException e) {",
" assert e.name=='b'",
" assert e.cause instanceof AbortException",
" } catch (AbortException e) {",
" assert e.message == 'died'",
" }",
"}"
)));
Expand All @@ -111,7 +108,6 @@ public void failure_in_subflow_will_fail_fast() throws Exception {
p = jenkins().createProject(WorkflowJob.class, "demo");
p.setDefinition(new CpsFlowDefinition(join(
"import "+AbortException.class.getName(),
"import "+ParallelStepException.class.getName(),

"node {",
" try {",
Expand All @@ -123,10 +119,8 @@ public void failure_in_subflow_will_fail_fast() throws Exception {
" failFast: true",
" )",
" assert false",
" } catch (ParallelStepException e) {",
" echo e.toString()",
" assert e.name=='b'",
" assert e.cause instanceof AbortException",
" } catch (AbortException e) {",
" assert e.message == 'died'",
" }",
"}"
)));
Expand All @@ -148,9 +142,6 @@ public void failFast_has_no_effect_on_success() throws Exception {
@Override public void evaluate() throws Throwable {
p = jenkins().createProject(WorkflowJob.class, "demo");
p.setDefinition(new CpsFlowDefinition(join(
"import "+AbortException.class.getName(),
"import "+ParallelStepException.class.getName(),

"node {",
" parallel(",
" a: { echo 'hello from a';sleep 1;echo 'goodbye from a' },",
Expand All @@ -169,6 +160,72 @@ public void failFast_has_no_effect_on_success() throws Exception {
});
}

@Issue("JENKINS-25894")
@Test public void failureReporting() throws Exception {
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("parallel a: {semaphore 'a'}, b: {semaphore 'b'}", true));
// Original bug report: AbortException not properly handled.
WorkflowRun b1 = p.scheduleBuild2(0).waitForStart();
SemaphoreStep.success("a/1", null);
SemaphoreStep.failure("b/1", new AbortException("normal failure"));
story.j.assertBuildStatus(Result.FAILURE, story.j.waitForCompletion(b1));
story.j.assertLogContains("Failed in branch b", b1);
// Apparently !b1.isBuilding() before WorkflowRun.finish has printed the stack trace, so need to wait for StreamBuildListener.finished:
story.j.waitForMessage("Finished: FAILURE", b1);
story.j.assertLogContains("normal failure", b1);
story.j.assertLogNotContains("AbortException", b1);
// Other exceptions should include a stack trace.
WorkflowRun b2 = p.scheduleBuild2(0).waitForStart();
SemaphoreStep.success("a/2", null);
SemaphoreStep.failure("b/2", new IllegalStateException("ouch"));
story.j.assertBuildStatus(Result.FAILURE, story.j.waitForCompletion(b2));
story.j.assertLogContains("Failed in branch b", b2);
story.j.waitForMessage("Finished: FAILURE", b2);
story.j.assertLogContains("java.lang.IllegalStateException: ouch", b2);
story.j.assertLogContains("\tat " + ParallelStepTest.class.getName(), b2);
// If multiple branches fail, we want to see all the stack traces.
WorkflowRun b3 = p.scheduleBuild2(0).waitForStart();
try {
failureInA();
fail();
} catch (IllegalStateException x) {
SemaphoreStep.failure("a/3", x);
}
try {
failureInB();
fail();
} catch (IllegalStateException x) {
SemaphoreStep.failure("b/3", x);
}
story.j.assertBuildStatus(Result.FAILURE, story.j.waitForCompletion(b3));
story.j.assertLogContains("Failed in branch a", b3);
story.j.assertLogContains("Failed in branch b", b3);
story.j.waitForMessage("Finished: FAILURE", b3);
story.j.assertLogContains("java.lang.IllegalStateException: first problem", b3);
story.j.assertLogContains("\tat " + ParallelStepTest.class.getName() + ".failureInA", b3);
story.j.assertLogContains("java.lang.IllegalStateException: second problem", b3);
story.j.assertLogContains("\tat " + ParallelStepTest.class.getName() + ".failureInB", b3);
// Also check stack traces within the script.
p.setDefinition(new CpsFlowDefinition(
"parallel bad: {\n" +
" throw new IllegalStateException('bad')\n" +
"}"));
WorkflowRun b4 = story.j.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
story.j.assertLogContains("Failed in branch bad", b4);
story.j.waitForMessage("Finished: FAILURE", b4);
story.j.assertLogContains("java.lang.IllegalStateException: bad", b4);
story.j.assertLogContains("\tat WorkflowScript.run(WorkflowScript:2)", b4);
}
});
}
private static void failureInA() {
throw new IllegalStateException("first problem");
}
private static void failureInB() {
throw new IllegalStateException("second problem");
}

@Test
public void localMethodCallWithinBranch() {
Expand Down

0 comments on commit 4fc3310

Please sign in to comment.