From 69d14a5ad04937e1917aa93b55cdf1c3e9564eb8 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 13 Feb 2021 10:42:57 +0000 Subject: [PATCH 1/3] Handle failure in publish invocation --- .../plugins/checks/steps/WithChecksStep.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 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 index 7fb16435..a901b464 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java @@ -147,10 +147,16 @@ private boolean publish(final StepContext context, final ChecksDetails.ChecksDet return false; } - 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()); + return true; + } + catch (Exception e) { + context.onFailure(e); + return false; + } } class WithChecksCallBack extends BodyExecutionCallback { From 55790f59bdd79a17377bdd8c552c504aad1980f7 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 13 Feb 2021 17:39:12 +0000 Subject: [PATCH 2/3] Only call onFailure if we know we haven't already --- .../java/io/jenkins/plugins/checks/steps/WithChecksStep.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 index a901b464..ebe5d71c 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java @@ -209,8 +209,9 @@ public void onFailure(final StepContext context, final Throwable t) { .withTitle("Failed") .withText(t.toString()).build()); } - publish(context, builder); - context.onFailure(t); + if (publish(context, builder)) { + context.onFailure(t); + } } } } From f88c550d6fcb8f2c3b8d4424fee0505c5e943e32 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 13 Feb 2021 17:43:38 +0000 Subject: [PATCH 3/3] Alternatively, this --- .../plugins/checks/steps/WithChecksStep.java | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 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 index ebe5d71c..8e2f05ec 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java @@ -1,6 +1,7 @@ package io.jenkins.plugins.checks.steps; import edu.hm.hafner.util.VisibleForTesting; +import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import hudson.model.Run; @@ -108,15 +109,17 @@ ChecksInfo extractChecksInfo() { @Override public void stop(final Throwable cause) { - if (publish(getContext(), new ChecksDetails.ChecksDetailsBuilder() + Exception publishFailure = publish(getContext(), new ChecksDetails.ChecksDetailsBuilder() .withName(step.getName()) .withStatus(ChecksStatus.COMPLETED) - .withConclusion(ChecksConclusion.CANCELED))) { - getContext().onFailure(cause); + .withConclusion(ChecksConclusion.CANCELED)); + if (publishFailure != null) { + cause.addSuppressed(publishFailure); } + getContext().onFailure(cause); } - private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) { + private @CheckForNull Exception publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) { TaskListener listener = TaskListener.NULL; try { listener = fixNull(context.get(TaskListener.class), TaskListener.NULL); @@ -135,27 +138,24 @@ private boolean publish(final StepContext context, final ChecksDetails.ChecksDet String msg = "Failed getting Run from the context on the start of withChecks step: " + e; pluginLogger.log(msg); SYSTEM_LOGGER.log(Level.WARNING, msg); - context.onFailure(new IllegalStateException(msg)); - return false; + return new IllegalStateException(msg); } 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; + return new IllegalStateException(msg); } try { ChecksPublisherFactory.fromRun(run, listener) .publish(builder.withDetailsURL(DisplayURLProvider.get().getRunURL(run)) .build()); - return true; + return null; } - catch (Exception e) { - context.onFailure(e); - return false; + catch (RuntimeException e) { + return e; } } @@ -172,10 +172,13 @@ class WithChecksCallBack extends BodyExecutionCallback { @Override public void onStart(final StepContext context) { - publish(context, new ChecksDetails.ChecksDetailsBuilder() + Exception publishFailure = publish(context, new ChecksDetails.ChecksDetailsBuilder() .withName(info.getName()) .withStatus(ChecksStatus.IN_PROGRESS) .withConclusion(ChecksConclusion.NONE)); + if (publishFailure != null) { + context.onFailure(publishFailure); + } } @Override @@ -209,9 +212,11 @@ public void onFailure(final StepContext context, final Throwable t) { .withTitle("Failed") .withText(t.toString()).build()); } - if (publish(context, builder)) { - context.onFailure(t); + Exception publishFailure = publish(context, builder); + if (publishFailure != null) { + t.addSuppressed(publishFailure); } + context.onFailure(t); } } }