Skip to content

Commit 4ab4f25

Browse files
zoewanggzoewangg
committed
Add apiCallTimeout and apiCallAttemptTimeout for synchronous operations
1 parent 3867927 commit 4ab4f25

File tree

48 files changed

+2632
-756
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+2632
-756
lines changed

‎.changes/next-release/feature-AWSSDKforJavav2-fade6e0.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"type": "feature",
4+
"description": "Add apiCallTimeout and apiCallAttemptTimeout feature for synchronous calls."
5+
}

‎core/sdk-core/src/it/java/software/amazon/awssdk/core/http/timers/client/AbortedExceptionClientExecutionTimerIntegrationTest.java

Lines changed: 6 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -15,51 +15,27 @@
1515

1616
package software.amazon.awssdk.core.http.timers.client;
1717

18-
import static org.hamcrest.Matchers.instanceOf;
19-
import static org.junit.Assert.assertThat;
20-
import static org.junit.Assert.fail;
2118
import static org.mockito.Matchers.any;
22-
import static org.mockito.Mockito.mock;
23-
import static org.mockito.Mockito.verify;
2419
import static org.mockito.Mockito.when;
2520
import static software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils.createMockGetRequest;
2621
import static software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils.execute;
27-
import static software.amazon.awssdk.core.internal.http.timers.ClientExecutionAndRequestTimerTestUtils.interruptCurrentThreadAfterDelay;
2822

29-
import java.io.InputStream;
30-
import java.util.Arrays;
23+
import java.time.Duration;
3124
import org.junit.Before;
32-
import org.junit.Ignore;
3325
import org.junit.Test;
3426
import org.junit.runner.RunWith;
3527
import org.mockito.Mock;
3628
import org.mockito.runners.MockitoJUnitRunner;
37-
import software.amazon.awssdk.annotations.ReviewBeforeRelease;
3829
import software.amazon.awssdk.core.exception.AbortedException;
3930
import software.amazon.awssdk.core.exception.ApiCallTimeoutException;
40-
import software.amazon.awssdk.core.exception.SdkClientException;
41-
import software.amazon.awssdk.core.http.ExecutionContext;
42-
import software.amazon.awssdk.core.http.MockServerTestBase;
43-
import software.amazon.awssdk.core.http.NoopTestRequest;
44-
import software.amazon.awssdk.core.http.server.MockServer;
45-
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
46-
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
4731
import software.amazon.awssdk.core.internal.http.AmazonSyncHttpClient;
48-
import software.amazon.awssdk.core.internal.http.request.SlowExecutionInterceptor;
49-
import software.amazon.awssdk.core.internal.http.response.DummyResponseHandler;
50-
import software.amazon.awssdk.core.interceptor.ExecutionInterceptorChain;
51-
import software.amazon.awssdk.core.interceptor.InterceptorContext;
52-
import software.amazon.awssdk.core.signer.NoOpSigner;
5332
import software.amazon.awssdk.http.AbortableCallable;
54-
import software.amazon.awssdk.http.AbortableInputStream;
5533
import software.amazon.awssdk.http.SdkHttpClient;
5634
import software.amazon.awssdk.http.SdkHttpFullResponse;
5735
import utils.HttpTestUtils;
5836

5937
@RunWith(MockitoJUnitRunner.class)
60-
@Ignore
61-
@ReviewBeforeRelease("add it back once execution time out is added back")
62-
public class AbortedExceptionClientExecutionTimerIntegrationTest extends MockServerTestBase {
38+
public class AbortedExceptionClientExecutionTimerIntegrationTest {
6339

6440
private AmazonSyncHttpClient httpClient;
6541

@@ -72,17 +48,14 @@ public class AbortedExceptionClientExecutionTimerIntegrationTest extends MockSer
7248
@Before
7349
public void setup() throws Exception {
7450
when(sdkHttpClient.prepareRequest(any())).thenReturn(abortableCallable);
75-
httpClient = HttpTestUtils.testClientBuilder().httpClient(sdkHttpClient).build();
51+
httpClient = HttpTestUtils.testClientBuilder().httpClient(sdkHttpClient)
52+
.apiCallTimeout(Duration.ofMillis(1000))
53+
.build();
7654
when(abortableCallable.call()).thenReturn(SdkHttpFullResponse.builder()
7755
.statusCode(200)
7856
.build());
7957
}
8058

81-
@Override
82-
protected MockServer buildMockServer() {
83-
return new MockServer(MockServer.DummyResponseServerBehavior.build(200, "Hi", "Dummy response"));
84-
}
85-
8659
@Test(expected = AbortedException.class)
8760
public void clientExecutionTimeoutEnabled_aborted_exception_occurs_timeout_not_expired() throws Exception {
8861
when(abortableCallable.call()).thenThrow(AbortedException.builder().build());
@@ -100,44 +73,4 @@ public void clientExecutionTimeoutEnabled_aborted_exception_occurs_timeout_expir
10073

10174
execute(httpClient, createMockGetRequest());
10275
}
103-
104-
/**
105-
* Tests that a streaming operation has it's request properly cleaned up if the client is interrupted after the
106-
* response is received.
107-
*
108-
* see TT0070103230
109-
*/
110-
@Test
111-
public void clientInterruptedDuringResponseHandlers_DoesNotLeakConnection() throws Exception {
112-
InputStream mockContent = mock(InputStream.class);
113-
when(abortableCallable.call()).thenReturn(SdkHttpFullResponse.builder()
114-
.statusCode(200)
115-
.content(AbortableInputStream.create(mockContent))
116-
.build());
117-
interruptCurrentThreadAfterDelay(1000);
118-
try {
119-
requestBuilder()
120-
.originalRequest(NoopTestRequest.builder().build())
121-
.executionContext(withInterceptors(new SlowExecutionInterceptor().afterTransmissionWaitInSeconds(10)))
122-
.execute(new DummyResponseHandler().leaveConnectionOpen());
123-
fail("Expected exception");
124-
} catch (SdkClientException e) {
125-
assertThat(e.getCause(), instanceOf(InterruptedException.class));
126-
}
127-
128-
verify(mockContent).close();
129-
}
130-
131-
private AmazonSyncHttpClient.RequestExecutionBuilder requestBuilder() {
132-
return httpClient.requestExecutionBuilder().request(newGetRequest());
133-
}
134-
135-
private ExecutionContext withInterceptors(ExecutionInterceptor... requestHandlers) {
136-
return ExecutionContext.builder()
137-
.signer(new NoOpSigner())
138-
.executionAttributes(new ExecutionAttributes())
139-
.interceptorContext(InterceptorContext.builder().request(NoopTestRequest.builder().build()).build())
140-
.interceptorChain(new ExecutionInterceptorChain(Arrays.asList(requestHandlers)))
141-
.build();
142-
}
143-
}
76+
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import software.amazon.awssdk.annotations.SdkPublicApi;
2929
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
3030
import software.amazon.awssdk.core.retry.RetryPolicy;
31+
import software.amazon.awssdk.core.sync.ResponseTransformer;
3132
import software.amazon.awssdk.utils.AttributeMap;
3233
import software.amazon.awssdk.utils.CollectionUtils;
3334
import software.amazon.awssdk.utils.ToString;
@@ -299,9 +300,14 @@ default Builder retryPolicy(Consumer<RetryPolicy.Builder> retryPolicy) {
299300
* requests that don't get aborted until several seconds after the timer has been breached. Because of this, the client
300301
* execution timeout feature should not be used when absolute precision is needed.
301302
*
303+
* <p>
304+
* For synchronous streaming operations, implementations of {@link ResponseTransformer} must handle interrupt
305+
* properly to allow the the SDK to timeout the request in a timely manner.
306+
*
302307
* <p>This may be used together with {@link #apiCallAttemptTimeout()} to enforce both a timeout on each individual HTTP
303308
* request (i.e. each retry) and the total time spent on all requests across retries (i.e. the 'api call' time).
304309
*
310+
*
305311
* @see ClientOverrideConfiguration#apiCallTimeout()
306312
*/
307313
Builder apiCallTimeout(Duration apiCallTimeout);
@@ -314,8 +320,11 @@ default Builder retryPolicy(Consumer<RetryPolicy.Builder> retryPolicy) {
314320
*
315321
* <p>The request timeout feature doesn't have strict guarantees on how quickly a request is aborted when the timeout is
316322
* breached. The typical case aborts the request within a few milliseconds but there may occasionally be requests that
317-
* don't get aborted until several seconds after the timer has been breached. Because of this, the request timeout
318-
* feature should not be used when absolute precision is needed.
323+
* don't get aborted until several seconds after the timer has been breached. Because of this, the api call attempt
324+
* timeout feature should not be used when absolute precision is needed.
325+
*
326+
* <p>For synchronous streaming operations, the process in {@link ResponseTransformer} is not timed and will not
327+
* be aborted.
319328
*
320329
* <p>This may be used together with {@link #apiCallTimeout()} to enforce both a timeout on each individual HTTP
321330
* request (i.e. each retry) and the total time spent on all requests across retries (i.e. the 'api call' time).
@@ -461,4 +470,4 @@ public ClientOverrideConfiguration build() {
461470
return new ClientOverrideConfiguration(this);
462471
}
463472
}
464-
}
473+
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/AbortedException.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,14 @@
2626
@SdkPublicApi
2727
public final class AbortedException extends SdkClientException {
2828

29-
protected AbortedException(Builder b) {
29+
private AbortedException(Builder b) {
3030
super(b);
3131
}
3232

33+
public static AbortedException create(String message) {
34+
return builder().message(message).build();
35+
}
36+
3337
public static AbortedException create(String message, Throwable cause) {
3438
return builder().message(message).cause(cause).build();
3539
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonSyncHttpClient.java

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import software.amazon.awssdk.core.internal.http.pipeline.RequestPipelineBuilder;
3131
import software.amazon.awssdk.core.internal.http.pipeline.stages.AfterExecutionInterceptorsStage;
3232
import software.amazon.awssdk.core.internal.http.pipeline.stages.AfterTransmissionExecutionInterceptorsStage;
33+
import software.amazon.awssdk.core.internal.http.pipeline.stages.ApiCallAttemptTimeoutTrackingStage;
34+
import software.amazon.awssdk.core.internal.http.pipeline.stages.ApiCallTimeoutTrackingStage;
3335
import software.amazon.awssdk.core.internal.http.pipeline.stages.ApplyTransactionIdStage;
3436
import software.amazon.awssdk.core.internal.http.pipeline.stages.ApplyUserAgentStage;
3537
import software.amazon.awssdk.core.internal.http.pipeline.stages.BeforeTransmissionExecutionInterceptorsStage;
@@ -45,6 +47,7 @@
4547
import software.amazon.awssdk.core.internal.http.pipeline.stages.MoveParametersToBodyStage;
4648
import software.amazon.awssdk.core.internal.http.pipeline.stages.RetryableStage;
4749
import software.amazon.awssdk.core.internal.http.pipeline.stages.SigningStage;
50+
import software.amazon.awssdk.core.internal.http.pipeline.stages.TimeoutExceptionHandlingStage;
4851
import software.amazon.awssdk.core.internal.http.pipeline.stages.UnwrapResponseContainer;
4952
import software.amazon.awssdk.core.internal.retry.SdkDefaultRetrySetting;
5053
import software.amazon.awssdk.core.internal.util.CapacityManager;
@@ -245,26 +248,29 @@ public <OutputT> OutputT execute(HttpResponseHandler<OutputT> responseHandler) {
245248
return RequestPipelineBuilder
246249
// Start of mutating request
247250
.first(RequestPipelineBuilder
248-
.first(MakeRequestMutableStage::new)
249-
.then(ApplyTransactionIdStage::new)
250-
.then(ApplyUserAgentStage::new)
251-
.then(MergeCustomHeadersStage::new)
252-
.then(MergeCustomQueryParamsStage::new)
253-
.then(MoveParametersToBodyStage::new)
254-
.then(MakeRequestImmutableStage::new)
255-
// End of mutating request
256-
.then(RequestPipelineBuilder
257-
.first(SigningStage::new)
258-
.then(BeforeTransmissionExecutionInterceptorsStage::new)
259-
.then(MakeHttpRequestStage::new)
260-
.then(AfterTransmissionExecutionInterceptorsStage::new)
261-
.then(Crc32ValidationStage::new)
262-
.then(BeforeUnmarshallingExecutionInterceptorsStage::new)
263-
.then(() -> new HandleResponseStage<>(
264-
getNonNullResponseHandler(responseHandler),
265-
getNonNullResponseHandler(errorResponseHandler)))
266-
.wrappedWith(RetryableStage::new)::build)
267-
.wrappedWith(StreamManagingStage::new)::build)
251+
.first(MakeRequestMutableStage::new)
252+
.then(ApplyTransactionIdStage::new)
253+
.then(ApplyUserAgentStage::new)
254+
.then(MergeCustomHeadersStage::new)
255+
.then(MergeCustomQueryParamsStage::new)
256+
.then(MoveParametersToBodyStage::new)
257+
.then(MakeRequestImmutableStage::new)
258+
// End of mutating request
259+
.then(RequestPipelineBuilder
260+
.first(SigningStage::new)
261+
.then(BeforeTransmissionExecutionInterceptorsStage::new)
262+
.then(MakeHttpRequestStage::new)
263+
.then(AfterTransmissionExecutionInterceptorsStage::new)
264+
.then(Crc32ValidationStage::new)
265+
.then(BeforeUnmarshallingExecutionInterceptorsStage::new)
266+
.then(() -> new HandleResponseStage<>(
267+
getNonNullResponseHandler(responseHandler),
268+
getNonNullResponseHandler(errorResponseHandler)))
269+
.wrappedWith(ApiCallAttemptTimeoutTrackingStage::new)
270+
.wrappedWith(TimeoutExceptionHandlingStage::new)
271+
.wrappedWith(RetryableStage::new)::build)
272+
.wrappedWith(StreamManagingStage::new)
273+
.wrappedWith(ApiCallTimeoutTrackingStage::new)::build)
268274
.then(() -> new UnwrapResponseContainer<>())
269275
.then(() -> new AfterExecutionInterceptorsStage<>())
270276
.wrappedWith(ExecutionFailureExceptionReportingStage::new)
@@ -291,4 +297,4 @@ private RequestExecutionContext createRequestExecutionDependencies() {
291297

292298
}
293299

294-
}
300+
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/RequestExecutionContext.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public final class RequestExecutionContext {
4141
private final SdkRequest originalRequest;
4242
private final ExecutionContext executionContext;
4343
private TimeoutTracker apiCallTimeoutTracker;
44+
private TimeoutTracker apiCallAttemptTimeoutTracker;
4445

4546
private RequestExecutionContext(Builder builder) {
4647
this.requestProvider = builder.requestProvider;
@@ -109,6 +110,13 @@ public void apiCallTimeoutTracker(TimeoutTracker timeoutTracker) {
109110
this.apiCallTimeoutTracker = timeoutTracker;
110111
}
111112

113+
public TimeoutTracker apiCallAttemptTimeoutTracker() {
114+
return apiCallAttemptTimeoutTracker;
115+
}
116+
117+
public void apiCallAttemptTimeoutTracker(TimeoutTracker timeoutTracker) {
118+
this.apiCallAttemptTimeoutTracker = timeoutTracker;
119+
}
112120

113121
/**
114122
* An SDK-internal implementation of {@link Builder}.

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/StreamManagingStage.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public StreamManagingStage(RequestPipeline<SdkHttpFullRequest, Response<OutputT>
5353
public Response<OutputT> execute(SdkHttpFullRequest request, RequestExecutionContext context) throws Exception {
5454
Optional<InputStream> toBeClosed = createManagedStream(request);
5555
try {
56+
InterruptMonitor.checkInterrupted();
5657
return wrapped.execute(request.toBuilder().content(nonCloseableInputStream(toBeClosed).orElse(null)).build(),
5758
context);
5859
} finally {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public class AfterTransmissionExecutionInterceptorsStage
3030
@Override
3131
public Pair<SdkHttpFullRequest, SdkHttpFullResponse> execute(Pair<SdkHttpFullRequest, SdkHttpFullResponse> input,
3232
RequestExecutionContext context) throws Exception {
33+
InterruptMonitor.checkInterrupted();
3334
// Update interceptor context
3435
InterceptorContext interceptorContext =
3536
context.executionContext().interceptorContext().copy(b -> b.httpResponse(input.right()));
@@ -43,8 +44,6 @@ public Pair<SdkHttpFullRequest, SdkHttpFullResponse> execute(Pair<SdkHttpFullReq
4344
// Store updated context
4445
context.executionContext().interceptorContext(interceptorContext);
4546

46-
// TODO: Why do we do this for sync, but not async? Are there other places it should be? Not having this fails the
47-
// AbortedExceptionClientExecutionTimerIntegrationTest
4847
InterruptMonitor.checkInterrupted(interceptorContext.httpResponse());
4948

5049
return Pair.of(input.left(), interceptorContext.httpResponse());

0 commit comments

Comments
 (0)