Merge pull request #535 from jglick/CpsStepContext.completed

Never print warnings from `CpsStepContext.completed`, but use `addSuppressed`
jenkinsci · Jun 3, 2022 · cd48bb9 · cd48bb9
2 parents 71dd22b + de95488
commit cd48bb9
Showing 1 changed file with 16 additions and 22 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsStepContext.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsStepContext.java
@@ -30,7 +30,6 @@
 import com.google.common.util.concurrent.SettableFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import groovy.lang.Closure;
-import hudson.Main;
 import hudson.model.Descriptor;
 import hudson.model.Result;
 import hudson.util.DaemonThreadFactory;
@@ -66,6 +65,7 @@
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Stream;
 import jenkins.model.CauseOfInterruption;
 import jenkins.util.ContextResettingExecutorService;
 import org.codehaus.groovy.runtime.InvokerInvocationException;
@@ -328,39 +328,33 @@ private void completed(@NonNull Outcome newOutcome) {
             whenOutcomeDelivered = new Throwable();
         } else {
             Throwable failure = newOutcome.getAbnormal();
-            Level level;
-            if (Main.isUnitTest) {
-                if (failure instanceof FlowInterruptedException && ((FlowInterruptedException) failure).getCauses().stream().anyMatch(BodyFailed.class::isInstance)) {
-                    // Very common and generally uninteresting.
-                    level = Level.FINE;
-                } else {
-                    // Possibly a minor bug.
-                    level = Level.INFO;
-                }
-            } else {
-                // Typically harmless; do not alarm users.
-                level = Level.FINE;
-            }
-            if (LOGGER.isLoggable(level)) {
-                LOGGER.log(level, "already completed " + this, new IllegalStateException("delivered here"));
+            Throwable earlierFailure = outcome.getAbnormal();
+            if (LOGGER.isLoggable(Level.FINE)) {
+                LOGGER.log(Level.FINE, "already completed " + this, new IllegalStateException("delivered here"));
                 if (failure != null) {
-                    LOGGER.log(level, "new failure", failure);
+                    LOGGER.log(Level.FINE, "new failure", failure);
                 } else {
-                    LOGGER.log(level, "new success: {0}", outcome.getNormal());
+                    LOGGER.log(Level.FINE, "new success: {0}", outcome.getNormal());
                 }
                 if (whenOutcomeDelivered != null) {
-                    LOGGER.log(level, "previously delivered here", whenOutcomeDelivered);
+                    LOGGER.log(Level.FINE, "previously delivered here", whenOutcomeDelivered);
                 }
-                Throwable earlierFailure = outcome.getAbnormal();
                 if (earlierFailure != null) {
-                    LOGGER.log(level, "earlier failure", earlierFailure);
+                    LOGGER.log(Level.FINE, "earlier failure", earlierFailure);
                 } else {
-                    LOGGER.log(level, "earlier success: {0}", outcome.getNormal());
+                    LOGGER.log(Level.FINE, "earlier success: {0}", outcome.getNormal());
                 }
             }
+            if (failure != null && earlierFailure != null && !refersTo(failure, earlierFailure)) {
+                earlierFailure.addSuppressed(failure);
+            }
         }
     }
 
+    private static boolean refersTo(Throwable t1, Throwable t2) {
+        return t1 == t2 || t1.getCause() != null && refersTo(t1.getCause(), t2) || Stream.of(t1.getSuppressed()).anyMatch(t3 -> refersTo(t3, t2));
+    }
+
     /**
      * When this step context has completed execution (successful or otherwise), plan the next action.
      */