Skip to content

Commit

Permalink
Add retry conditions to the strategy if none is configured (#5648)
Browse files Browse the repository at this point in the history
* Add retry conditions to the strategy if none is configured

* Update core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java

* Fix a typo in the name of the class
  • Loading branch information
sugmanue authored Oct 8, 2024
1 parent 85857b5 commit f5b49a7
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 0 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-8d6fb63.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "",
"type": "bugfix",
"description": "This change adds the default retry conditions in the client builder if none are configured to behave similarly to retry policies that are configured behind the scenes without the users having to do that themselves. This will prevent customers using directly the retry strategies builders from `DefaultRetryStrategy` to end up with a no-op strategy."
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import software.amazon.awssdk.core.client.config.SdkClientOption;
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption;
import software.amazon.awssdk.core.internal.retry.SdkDefaultRetryStrategy;
import software.amazon.awssdk.core.retry.RetryMode;
import software.amazon.awssdk.core.retry.RetryPolicy;
import software.amazon.awssdk.http.SdkHttpClient;
Expand All @@ -64,6 +65,7 @@
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;
import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain;
import software.amazon.awssdk.retries.api.RetryStrategy;
import software.amazon.awssdk.retries.internal.BaseRetryStrategy;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.AttributeMap.LazyValueSource;
import software.amazon.awssdk.utils.CollectionUtils;
Expand Down Expand Up @@ -422,6 +424,20 @@ private void configureRetryPolicy(SdkClientConfiguration.Builder config) {
private void configureRetryStrategy(SdkClientConfiguration.Builder config) {
RetryStrategy strategy = config.option(SdkClientOption.RETRY_STRATEGY);
if (strategy != null) {
// Fix the retry strategy if no retry predicates were configured. Users using
// DefaultRetryStrategy do not have any conditions configured as that package
// does not know anything about the SDK or AWS retry conditions. This mimics the
// retry policies behavior that are configured behind the scenes and avoids the
// risk of customers inadvertently using a no-op retry strategy.
if (strategy.maxAttempts() > 1
&& (strategy instanceof BaseRetryStrategy)
&& !((BaseRetryStrategy) strategy).hasRetryPredicates()
) {
RetryStrategy.Builder<?, ?> builder = strategy.toBuilder();
SdkDefaultRetryStrategy.configureStrategy(builder);
AwsRetryStrategy.configureStrategy(builder);
config.option(SdkClientOption.RETRY_STRATEGY, builder.build());
}
return;
}
config.lazyOption(SdkClientOption.RETRY_STRATEGY, this::resolveAwsRetryStrategy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,15 @@ protected int exceptionCost(RefreshRetryTokenRequest request) {
return 0;
}

/**
* Returns true if there are retry predicates configured for this retry strategy.
*
* @return true if there are retry predicates configured for this retry strategy.
*/
public final boolean hasRetryPredicates() {
return !retryPredicates.isEmpty();
}

private DefaultRetryToken refreshToken(RefreshRetryTokenRequest request, AcquireResponse acquireResponse) {
DefaultRetryToken token = asDefaultRetryToken(request.token());
return token.toBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ public static AdaptiveRetryStrategy.Builder adaptiveRetryStrategyBuilder() {
return builder;
}

/**
* Configures a retry strategy using its builder to add SDK-generic retry exceptions.
*
* @param builder The builder to add the SDK-generic retry exceptions
* @return The given builder
*/
public static RetryStrategy.Builder<?, ?> configureStrategy(RetryStrategy.Builder<?, ?> builder) {
builder.retryOnException(SdkDefaultRetryStrategy::retryOnRetryableException)
.retryOnException(SdkDefaultRetryStrategy::retryOnStatusCodes)
.retryOnException(SdkDefaultRetryStrategy::retryOnClockSkewException)
.retryOnException(SdkDefaultRetryStrategy::retryOnThrottlingCondition);
SdkDefaultRetrySetting.RETRYABLE_EXCEPTIONS.forEach(builder::retryOnExceptionOrCauseInstanceOf);
builder.treatAsThrottling(SdkDefaultRetryStrategy::treatAsThrottling);
return builder;
}

private static boolean treatAsThrottling(Throwable t) {
if (t instanceof SdkException) {
return RetryUtils.isThrottlingException((SdkException) t);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/*
* 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.protocol.tests.retry;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.post;
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

import com.github.tomakehurst.wiremock.junit.WireMockRule;
import com.github.tomakehurst.wiremock.stubbing.Scenario;
import java.net.URI;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.retries.DefaultRetryStrategy;
import software.amazon.awssdk.services.protocoljsonrpc.ProtocolJsonRpcClient;
import software.amazon.awssdk.services.protocoljsonrpc.model.AllTypesRequest;
import software.amazon.awssdk.services.protocoljsonrpc.model.AllTypesResponse;
import software.amazon.awssdk.services.protocoljsonrpc.model.ProtocolJsonRpcException;

public class RetryStrategyBackfillsEmptyRetryConditions {

private static final String PATH = "/";
private static final String JSON_BODY = "{\"StringMember\":\"foo\"}";

@Rule
public WireMockRule wireMock = new WireMockRule(0);

private ProtocolJsonRpcClient client;

@Before
public void setupClient() {
client = ProtocolJsonRpcClient.builder()
.credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("akid",
"skid")))
.region(Region.US_EAST_1)
.overrideConfiguration(o -> o.retryStrategy(DefaultRetryStrategy.standardStrategyBuilder().build()))
.endpointOverride(URI.create("http://localhost:" + wireMock.port()))
.build();
}

@Test
public void shouldRetryOn500() {
stubFor(post(urlEqualTo(PATH))
.inScenario("retry at 500")
.whenScenarioStateIs(Scenario.STARTED)
.willSetStateTo("first attempt")
.willReturn(aResponse()
.withStatus(500)));

stubFor(post(urlEqualTo(PATH))
.inScenario("retry at 500")
.whenScenarioStateIs("first attempt")
.willSetStateTo("second attempt")
.willReturn(aResponse()
.withStatus(200)
.withBody(JSON_BODY)));

AllTypesResponse allTypesResponse = client.allTypes(AllTypesRequest.builder().build());
assertThat(allTypesResponse).isNotNull();
}

@Test
public void shouldRetryOnRetryableAwsErrorCode() {
stubFor(post(urlEqualTo(PATH))
.inScenario("retry at PriorRequestNotComplete")
.whenScenarioStateIs(Scenario.STARTED)
.willSetStateTo("first attempt")
.willReturn(aResponse()
.withStatus(400)
.withHeader("x-amzn-ErrorType", "PriorRequestNotComplete")
.withBody("\"{\"__type\":\"PriorRequestNotComplete\",\"message\":\"Blah "
+ "error\"}\"")));

stubFor(post(urlEqualTo(PATH))
.inScenario("retry at PriorRequestNotComplete")
.whenScenarioStateIs("first attempt")
.willSetStateTo("second attempt")
.willReturn(aResponse()
.withStatus(200)
.withBody(JSON_BODY)));

AllTypesResponse allTypesResponse = client.allTypes(AllTypesRequest.builder().build());
assertThat(allTypesResponse).isNotNull();
}

@Test
public void shouldRetryOnAwsThrottlingErrorCode() {
stubFor(post(urlEqualTo(PATH))
.inScenario("retry at SlowDown")
.whenScenarioStateIs(Scenario.STARTED)
.willSetStateTo("first attempt")
.willReturn(aResponse()
.withStatus(400)
.withHeader("x-amzn-ErrorType", "SlowDown")
.withBody("\"{\"__type\":\"SlowDown\",\"message\":\"Blah "
+ "error\"}\"")));

stubFor(post(urlEqualTo(PATH))
.inScenario("retry at SlowDown")
.whenScenarioStateIs("first attempt")
.willSetStateTo("second attempt")
.willReturn(aResponse()
.withStatus(200)
.withBody(JSON_BODY)));

AllTypesResponse allTypesResponse = client.allTypes(AllTypesRequest.builder().build());
assertThat(allTypesResponse).isNotNull();
}

@Test
public void retryStrategyNone_shouldNotRetry() {
stubFor(post(urlEqualTo(PATH))
.inScenario("retry at 500")
.whenScenarioStateIs(Scenario.STARTED)
.willSetStateTo("first attempt")
.willReturn(aResponse()
.withStatus(500)));

stubFor(post(urlEqualTo(PATH))
.inScenario("retry at 500")
.whenScenarioStateIs("first attempt")
.willSetStateTo("second attempt")
.willReturn(aResponse()
.withStatus(200)
.withBody(JSON_BODY)));

ProtocolJsonRpcClient clientWithNoRetry =
ProtocolJsonRpcClient.builder()
.credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("akid",
"skid")))
.region(Region.US_EAST_1)
.endpointOverride(URI.create("http://localhost:" + wireMock.port()))
// When max attempts is 1 (i.e., just the first attempt is allowed but no retries),
// the back filling should not happen.
.overrideConfiguration(c -> c.retryStrategy(DefaultRetryStrategy.standardStrategyBuilder()
.maxAttempts(1)
.build()))
.build();

assertThatThrownBy(() -> clientWithNoRetry.allTypes(AllTypesRequest.builder().build())).isInstanceOf(ProtocolJsonRpcException.class);
}
}

0 comments on commit f5b49a7

Please sign in to comment.