From a9f82f166ca82ac660178c4872955dcc4a59065c Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Wed, 3 Mar 2021 15:13:56 +0000 Subject: [PATCH 1/3] Handle failures in publish invocation --- .../plugins/checks/steps/WithChecksStep.java | 54 +++++++++++++------ 1 file changed, 39 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 af4f3160..6be1391a 100644 --- 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,13 @@ public String getDisplayName() { } } + private static class WithChecksPublishException extends Exception { + + public WithChecksPublishException(Throwable cause) { + super(cause); + } + } + /** * The step's execution to actually inject the {@link ChecksInfo} into the closure. */ @@ -108,15 +115,19 @@ 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 { + if (publish(getContext(), new ChecksDetails.ChecksDetailsBuilder() + .withName(step.getName()) + .withStatus(ChecksStatus.COMPLETED) + .withConclusion(ChecksConclusion.CANCELED))) { + getContext().onFailure(cause); + } + } catch (WithChecksPublishException e) { + getContext().onFailure(e); } } - private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) { + private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) throws WithChecksPublishException{ TaskListener listener = TaskListener.NULL; try { listener = fixNull(context.get(TaskListener.class), TaskListener.NULL); @@ -148,10 +159,14 @@ 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 (RuntimeException e) { + throw new WithChecksPublishException(e); + } } static class WithChecksCallBack extends BodyExecutionCallback { @@ -169,10 +184,14 @@ 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 +225,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); } } From 9ccc2ebf554dd3d049c8fe70e4309710591fa2b7 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Wed, 3 Mar 2021 15:19:37 +0000 Subject: [PATCH 2/3] Make static anlysis happy --- .../plugins/checks/steps/WithChecksStep.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 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 6be1391a..1c3a729a 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java @@ -79,7 +79,7 @@ public String getDisplayName() { private static class WithChecksPublishException extends Exception { - public WithChecksPublishException(Throwable cause) { + WithChecksPublishException(final Throwable cause) { super(cause); } } @@ -122,12 +122,14 @@ public void stop(final Throwable cause) { .withConclusion(ChecksConclusion.CANCELED))) { getContext().onFailure(cause); } - } catch (WithChecksPublishException e) { + } + catch (WithChecksPublishException e) { getContext().onFailure(e); } } - private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) throws WithChecksPublishException{ + @SuppressWarnings("IllegalCatch") + private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) throws WithChecksPublishException { TaskListener listener = TaskListener.NULL; try { listener = fixNull(context.get(TaskListener.class), TaskListener.NULL); @@ -164,7 +166,8 @@ private boolean publish(final StepContext context, final ChecksDetails.ChecksDet .publish(builder.withDetailsURL(DisplayURLProvider.get().getRunURL(run)) .build()); return true; - } catch (RuntimeException e) { + } + catch (RuntimeException e) { throw new WithChecksPublishException(e); } } @@ -189,7 +192,8 @@ public void onStart(final StepContext context) { .withName(info.getName()) .withStatus(ChecksStatus.IN_PROGRESS) .withConclusion(ChecksConclusion.NONE)); - } catch (WithChecksPublishException e) { + } + catch (WithChecksPublishException e) { context.onFailure(e); } } From 6cabcbda2f89896d99532b190c72339bdf71ee17 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Wed, 3 Mar 2021 15:28:31 +0000 Subject: [PATCH 3/3] Handle all errors via checked exception --- .../plugins/checks/steps/WithChecksStep.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 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 1c3a729a..0e0e9201 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java @@ -79,9 +79,19 @@ 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); + } } /** @@ -116,20 +126,19 @@ ChecksInfo extractChecksInfo() { @Override public void stop(final Throwable cause) { try { - if (publish(getContext(), new ChecksDetails.ChecksDetailsBuilder() + publish(getContext(), new ChecksDetails.ChecksDetailsBuilder() .withName(step.getName()) .withStatus(ChecksStatus.COMPLETED) - .withConclusion(ChecksConclusion.CANCELED))) { - getContext().onFailure(cause); - } + .withConclusion(ChecksConclusion.CANCELED)); } catch (WithChecksPublishException e) { - getContext().onFailure(e); + cause.addSuppressed(e); } + getContext().onFailure(cause); } @SuppressWarnings("IllegalCatch") - private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) throws WithChecksPublishException { + 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); @@ -146,26 +155,23 @@ 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); } try { ChecksPublisherFactory.fromRun(run, listener) .publish(builder.withDetailsURL(DisplayURLProvider.get().getRunURL(run)) .build()); - return true; } catch (RuntimeException e) { throw new WithChecksPublishException(e);