Fixed an issue in the SDK that caused UncheckedIOException to be thrown instead of /when …

.changes/next-release/bugfix-AWSSDKforJavav2-1e8d215.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Fixed an issue in the SDK that caused `UncheckedIOException` to be thrown instead of `ApiCallTimeoutException`/`ApiCallAttemptTimeoutException` when API call/API call attempt timeout is breached."
+}

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

@@ -17,7 +17,7 @@
 
 import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis;
 import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.timeSyncTaskIfNeeded;
-import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
+import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;
 
 import java.time.Duration;
 import java.util.concurrent.ScheduledExecutorService;
@@ -35,12 +35,14 @@
 import software.amazon.awssdk.core.internal.http.timers.SyncTimeoutTask;
 import software.amazon.awssdk.core.internal.http.timers.TimeoutTracker;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.utils.Logger;
 
 /**
  * Wrapper around a {@link RequestPipeline} to manage the api call attempt timeout feature.
  */
 @SdkInternalApi
 public final class ApiCallAttemptTimeoutTrackingStage<OutputT> implements RequestToResponsePipeline<OutputT> {
+    private static final Logger log = Logger.loggerFor(ApiCallAttemptTimeoutTrackingStage.class);
 
     private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> wrapped;
     private final Duration apiCallAttemptTimeout;
@@ -129,7 +131,10 @@ private Exception translatePipelineException(RequestExecutionContext context, Ex
      */
     private RuntimeException handleInterruptedException(RequestExecutionContext context, InterruptedException e) {
         if (e instanceof SdkInterruptedException) {
-            ((SdkInterruptedException) e).getResponseStream().ifPresent(r -> invokeSafely(r::close));
+            ((SdkInterruptedException) e).getResponseStream()
+                                         .ifPresent(r -> runAndLogError(log.logger(),
+                                                                        "Failed to close the response stream",
+                                                                        r::close));
         }
         if (context.apiCallAttemptTimeoutTracker().hasExecuted()) {
             // Clear the interrupt status

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

@@ -17,7 +17,7 @@
 
 import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis;
 import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.timeSyncTaskIfNeeded;
-import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
+import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;
 
 import java.time.Duration;
 import java.util.concurrent.ScheduledExecutorService;
@@ -35,12 +35,14 @@
 import software.amazon.awssdk.core.internal.http.timers.SyncTimeoutTask;
 import software.amazon.awssdk.core.internal.http.timers.TimeoutTracker;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.utils.Logger;
 
 /**
  * Wrapper around a {@link RequestPipeline} to manage the api call timeout feature.
  */
 @SdkInternalApi
 public final class ApiCallTimeoutTrackingStage<OutputT> implements RequestToResponsePipeline<OutputT> {
+    private static final Logger log = Logger.loggerFor(ApiCallTimeoutTrackingStage.class);
     private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> wrapped;
     private final SdkClientConfiguration clientConfig;
     private final ScheduledExecutorService timeoutExecutor;
@@ -131,7 +133,10 @@ private Exception translatePipelineException(RequestExecutionContext context, Ex
      */
     private RuntimeException handleInterruptedException(RequestExecutionContext context, InterruptedException e) {
         if (e instanceof SdkInterruptedException) {
-            ((SdkInterruptedException) e).getResponseStream().ifPresent(r -> invokeSafely(r::close));
+            ((SdkInterruptedException) e).getResponseStream()
+                                         .ifPresent(r -> runAndLogError(log.logger(),
+                                                                        "Failed to close the response stream",
+                                                                        r::close));
         }
         if (apiCallTimerExecuted(context)) {
             // Clear the interrupt status

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

@@ -16,7 +16,7 @@
 package software.amazon.awssdk.core.internal.http.pipeline.stages;
 
 import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis;
-import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
+import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;
 
 import java.io.IOException;
 import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -32,14 +32,15 @@
 import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
 import software.amazon.awssdk.core.internal.http.pipeline.RequestToResponsePipeline;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.utils.Logger;
 
 /**
  * Check if an {@link Exception} is caused by either ApiCallTimeout or ApiAttemptTimeout and translate that
  * exception to a more appropriate timeout related exception so that it can be handled in other stages.
  */
 @SdkInternalApi
 public final class TimeoutExceptionHandlingStage<OutputT> implements RequestToResponsePipeline<OutputT> {
-
+    private static final Logger log = Logger.loggerFor(TimeoutExceptionHandlingStage.class);
     private final HttpClientDependencies dependencies;
     private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> requestPipeline;
 
@@ -100,7 +101,10 @@ private Exception translatePipelineException(RequestExecutionContext context, Ex
 
     private Exception handleTimeoutCausedException(RequestExecutionContext context, Exception e) {
         if (e instanceof SdkInterruptedException) {
-            ((SdkInterruptedException) e).getResponseStream().ifPresent(r -> invokeSafely(r::close));
+            ((SdkInterruptedException) e).getResponseStream()
+                                         .ifPresent(r -> runAndLogError(log.logger(),
+                                                                        "Failed to close the response stream",
+                                                                        r::close));
         }
 
         if (isCausedByApiCallTimeout(context)) {

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

@@ -16,12 +16,18 @@
 package software.amazon.awssdk.core.internal.http.pipeline.stages;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static software.amazon.awssdk.core.client.config.SdkClientOption.SCHEDULED_EXECUTOR_SERVICE;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.time.Duration;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
@@ -35,14 +41,20 @@
 import software.amazon.awssdk.core.SdkRequest;
 import software.amazon.awssdk.core.SdkRequestOverrideConfiguration;
 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.SdkInterruptedException;
 import software.amazon.awssdk.core.http.NoopTestRequest;
 import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
 import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
 import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
 import software.amazon.awssdk.core.internal.http.timers.ApiCallTimeoutTracker;
 import software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils;
 import software.amazon.awssdk.core.internal.http.timers.NoOpTimeoutTracker;
+import software.amazon.awssdk.http.AbortableInputStream;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpFullResponse;
+import utils.ValidSdkObjects;
 
 @RunWith(MockitoJUnitRunner.class)
 public class ApiCallAttemptTimeoutTrackingStageTest {
@@ -56,6 +68,9 @@ public class ApiCallAttemptTimeoutTrackingStageTest {
     @Mock
     private ScheduledFuture scheduledFuture;
 
+    @Mock
+    private InputStream responseStream;
+
     private ApiCallAttemptTimeoutTrackingStage<Void> stage;
 
     @Before
@@ -91,6 +106,18 @@ public void timeoutDisabled_shouldNotExecuteTimer() throws Exception {
         assertThat(context.apiCallAttemptTimeoutTracker().hasExecuted()).isFalse();
     }
 
+    @Test
+    public void interruptedException_streamCloseFailed_shouldNotSurface() throws Exception {
+        SdkHttpFullResponse response = SdkHttpFullResponse.builder().content(AbortableInputStream.create(responseStream,
+                                                                                                         () -> {})).build();
+        when(timeoutExecutor.schedule(any(Runnable.class), anyLong(), any(TimeUnit.class))).thenReturn(scheduledFuture);
+        when(wrapped.execute(any(), any())).thenThrow(new SdkInterruptedException(response));
+        RequestExecutionContext context = requestContext(1);
+        doThrow(new IOException()).when(responseStream).close();
+        assertThatThrownBy(() -> stage.execute(ValidSdkObjects.sdkHttpFullRequest().build(), context))
+            .isInstanceOfAny(AbortedException.class);
+        verify(responseStream).close();
+    }
 
     private RequestExecutionContext requestContext(long timeout) {
         SdkRequestOverrideConfiguration.Builder configBuilder = SdkRequestOverrideConfiguration.builder();

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

@@ -19,12 +19,19 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.time.Duration;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -37,20 +44,27 @@
 import software.amazon.awssdk.core.client.config.SdkClientOption;
 import software.amazon.awssdk.core.exception.AbortedException;
 import software.amazon.awssdk.core.exception.ApiCallTimeoutException;
+import software.amazon.awssdk.core.exception.SdkInterruptedException;
 import software.amazon.awssdk.core.http.NoopTestRequest;
 import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
 import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
 import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
 import software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils;
+import software.amazon.awssdk.http.AbortableInputStream;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpFullResponse;
 import software.amazon.awssdk.utils.ThreadFactoryBuilder;
+import utils.ValidSdkObjects;
 
 @RunWith(MockitoJUnitRunner.class)
 public class ApiCallTimeoutTrackingStageTest {
 
     @Mock
     private RequestPipeline<SdkHttpFullRequest, Response<Void>> wrapped;
 
+    @Mock
+    private InputStream responseStream;
+
     private ApiCallTimeoutTrackingStage<Void> stage;
 
     @Before
@@ -91,6 +105,19 @@ public void timeoutDisabled_shouldNotExecuteTimer() throws Exception {
         assertThat(context.apiCallTimeoutTracker().hasExecuted()).isFalse();
     }
 
+    @Test
+    public void timeoutException_streamCloseFailed_shouldNotSurface() throws Exception {
+        SdkHttpFullResponse response = SdkHttpFullResponse.builder().content(AbortableInputStream.create(responseStream,
+                                                                                                         () -> {})).build();
+        when(wrapped.execute(any(), any())).thenThrow(new SdkInterruptedException(response));
+        RequestExecutionContext context = requestContext(300);
+        doThrow(new IOException()).when(responseStream).close();
+        assertThatThrownBy(() -> stage.execute(ValidSdkObjects.sdkHttpFullRequest().build(), context))
+            .isInstanceOfAny(AbortedException.class);
+        verify(responseStream).close();
+    }
+
+
     @Test(expected = RuntimeException.class)
     public void nonTimerInterruption_RuntimeExceptionThrown_interruptFlagIsPreserved() throws Exception {
         nonTimerInterruption_interruptFlagIsPreserved(new RuntimeException());

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

@@ -18,10 +18,15 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.SocketException;
+import java.util.Optional;
 import java.util.concurrent.ScheduledFuture;
 import org.junit.Before;
 import org.junit.Test;
@@ -35,14 +40,17 @@
 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.http.NoopTestRequest;
 import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
 import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
 import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
 import software.amazon.awssdk.core.internal.http.timers.ApiCallTimeoutTracker;
 import software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils;
 import software.amazon.awssdk.core.internal.http.timers.TimeoutTask;
+import software.amazon.awssdk.http.AbortableInputStream;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpFullResponse;
 import utils.ValidSdkObjects;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -60,6 +68,8 @@ public class TimeoutExceptionHandlingStageTest {
     @Mock
     private ScheduledFuture scheduledFuture;
 
+    @Mock
+    private InputStream responseStream;
 
     private TimeoutExceptionHandlingStage<String> stage;
 
@@ -117,6 +127,18 @@ public void AbortedException_causedByCallTimeout_shouldThrowInterruptedException
         verifyExceptionThrown(InterruptedException.class);
     }
 
+    @Test
+    public void timeoutException_streamCloseFailed_shouldNotSurface() throws Exception {
+        when(apiCallAttemptTimeoutTask.hasExecuted()).thenReturn(true);
+        SdkHttpFullResponse response = SdkHttpFullResponse.builder().content(AbortableInputStream.create(responseStream,
+                                                                                                         () -> {})).build();
+        when(requestPipeline.execute(any(), any())).thenThrow(new SdkInterruptedException(response));
+
+        doThrow(new IOException()).when(responseStream).close();
+        verifyExceptionThrown(ApiCallAttemptTimeoutException.class);
+        verify(responseStream).close();
+    }
+
     @Test
     public void nonTimeoutCausedException_shouldPropagate() throws Exception {
         when(requestPipeline.execute(any(), any())).thenThrow(new RuntimeException());
@@ -188,9 +210,6 @@ public void abortedException_causedByApiCallAttemptTimeoutTask_shouldNotPropagat
         verifyExceptionThrown(ApiCallAttemptTimeoutException.class);
     }
 
-
-
-
     private void verifyInterruptStatusPreserved() {
         assertThat(Thread.currentThread().isInterrupted()).isTrue();
     }