Improve artifact upload error handling

A customer reported on issue #40 that if OutOfMemoryError is thrown during artifact upload, the upload fails but the action execution is still marked as succeeded. This change fixes that issue.
awslabs · May 13, 2020 · 6eac484 · 6eac484
1 parent 21b2c24
commit 6eac484
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 4 deletions.
diff --git a/src/main/java/com/amazonaws/codepipeline/jenkinsplugin/AWSCodePipelinePublisher.java b/src/main/java/com/amazonaws/codepipeline/jenkinsplugin/AWSCodePipelinePublisher.java
@@ -170,6 +170,10 @@ public boolean perform(
             LoggingHelper.log(listener, ex.getMessage());
             LoggingHelper.log(listener, ex);
             awsStatus = false;
+        } catch (final Throwable ex) {
+            error = "Failed to upload output artifact(s): " + ex.getMessage();
+            awsStatus = false;
+            throw ex;
         } finally {
             PublisherTools.putJobResult(
                     awsStatus,

diff --git a/src/test/java/com/amazonaws/codepipeline/jenkinsplugin/AWSCodePipelinePublisherTest.java b/src/test/java/com/amazonaws/codepipeline/jenkinsplugin/AWSCodePipelinePublisherTest.java
@@ -19,24 +19,21 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.isA;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
 
 
-import hudson.remoting.VirtualChannel;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -455,6 +452,40 @@ public void putsJobFailedWhenArtifactUploadFails() {
         assertContainsIgnoreCase(PUT_JOB_FAILURE_MESSAGE, outContent.toString());
     }
 
+    @Test
+    public void putsJobFailedWhenUploadThrowsError() {
+        // given
+        when(mockBuild.getResult()).thenReturn(Result.SUCCESS);
+
+        when(mockJobData.getOutputArtifacts()).thenReturn(generateOutputArtifactsWithNames(Arrays.asList("artifact_1", "artifact_2")));
+
+        final AWSCodePipelinePublisher uploadFailurePublisher
+                = new AWSCodePipelinePublisherMockError(outputLocations, mockFactory);
+
+        // when
+        try {
+            assertFalse(uploadFailurePublisher.perform(mockBuild, null, null));
+            fail("Expected MockError not thrown");
+        } catch (final TestError e) {
+            // empty
+        }
+
+        // then
+        final InOrder inOrder = inOrder(mockFactory, mockAWS, mockCodePipelineClient);
+        inOrder.verify(mockFactory).getAwsClient(ACCESS_KEY, SECRET_KEY, PROXY_HOST, PROXY_PORT, REGION, PLUGIN_VERSION);
+        inOrder.verify(mockAWS).getCodePipelineClient();
+        inOrder.verify(mockCodePipelineClient).putJobFailureResult(putJobFailureResultRequest.capture());
+
+        final PutJobFailureResultRequest request = putJobFailureResultRequest.getValue();
+        assertEquals(jobId, request.getJobId());
+        assertEquals(BUILD_ID, request.getFailureDetails().getExternalExecutionId());
+        assertEquals("Failed to upload output artifact(s): Error", request.getFailureDetails().getMessage());
+        assertEquals(FailureType.JobFailed.toString(), request.getFailureDetails().getType());
+
+        assertContainsIgnoreCase(PUBLISHING_ARTIFACTS_MESSAGE, outContent.toString());
+        assertContainsIgnoreCase(PUT_JOB_FAILURE_MESSAGE, outContent.toString());
+    }
+
     @Test
     public void cleanUpSuccess() {
         // given
@@ -493,6 +524,23 @@ public void callPublish(final AbstractBuild<?,?> action, final CodePipelineState
         }
     }
 
+    private static class AWSCodePipelinePublisherMockError extends AWSCodePipelinePublisher {
+        public AWSCodePipelinePublisherMockError(final JSONArray outputLocations, final AWSClientFactory mockFactory) {
+            super(outputLocations, mockFactory);
+        }
+
+        @Override
+        public void callPublish(final AbstractBuild<?,?> action, final CodePipelineStateModel model, final BuildListener listener) {
+            throw new TestError();
+        }
+    }
+
+    private static class TestError extends Error {
+        public TestError() {
+            super("Error");
+        }
+    }
+
     private List<Artifact> generateOutputArtifactsWithNames(List<String> artifactNames) {
         final List<Artifact> outputArtifacts = new ArrayList<>();
         for (String artifactName : artifactNames) {