diff --git a/pom.xml b/pom.xml
index 3c420334..da483158 100644
--- a/pom.xml
+++ b/pom.xml
@@ -73,7 +73,7 @@
io.jenkins.tools.bom
bom-2.426.x
- 2555.v3190a_8a_c60c6
+ 2745.vc7b_fe4c876fa_
import
pom
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java
index a6853bff..f8d58295 100644
--- a/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java
+++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java
@@ -37,6 +37,8 @@
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import jenkins.model.Jenkins;
import org.apache.commons.io.output.NullOutputStream;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
@@ -53,23 +55,33 @@
*/
public class ErrorAction implements PersistentAction {
+ private static final Logger LOGGER = Logger.getLogger(ErrorAction.class.getName());
+
private final @NonNull Throwable error;
public ErrorAction(@NonNull Throwable error) {
+ Throwable errorForAction = error;
if (isUnserializableException(error, new HashSet<>())) {
- error = new ProxyException(error);
+ LOGGER.log(Level.FINE, "sanitizing unserializable error", error);
+ errorForAction = new ProxyException(error);
} else if (error != null) {
try {
Jenkins.XSTREAM2.toXMLUTF8(error, new NullOutputStream());
} catch (Exception x) {
+ LOGGER.log(Level.FINE, "unable to serialize to XML", x);
// Typically SecurityException from ClassFilter.
- error = new ProxyException(error);
+ errorForAction = new ProxyException(error);
}
}
- this.error = error;
+ this.error = errorForAction;
String id = findId(error, new HashSet<>());
- if (id == null && error != null) {
- error.addSuppressed(new ErrorId());
+ if (id == null) {
+ var errorId = new ErrorId();
+ errorForAction.addSuppressed(errorId);
+ if (error != errorForAction) {
+ // Make sure the original exception has the error ID, not just the copy here.
+ error.addSuppressed(errorId);
+ }
}
}
@@ -170,28 +182,49 @@ public String getUrlName() {
@Restricted(Beta.class)
public static boolean equals(Throwable t1, Throwable t2) {
if (t1 == t2) {
+ LOGGER.fine(() -> "Same object: " + t1);
return true;
- } else if (t1.getClass() != t2.getClass()) {
+ }
+ boolean noProxy = t1.getClass() != ProxyException.class && t2.getClass() != ProxyException.class;
+ if (noProxy && t1.getClass() != t2.getClass()) {
+ LOGGER.fine(() -> "Different types: " + t1.getClass() + " vs. " + t2.getClass());
return false;
- } else if (!Objects.equals(t1.getMessage(), t2.getMessage())) {
+ } else if (noProxy && !Objects.equals(t1.getMessage(), t2.getMessage())) {
+ LOGGER.fine(() -> "Different messages: " + t1.getMessage() + " vs. " + t2.getMessage());
return false;
} else {
String id1 = findId(t1, new HashSet<>());
if (id1 != null) {
- return id1.equals(findId(t2, new HashSet<>()));
+ String id2 = findId(t2, new HashSet<>());
+ if (id1.equals(id2)) {
+ LOGGER.fine(() -> "ErrorId matches: " + id1);
+ return true;
+ } else {
+ LOGGER.fine(() -> "ErrorId mismatch: " + t1 + " " + id1 + " vs. " + t2 + " " + id2);
+ return false;
+ }
}
// No ErrorId, use a best-effort approach that doesn't work across restarts for exceptions thrown
// synchronously from the CPS VM thread.
// Check that stack traces match, but specifically avoid checking suppressed exceptions, which are often
// modified after ErrorAction is written to disk when steps like parallel are involved.
- while (t1 != null && t2 != null) {
- if (!Arrays.equals(t1.getStackTrace(), t2.getStackTrace())) {
+ var _t1 = t1;
+ var _t2 = t2;
+ while (_t1 != null && _t2 != null) {
+ if (!Arrays.equals(_t1.getStackTrace(), _t2.getStackTrace())) {
+ LOGGER.fine(() -> "Different stack traces between " + t1 + " vs. " + t2); // not showing details
return false;
}
- t1 = t1.getCause();
- t2 = t2.getCause();
+ _t1 = _t1.getCause();
+ _t2 = _t2.getCause();
+ }
+ if ((_t1 == null) == (_t2 == null)) {
+ LOGGER.fine(() -> "Same stack traces in " + t1 + " vs. " + t2);
+ return true;
+ } else {
+ LOGGER.fine(() -> "Different cause depths between " + t1 + " vs. " + t2);
+ return false;
}
- return (t1 == null) == (t2 == null);
}
}
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java b/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
index 6165330e..6ec2ac99 100644
--- a/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
+++ b/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
@@ -47,21 +47,44 @@
import org.jvnet.hudson.test.JenkinsSessionRule;
import groovy.lang.MissingMethodException;
+import hudson.FilePath;
import hudson.Functions;
import hudson.model.Result;
+import hudson.model.TaskListener;
import hudson.remoting.ProxyException;
+import hudson.remoting.VirtualChannel;
+import java.io.File;
+import java.io.IOException;
+import java.util.Set;
+import java.util.logging.Level;
+import jenkins.MasterToSlaveFileCallable;
import org.codehaus.groovy.runtime.NullObject;
+import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
+import org.jenkinsci.plugins.workflow.steps.Step;
+import org.jenkinsci.plugins.workflow.steps.StepContext;
+import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
+import org.jenkinsci.plugins.workflow.steps.StepExecution;
+import org.jenkinsci.plugins.workflow.steps.StepExecutions;
+import org.jvnet.hudson.test.InboundAgentRule;
+import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.DataBoundConstructor;
/**
* Tests for {@link ErrorAction}
*/
public class ErrorActionTest {
+
@ClassRule
public static BuildWatcher buildWatcher = new BuildWatcher();
@Rule
public JenkinsSessionRule rr = new JenkinsSessionRule();
+ @Rule public InboundAgentRule agents = new InboundAgentRule();
+
+ @Rule public LoggerRule logging = new LoggerRule().record(ErrorAction.class, Level.FINE);
+
private List extractErrorActions(FlowExecution exec) {
List ret = new ArrayList<>();
@@ -228,6 +251,87 @@ public static class X extends Exception {
});
}
+ @Test public void findOriginFromBodyExecutionCallback() throws Throwable {
+ rr.then(r -> {
+ agents.createAgent(r, "remote");
+ var p = r.createProject(WorkflowJob.class);
+ p.setDefinition(new CpsFlowDefinition("callsFindOrigin {node('remote') {fails()}}", true));
+ var b = p.scheduleBuild2(0).waitForStart();
+ r.waitForMessage("Acting slowly in ", b);
+ agents.stop("remote");
+ r.assertBuildStatus(Result.FAILURE, r.waitForCompletion(b));
+ r.assertLogContains("Found in: fails", b);
+ });
+ }
+ public static final class WrapperStep extends Step {
+ @DataBoundConstructor public WrapperStep() {}
+ @Override public StepExecution start(StepContext context) throws Exception {
+ return new ExecutionImpl(context);
+ }
+ private static final class ExecutionImpl extends StepExecution {
+ ExecutionImpl(StepContext context) {
+ super(context);
+ }
+ @Override public boolean start() throws Exception {
+ getContext().newBodyInvoker().withCallback(new Callback()).start();
+ return false;
+ }
+ }
+ private static class Callback extends BodyExecutionCallback {
+ @Override public void onSuccess(StepContext context, Object result) {
+ context.onSuccess(result);
+ }
+ @Override
+ public void onFailure(StepContext context, Throwable t) {
+ try {
+ var l = context.get(TaskListener.class);
+ Functions.printStackTrace(t, l.error("Original failure:"));
+ l.getLogger().println("Found in: " + ErrorAction.findOrigin(t, context.get(FlowExecution.class)).getDisplayFunctionName());
+ } catch (Exception x) {
+ assert false : x;
+ }
+ context.onFailure(t);
+ }
+ }
+ @TestExtension("findOriginFromBodyExecutionCallback") public static final class DescriptorImpl extends StepDescriptor {
+ @Override public String getFunctionName() {
+ return "callsFindOrigin";
+ }
+ @Override public Set extends Class>> getRequiredContext() {
+ return Set.of();
+ }
+ @Override public boolean takesImplicitBlockArgument() {
+ return true;
+ }
+ }
+ }
+ public static final class FailingStep extends Step {
+ @DataBoundConstructor public FailingStep() {}
+ @Override public StepExecution start(StepContext context) throws Exception {
+ return StepExecutions.synchronousNonBlockingVoid(context, c -> c.get(FilePath.class).act(new Sleep(c.get(TaskListener.class))));
+ }
+ private static final class Sleep extends MasterToSlaveFileCallable {
+ private final TaskListener l;
+ Sleep(TaskListener l) {
+ this.l = l;
+ }
+ @Override public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
+ l.getLogger().println("Acting slowly in " + f);
+ l.getLogger().flush();
+ Thread.sleep(Long.MAX_VALUE);
+ return null;
+ }
+ }
+ @TestExtension("findOriginFromBodyExecutionCallback") public static final class DescriptorImpl extends StepDescriptor {
+ @Override public String getFunctionName() {
+ return "fails";
+ }
+ @Override public Set extends Class>> getRequiredContext() {
+ return Set.of(FilePath.class);
+ }
+ }
+ }
+
@Test public void cyclicErrorsAreSupported() throws Throwable {
Exception cyclic1 = new Exception();
Exception cyclic2 = new Exception(cyclic1);