Handle failures in publish invocation (#91)

* Handle failures in publish invocation * Make static anlysis happy * Handle all errors via checked exception
jenkinsci · Mar 3, 2021 · 26b910f · 26b910f
1 parent 4996cf4
commit 26b910f
Showing 1 changed file with 56 additions and 22 deletions.
diff --git a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java
@@ -77,6 +77,23 @@ public String getDisplayName() {
         }
     }
 
+    private static class WithChecksPublishException extends Exception {
+
+        public static final long serialVersionUID = 1L;
+
+        WithChecksPublishException(final Throwable cause) {
+            super(cause);
+        }
+
+        WithChecksPublishException(final String msg) {
+            super(msg);
+        }
+
+        WithChecksPublishException(final String msg, final Throwable e) {
+            super(msg, e);
+        }
+    }
+
     /**
      * The step's execution to actually inject the {@link ChecksInfo} into the closure.
      */
@@ -108,15 +125,20 @@ ChecksInfo extractChecksInfo() {
 
         @Override
         public void stop(final Throwable cause) {
-            if (publish(getContext(), new ChecksDetails.ChecksDetailsBuilder()
-                    .withName(step.getName())
-                    .withStatus(ChecksStatus.COMPLETED)
-                    .withConclusion(ChecksConclusion.CANCELED))) {
-                getContext().onFailure(cause);
+            try {
+                publish(getContext(), new ChecksDetails.ChecksDetailsBuilder()
+                        .withName(step.getName())
+                        .withStatus(ChecksStatus.COMPLETED)
+                        .withConclusion(ChecksConclusion.CANCELED));
             }
+            catch (WithChecksPublishException e) {
+                cause.addSuppressed(e);
+            }
+            getContext().onFailure(cause);
         }
 
-        private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) {
+        @SuppressWarnings("IllegalCatch")
+        private void publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) throws WithChecksPublishException {
             TaskListener listener = TaskListener.NULL;
             try {
                 listener = fixNull(context.get(TaskListener.class), TaskListener.NULL);
@@ -133,25 +155,27 @@ private boolean publish(final StepContext context, final ChecksDetails.ChecksDet
                 run = context.get(Run.class);
             }
             catch (IOException | InterruptedException e) {
-                String msg = "Failed getting Run from the context on the start of withChecks step: " + e;
-                pluginLogger.log(msg.replaceAll("\r\n", ""));
-                SYSTEM_LOGGER.log(Level.WARNING, msg.replaceAll("\r\n", ""));
-                context.onFailure(new IllegalStateException(msg));
-                return false;
+                String msg = "Failed getting Run from the context on the start of withChecks step";
+                pluginLogger.log((msg + ": " + e).replaceAll("\r\n", ""));
+                SYSTEM_LOGGER.log(Level.WARNING, msg, e);
+                throw new WithChecksPublishException(msg, e);
             }
 
             if (run == null) {
                 String msg = "No Run found in the context.";
                 pluginLogger.log(msg);
                 SYSTEM_LOGGER.log(Level.WARNING, msg);
-                context.onFailure(new IllegalStateException(msg));
-                return false;
+                throw new WithChecksPublishException(msg);
             }
 
-            ChecksPublisherFactory.fromRun(run, listener)
-                    .publish(builder.withDetailsURL(DisplayURLProvider.get().getRunURL(run))
-                            .build());
-            return true;
+            try {
+                ChecksPublisherFactory.fromRun(run, listener)
+                        .publish(builder.withDetailsURL(DisplayURLProvider.get().getRunURL(run))
+                                .build());
+            }
+            catch (RuntimeException e) {
+                throw new WithChecksPublishException(e);
+            }
         }
 
         static class WithChecksCallBack extends BodyExecutionCallback {
@@ -169,10 +193,15 @@ static class WithChecksCallBack extends BodyExecutionCallback {
 
             @Override
             public void onStart(final StepContext context) {
-                execution.publish(context, new ChecksDetails.ChecksDetailsBuilder()
-                        .withName(info.getName())
-                        .withStatus(ChecksStatus.IN_PROGRESS)
-                        .withConclusion(ChecksConclusion.NONE));
+                try {
+                    execution.publish(context, new ChecksDetails.ChecksDetailsBuilder()
+                            .withName(info.getName())
+                            .withStatus(ChecksStatus.IN_PROGRESS)
+                            .withConclusion(ChecksConclusion.NONE));
+                }
+                catch (WithChecksPublishException e) {
+                    context.onFailure(e);
+                }
             }
 
             @Override
@@ -206,7 +235,12 @@ public void onFailure(final StepContext context, final Throwable t) {
                                     .withTitle("Failed")
                                     .withText(t.toString()).build());
                 }
-                execution.publish(context, builder);
+                try {
+                    execution.publish(context, builder);
+                }
+                catch (WithChecksPublishException e) {
+                    t.addSuppressed(e);
+                }
                 context.onFailure(t);
             }
         }