Skip to content

Commit 253e507

Browse files
zoewanggzoewangg
committed
Fixed an issue in the SDK that caused to be thrown instead of /when API call/API call attempt timeout is breached
1 parent fc3ca29 commit 253e507

File tree

7 files changed

+103
-10
lines changed

7 files changed

+103
-10
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"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."
6+
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis;
1919
import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.timeSyncTaskIfNeeded;
20-
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
20+
import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;
2121

2222
import java.time.Duration;
2323
import java.util.concurrent.ScheduledExecutorService;
@@ -35,12 +35,14 @@
3535
import software.amazon.awssdk.core.internal.http.timers.SyncTimeoutTask;
3636
import software.amazon.awssdk.core.internal.http.timers.TimeoutTracker;
3737
import software.amazon.awssdk.http.SdkHttpFullRequest;
38+
import software.amazon.awssdk.utils.Logger;
3839

3940
/**
4041
* Wrapper around a {@link RequestPipeline} to manage the api call attempt timeout feature.
4142
*/
4243
@SdkInternalApi
4344
public final class ApiCallAttemptTimeoutTrackingStage<OutputT> implements RequestToResponsePipeline<OutputT> {
45+
private static final Logger log = Logger.loggerFor(ApiCallAttemptTimeoutTrackingStage.class);
4446

4547
private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> wrapped;
4648
private final Duration apiCallAttemptTimeout;
@@ -129,7 +131,10 @@ private Exception translatePipelineException(RequestExecutionContext context, Ex
129131
*/
130132
private RuntimeException handleInterruptedException(RequestExecutionContext context, InterruptedException e) {
131133
if (e instanceof SdkInterruptedException) {
132-
((SdkInterruptedException) e).getResponseStream().ifPresent(r -> invokeSafely(r::close));
134+
((SdkInterruptedException) e).getResponseStream()
135+
.ifPresent(r -> runAndLogError(log.logger(),
136+
"Failed to close the response stream",
137+
r::close));
133138
}
134139
if (context.apiCallAttemptTimeoutTracker().hasExecuted()) {
135140
// Clear the interrupt status

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis;
1919
import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.timeSyncTaskIfNeeded;
20-
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
20+
import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;
2121

2222
import java.time.Duration;
2323
import java.util.concurrent.ScheduledExecutorService;
@@ -35,12 +35,14 @@
3535
import software.amazon.awssdk.core.internal.http.timers.SyncTimeoutTask;
3636
import software.amazon.awssdk.core.internal.http.timers.TimeoutTracker;
3737
import software.amazon.awssdk.http.SdkHttpFullRequest;
38+
import software.amazon.awssdk.utils.Logger;
3839

3940
/**
4041
* Wrapper around a {@link RequestPipeline} to manage the api call timeout feature.
4142
*/
4243
@SdkInternalApi
4344
public final class ApiCallTimeoutTrackingStage<OutputT> implements RequestToResponsePipeline<OutputT> {
45+
private static final Logger log = Logger.loggerFor(ApiCallTimeoutTrackingStage.class);
4446
private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> wrapped;
4547
private final SdkClientConfiguration clientConfig;
4648
private final ScheduledExecutorService timeoutExecutor;
@@ -131,7 +133,10 @@ private Exception translatePipelineException(RequestExecutionContext context, Ex
131133
*/
132134
private RuntimeException handleInterruptedException(RequestExecutionContext context, InterruptedException e) {
133135
if (e instanceof SdkInterruptedException) {
134-
((SdkInterruptedException) e).getResponseStream().ifPresent(r -> invokeSafely(r::close));
136+
((SdkInterruptedException) e).getResponseStream()
137+
.ifPresent(r -> runAndLogError(log.logger(),
138+
"Failed to close the response stream",
139+
r::close));
135140
}
136141
if (apiCallTimerExecuted(context)) {
137142
// Clear the interrupt status

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
package software.amazon.awssdk.core.internal.http.pipeline.stages;
1717

1818
import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis;
19-
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
19+
import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;
2020

2121
import java.io.IOException;
2222
import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -32,14 +32,15 @@
3232
import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
3333
import software.amazon.awssdk.core.internal.http.pipeline.RequestToResponsePipeline;
3434
import software.amazon.awssdk.http.SdkHttpFullRequest;
35+
import software.amazon.awssdk.utils.Logger;
3536

3637
/**
3738
* Check if an {@link Exception} is caused by either ApiCallTimeout or ApiAttemptTimeout and translate that
3839
* exception to a more appropriate timeout related exception so that it can be handled in other stages.
3940
*/
4041
@SdkInternalApi
4142
public final class TimeoutExceptionHandlingStage<OutputT> implements RequestToResponsePipeline<OutputT> {
42-
43+
private static final Logger log = Logger.loggerFor(TimeoutExceptionHandlingStage.class);
4344
private final HttpClientDependencies dependencies;
4445
private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> requestPipeline;
4546

@@ -100,7 +101,10 @@ private Exception translatePipelineException(RequestExecutionContext context, Ex
100101

101102
private Exception handleTimeoutCausedException(RequestExecutionContext context, Exception e) {
102103
if (e instanceof SdkInterruptedException) {
103-
((SdkInterruptedException) e).getResponseStream().ifPresent(r -> invokeSafely(r::close));
104+
((SdkInterruptedException) e).getResponseStream()
105+
.ifPresent(r -> runAndLogError(log.logger(),
106+
"Failed to close the response stream",
107+
r::close));
104108
}
105109

106110
if (isCausedByApiCallTimeout(context)) {

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,18 @@
1616
package software.amazon.awssdk.core.internal.http.pipeline.stages;
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1920
import static org.mockito.ArgumentMatchers.any;
2021
import static org.mockito.ArgumentMatchers.anyLong;
22+
import static org.mockito.Mockito.doNothing;
23+
import static org.mockito.Mockito.doThrow;
2124
import static org.mockito.Mockito.mock;
25+
import static org.mockito.Mockito.verify;
2226
import static org.mockito.Mockito.when;
2327
import static software.amazon.awssdk.core.client.config.SdkClientOption.SCHEDULED_EXECUTOR_SERVICE;
2428

29+
import java.io.IOException;
30+
import java.io.InputStream;
2531
import java.time.Duration;
2632
import java.util.concurrent.ScheduledExecutorService;
2733
import java.util.concurrent.ScheduledFuture;
@@ -35,14 +41,20 @@
3541
import software.amazon.awssdk.core.SdkRequest;
3642
import software.amazon.awssdk.core.SdkRequestOverrideConfiguration;
3743
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
44+
import software.amazon.awssdk.core.exception.AbortedException;
45+
import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException;
46+
import software.amazon.awssdk.core.exception.SdkInterruptedException;
3847
import software.amazon.awssdk.core.http.NoopTestRequest;
3948
import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
4049
import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
4150
import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
4251
import software.amazon.awssdk.core.internal.http.timers.ApiCallTimeoutTracker;
4352
import software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils;
4453
import software.amazon.awssdk.core.internal.http.timers.NoOpTimeoutTracker;
54+
import software.amazon.awssdk.http.AbortableInputStream;
4555
import software.amazon.awssdk.http.SdkHttpFullRequest;
56+
import software.amazon.awssdk.http.SdkHttpFullResponse;
57+
import utils.ValidSdkObjects;
4658

4759
@RunWith(MockitoJUnitRunner.class)
4860
public class ApiCallAttemptTimeoutTrackingStageTest {
@@ -56,6 +68,9 @@ public class ApiCallAttemptTimeoutTrackingStageTest {
5668
@Mock
5769
private ScheduledFuture scheduledFuture;
5870

71+
@Mock
72+
private InputStream responseStream;
73+
5974
private ApiCallAttemptTimeoutTrackingStage<Void> stage;
6075

6176
@Before
@@ -91,6 +106,18 @@ public void timeoutDisabled_shouldNotExecuteTimer() throws Exception {
91106
assertThat(context.apiCallAttemptTimeoutTracker().hasExecuted()).isFalse();
92107
}
93108

109+
@Test
110+
public void interruptedException_streamCloseFailed_shouldNotSurface() throws Exception {
111+
SdkHttpFullResponse response = SdkHttpFullResponse.builder().content(AbortableInputStream.create(responseStream,
112+
() -> {})).build();
113+
when(timeoutExecutor.schedule(any(Runnable.class), anyLong(), any(TimeUnit.class))).thenReturn(scheduledFuture);
114+
when(wrapped.execute(any(), any())).thenThrow(new SdkInterruptedException(response));
115+
RequestExecutionContext context = requestContext(1);
116+
doThrow(new IOException()).when(responseStream).close();
117+
assertThatThrownBy(() -> stage.execute(ValidSdkObjects.sdkHttpFullRequest().build(), context))
118+
.isInstanceOfAny(AbortedException.class);
119+
verify(responseStream).close();
120+
}
94121

95122
private RequestExecutionContext requestContext(long timeout) {
96123
SdkRequestOverrideConfiguration.Builder configBuilder = SdkRequestOverrideConfiguration.builder();

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,19 @@
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2020
import static org.assertj.core.api.Assertions.fail;
2121
import static org.mockito.ArgumentMatchers.any;
22+
import static org.mockito.ArgumentMatchers.anyLong;
23+
import static org.mockito.Mockito.doThrow;
2224
import static org.mockito.Mockito.mock;
25+
import static org.mockito.Mockito.verify;
2326
import static org.mockito.Mockito.when;
2427

28+
import java.io.IOException;
29+
import java.io.InputStream;
2530
import java.time.Duration;
2631
import java.util.concurrent.Executors;
2732
import java.util.concurrent.ScheduledExecutorService;
33+
import java.util.concurrent.ScheduledFuture;
34+
import java.util.concurrent.TimeUnit;
2835
import org.junit.Before;
2936
import org.junit.Test;
3037
import org.junit.runner.RunWith;
@@ -37,20 +44,27 @@
3744
import software.amazon.awssdk.core.client.config.SdkClientOption;
3845
import software.amazon.awssdk.core.exception.AbortedException;
3946
import software.amazon.awssdk.core.exception.ApiCallTimeoutException;
47+
import software.amazon.awssdk.core.exception.SdkInterruptedException;
4048
import software.amazon.awssdk.core.http.NoopTestRequest;
4149
import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
4250
import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
4351
import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
4452
import software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils;
53+
import software.amazon.awssdk.http.AbortableInputStream;
4554
import software.amazon.awssdk.http.SdkHttpFullRequest;
55+
import software.amazon.awssdk.http.SdkHttpFullResponse;
4656
import software.amazon.awssdk.utils.ThreadFactoryBuilder;
57+
import utils.ValidSdkObjects;
4758

4859
@RunWith(MockitoJUnitRunner.class)
4960
public class ApiCallTimeoutTrackingStageTest {
5061

5162
@Mock
5263
private RequestPipeline<SdkHttpFullRequest, Response<Void>> wrapped;
5364

65+
@Mock
66+
private InputStream responseStream;
67+
5468
private ApiCallTimeoutTrackingStage<Void> stage;
5569

5670
@Before
@@ -91,6 +105,19 @@ public void timeoutDisabled_shouldNotExecuteTimer() throws Exception {
91105
assertThat(context.apiCallTimeoutTracker().hasExecuted()).isFalse();
92106
}
93107

108+
@Test
109+
public void timeoutException_streamCloseFailed_shouldNotSurface() throws Exception {
110+
SdkHttpFullResponse response = SdkHttpFullResponse.builder().content(AbortableInputStream.create(responseStream,
111+
() -> {})).build();
112+
when(wrapped.execute(any(), any())).thenThrow(new SdkInterruptedException(response));
113+
RequestExecutionContext context = requestContext(300);
114+
doThrow(new IOException()).when(responseStream).close();
115+
assertThatThrownBy(() -> stage.execute(ValidSdkObjects.sdkHttpFullRequest().build(), context))
116+
.isInstanceOfAny(AbortedException.class);
117+
verify(responseStream).close();
118+
}
119+
120+
94121
@Test(expected = RuntimeException.class)
95122
public void nonTimerInterruption_RuntimeExceptionThrown_interruptFlagIsPreserved() throws Exception {
96123
nonTimerInterruption_interruptFlagIsPreserved(new RuntimeException());

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,15 @@
1818
import static org.assertj.core.api.Assertions.assertThat;
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2020
import static org.mockito.ArgumentMatchers.any;
21+
import static org.mockito.Mockito.doNothing;
22+
import static org.mockito.Mockito.doThrow;
23+
import static org.mockito.Mockito.verify;
2124
import static org.mockito.Mockito.when;
2225

2326
import java.io.IOException;
27+
import java.io.InputStream;
2428
import java.net.SocketException;
29+
import java.util.Optional;
2530
import java.util.concurrent.ScheduledFuture;
2631
import org.junit.Before;
2732
import org.junit.Test;
@@ -35,14 +40,17 @@
3540
import software.amazon.awssdk.core.exception.AbortedException;
3641
import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException;
3742
import software.amazon.awssdk.core.exception.SdkClientException;
43+
import software.amazon.awssdk.core.exception.SdkInterruptedException;
3844
import software.amazon.awssdk.core.http.NoopTestRequest;
3945
import software.amazon.awssdk.core.internal.http.HttpClientDependencies;
4046
import software.amazon.awssdk.core.internal.http.RequestExecutionContext;
4147
import software.amazon.awssdk.core.internal.http.pipeline.RequestPipeline;
4248
import software.amazon.awssdk.core.internal.http.timers.ApiCallTimeoutTracker;
4349
import software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils;
4450
import software.amazon.awssdk.core.internal.http.timers.TimeoutTask;
51+
import software.amazon.awssdk.http.AbortableInputStream;
4552
import software.amazon.awssdk.http.SdkHttpFullRequest;
53+
import software.amazon.awssdk.http.SdkHttpFullResponse;
4654
import utils.ValidSdkObjects;
4755

4856
@RunWith(MockitoJUnitRunner.class)
@@ -60,6 +68,8 @@ public class TimeoutExceptionHandlingStageTest {
6068
@Mock
6169
private ScheduledFuture scheduledFuture;
6270

71+
@Mock
72+
private InputStream responseStream;
6373

6474
private TimeoutExceptionHandlingStage<String> stage;
6575

@@ -117,6 +127,18 @@ public void AbortedException_causedByCallTimeout_shouldThrowInterruptedException
117127
verifyExceptionThrown(InterruptedException.class);
118128
}
119129

130+
@Test
131+
public void timeoutException_streamCloseFailed_shouldNotSurface() throws Exception {
132+
when(apiCallAttemptTimeoutTask.hasExecuted()).thenReturn(true);
133+
SdkHttpFullResponse response = SdkHttpFullResponse.builder().content(AbortableInputStream.create(responseStream,
134+
() -> {})).build();
135+
when(requestPipeline.execute(any(), any())).thenThrow(new SdkInterruptedException(response));
136+
137+
doThrow(new IOException()).when(responseStream).close();
138+
verifyExceptionThrown(ApiCallAttemptTimeoutException.class);
139+
verify(responseStream).close();
140+
}
141+
120142
@Test
121143
public void nonTimeoutCausedException_shouldPropagate() throws Exception {
122144
when(requestPipeline.execute(any(), any())).thenThrow(new RuntimeException());
@@ -188,9 +210,6 @@ public void abortedException_causedByApiCallAttemptTimeoutTask_shouldNotPropagat
188210
verifyExceptionThrown(ApiCallAttemptTimeoutException.class);
189211
}
190212

191-
192-
193-
194213
private void verifyInterruptStatusPreserved() {
195214
assertThat(Thread.currentThread().isInterrupted()).isTrue();
196215
}

0 commit comments

Comments
 (0)