Fix for Issue #1684 Changes in handleTimeoutCausedException in TimeoutExceptionHandlingStage

Author: John Viegas

.changes/next-release/bugfix-AWSSDKforJavav2-b5a2807.json

@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "", 
+    "type": "bugfix", 
+    "description": "Fix for [#1684](https://github.com/aws/aws-sdk-java-v2/issues/1684) Some of the Retry attempts which failed due to the API TimeOuts did not successfully retried but ended up with AbortedException."
+}

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/TimeoutExceptionHandlingStage.java

@@ -25,6 +25,7 @@
 import software.amazon.awssdk.core.client.config.SdkClientOption;
 import software.amazon.awssdk.core.exception.AbortedException;
 import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException;
+import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.exception.SdkInterruptedException;
 import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
 import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
@@ -90,7 +91,8 @@ public Response<OutputT> execute(SdkHttpFullRequest request, RequestExecutionCon
      */
     private Exception translatePipelineException(RequestExecutionContext context, Exception e) {
         if (e instanceof InterruptedException || e instanceof IOException ||
-            e instanceof AbortedException || Thread.currentThread().isInterrupted()) {
+                e instanceof AbortedException || Thread.currentThread().isInterrupted()
+                || (e instanceof SdkClientException && isCausedByApiCallAttemptTimeout(context))) {
             return handleTimeoutCausedException(context, e);
         }
         return e;

core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/TimeoutExceptionHandlingStageTest.java

@@ -34,6 +34,7 @@
 import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
 import software.amazon.awssdk.core.exception.AbortedException;
 import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException;
+import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.http.NoopTestRequest;
 import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
 import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
@@ -164,6 +165,33 @@ public void interruptFlagWasSet_causedByApiCallTimeout_shouldThrowInterruptExcep
         verifyInterruptStatusPreserved();
     }
 
+    @Test
+    public void apiCallAttemptTimeoutException_causedBySdkClientException_as_apiCallAttemptTimeoutTask_Caused_SdkClientException() throws Exception {
+        when(apiCallTimeoutTask.hasExecuted()).thenReturn(false);
+        when(apiCallAttemptTimeoutTask.hasExecuted()).thenReturn(true);
+        when(requestPipeline.execute(any(), any())).thenThrow(SdkClientException.create(""));
+        verifyExceptionThrown(ApiCallAttemptTimeoutException.class);
+    }
+
+    @Test
+    public void interruptedException_causedByApiCallAttemptTimeoutTask() throws Exception {
+        when(apiCallTimeoutTask.hasExecuted()).thenReturn(true);
+        when(apiCallAttemptTimeoutTask.hasExecuted()).thenReturn(true);
+        when(requestPipeline.execute(any(), any())).thenThrow(SdkClientException.class);
+        verifyExceptionThrown(InterruptedException.class);
+    }
+
+
+    @Test
+    public void abortedException_causedByApiCallAttemptTimeoutTask_shouldNotPropagate() throws Exception {
+        when(apiCallTimeoutTask.hasExecuted()).thenReturn(false);
+        when(apiCallAttemptTimeoutTask.hasExecuted()).thenReturn(true);
+        when(requestPipeline.execute(any(), any())).thenThrow(AbortedException.class);
+        verifyExceptionThrown(ApiCallAttemptTimeoutException.class);
+    }
+
+
+
 
     private void verifyInterruptStatusPreserved() {
         assertThat(Thread.currentThread().isInterrupted()).isTrue();

services/dynamodb/src/it/java/software/amazon/awssdk/services/dynamodb/DynamoDbJavaClientRetryOnTimeoutIntegrationTest.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.services.dynamodb;
+
+import org.junit.Test;
+import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
+import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException;
+import software.amazon.awssdk.core.retry.RetryPolicy;
+import software.amazon.awssdk.core.retry.conditions.OrRetryCondition;
+import software.amazon.awssdk.core.retry.conditions.RetryCondition;
+import software.amazon.awssdk.testutils.service.AwsTestBase;
+
+import java.time.Duration;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+
+/**
+ * Simple test to check all the retries are made when all the API calls timeouts.
+ */
+public class DynamoDbJavaClientRetryOnTimeoutIntegrationTest extends AwsTestBase {
+
+    private static DynamoDbClient ddb;
+    private final Integer RETRY_ATTEMPTS = 3;
+
+    public static void setupWithRetryPolicy(RetryPolicy retryPolicy, Integer attemptTimeOutMillis) throws Exception {
+        ddb = DynamoDbClient.builder()
+                .overrideConfiguration(ClientOverrideConfiguration.builder()
+                        .retryPolicy(retryPolicy)
+                        .apiCallAttemptTimeout(Duration.ofMillis(attemptTimeOutMillis)) //forces really quick api call timeout
+                        .build())
+                .build();
+
+    }
+
+    public static RetryPolicy createRetryPolicyWithCounter(AtomicInteger counter, Integer numOfAttempts) {
+        final RetryPolicy retryPolicy = RetryPolicy.defaultRetryPolicy().toBuilder()
+                .numRetries(numOfAttempts)
+                .retryCondition(OrRetryCondition.create(
+                        context -> {
+                            counter.incrementAndGet();
+                            return false;
+                        }, RetryCondition.defaultRetryCondition())).build();
+
+        return retryPolicy;
+
+    }
+
+    @Test
+    public void testRetryAttemptsOnTimeOut() throws Exception {
+        AtomicInteger atomicInteger = new AtomicInteger(0);
+        Boolean apiTimeOutExceptionOccurred = Boolean.FALSE;
+        setupWithRetryPolicy(createRetryPolicyWithCounter(atomicInteger, RETRY_ATTEMPTS), 2);
+        try {
+
+            ddb.listTables();
+        } catch (ApiCallAttemptTimeoutException e) {
+            apiTimeOutExceptionOccurred = true;
+        }
+        assertEquals(RETRY_ATTEMPTS.intValue(), atomicInteger.get());
+        assertTrue(apiTimeOutExceptionOccurred);
+    }
+
+}