diff --git a/.changelog/bugfix-fix-new-retries-gaps.json b/.changelog/bugfix-fix-new-retries-gaps.json new file mode 100644 index 00000000000..be5ea72e65e --- /dev/null +++ b/.changelog/bugfix-fix-new-retries-gaps.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "aws-cpp-sdk-core", + "contributor": "pulimsr", + "description": "Fix AWS_NEW_RETRIES_2026 gating: restore DefaultRetryStrategy fallback when the flag is off, read the flag case-insensitively in smithy clients, and let AWS_MAX_ATTEMPTS override the DynamoDB default when the flag is on" +} diff --git a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h index 038673b389d..761e9d7f193 100644 --- a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h +++ b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h @@ -267,7 +267,7 @@ namespace client ResponseHandlerFunc&& responseHandler, EndpointUpdateCallback&& endpointCallback ) const; - bool m_enableNewRetries; + bool m_enableNewRetries = false; }; } // namespace client } // namespace smithy diff --git a/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp b/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp index 43a665485fa..5449e28d8d1 100644 --- a/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp +++ b/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp @@ -617,26 +617,33 @@ std::shared_ptr InitRetryStrategy(int maxAttempts, Aws::String re } else { - if (Aws::Utils::StringUtils::ToLower(Aws::Environment::GetEnv("AWS_NEW_RETRIES_2026").c_str()) != "true") - { - if (maxAttempts < 0) - { - retryStrategy = Aws::MakeShared(CLIENT_CONFIG_TAG); - } - else - { - retryStrategy = Aws::MakeShared(CLIENT_CONFIG_TAG, maxAttempts); - } - } - else - { - retryStrategy = Aws::MakeShared(CLIENT_CONFIG_TAG); - } + retryStrategy = Aws::MakeShared(CLIENT_CONFIG_TAG); } return retryStrategy; } +static int ResolveConfiguredMaxAttempts() +{ + Aws::String maxAttemptsString = Aws::Environment::GetEnv("AWS_MAX_ATTEMPTS"); + if (maxAttemptsString.empty()) + { + maxAttemptsString = Aws::Config::GetCachedConfigValue("max_attempts"); + } + // In case users specify 0 explicitly to disable retry. + if (maxAttemptsString == "0") + { + return 0; + } + int maxAttempts = static_cast(Aws::Utils::StringUtils::ConvertToInt32(maxAttemptsString.c_str())); + if (maxAttempts == 0) + { + AWS_LOGSTREAM_INFO(CLIENT_CONFIG_TAG, "Retry Strategy will use the default max attempts."); + return -1; + } + return maxAttempts; +} + std::shared_ptr InitRetryStrategy(int maxAttempts, Aws::String retryMode, double transientBackoffBaseSec) { if (Aws::Utils::StringUtils::ToLower(Aws::Environment::GetEnv("AWS_NEW_RETRIES_2026").c_str()) != "true") { @@ -644,6 +651,13 @@ std::shared_ptr InitRetryStrategy(int maxAttempts, Aws::String re return InitRetryStrategy(retryMode); } + // maxAttempts is a service default. A max attempts value the customer configured wins. + const int configuredMaxAttempts = ResolveConfiguredMaxAttempts(); + if (configuredMaxAttempts >= 0) + { + maxAttempts = configuredMaxAttempts; + } + // Only standard mode honors a service-specific backoff base; other modes are unaffected, // so they go through the default-base path. if (ResolveRetryMode(retryMode) != "standard") @@ -657,28 +671,7 @@ std::shared_ptr InitRetryStrategy(int maxAttempts, Aws::String re std::shared_ptr InitRetryStrategy(Aws::String retryMode) { - int maxAttempts = 0; - Aws::String maxAttemptsString = Aws::Environment::GetEnv("AWS_MAX_ATTEMPTS"); - if (maxAttemptsString.empty()) - { - maxAttemptsString = Aws::Config::GetCachedConfigValue("max_attempts"); - } - // In case users specify 0 explicitly to disable retry. - if (maxAttemptsString == "0") - { - maxAttempts = 0; - } - else - { - maxAttempts = static_cast(Aws::Utils::StringUtils::ConvertToInt32(maxAttemptsString.c_str())); - if (maxAttempts == 0) - { - AWS_LOGSTREAM_INFO(CLIENT_CONFIG_TAG, "Retry Strategy will use the default max attempts."); - maxAttempts = -1; - } - } - - return InitRetryStrategy(maxAttempts, retryMode); + return InitRetryStrategy(ResolveConfiguredMaxAttempts(), retryMode); } Aws::String ClientConfiguration::LoadConfigFromEnvOrProfile(const Aws::String& envKey, diff --git a/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp b/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp index 8886461ed0d..5893181de46 100644 --- a/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp +++ b/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp @@ -108,6 +108,7 @@ void AwsSmithyClientBase::baseCopyAssign(const AwsSmithyClientBase& other, std::shared_ptr errorMarshaller) { m_serviceName = other.m_serviceName; m_serviceUserAgentName = other.m_serviceUserAgentName; + m_enableNewRetries = other.m_enableNewRetries; m_httpClient = std::move(httpClient); m_errorMarshaller = std::move(errorMarshaller); @@ -129,6 +130,7 @@ void AwsSmithyClientBase::baseMoveAssign(AwsSmithyClientBase&& other) { m_errorMarshaller = std::move(other.m_errorMarshaller); m_interceptors = std::move(other.m_interceptors); m_userAgentInterceptor = std::move(other.m_userAgentInterceptor); + m_enableNewRetries = other.m_enableNewRetries; } std::shared_ptr @@ -335,10 +337,7 @@ std::shared_ptr AwsSmithyClientBase::CreateR } pRequestCtx->m_requestInfo.attempt = 1; pRequestCtx->m_attemptSkew = m_clientSkew->Load(); - pRequestCtx->m_requestInfo.maxAttempts = - Aws::Environment::GetEnv("AWS_NEW_RETRIES_2026") == "true" - ? m_clientConfig->retryStrategy->GetMaxAttempts() - : 0; + pRequestCtx->m_requestInfo.maxAttempts = m_enableNewRetries ? m_clientConfig->retryStrategy->GetMaxAttempts() : 0; pRequestCtx->m_interceptorContext = Aws::MakeShared(AWS_SMITHY_CLIENT_LOG, *request); pRequestCtx->m_responseHandler = std::move(responseHandler); pRequestCtx->m_authResolvedCallback = std::move(authCallback); diff --git a/tests/aws-cpp-sdk-core-tests/aws/client/RetryStrategyTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/client/RetryStrategyTest.cpp index 27b57994acf..8f8290e7bc4 100644 --- a/tests/aws-cpp-sdk-core-tests/aws/client/RetryStrategyTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/aws/client/RetryStrategyTest.cpp @@ -7,8 +7,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -317,3 +319,40 @@ TEST_F(NewRetriesStrategyTest, DynamoDBBackoffBase) ASSERT_GE(delay, 0); ASSERT_LE(delay, 1000); } + +class InitRetryStrategyTest : public Aws::Testing::AwsCppSdkGTestSuite +{ +protected: + void SetUp() override { Aws::Config::ReloadCachedConfigFile(); } + void TearDown() override { Aws::Config::ReloadCachedConfigFile(); } +}; + +TEST_F(InitRetryStrategyTest, GateOffUnsetRetryModeUsesDefaultRetryStrategy) +{ + Aws::Environment::EnvironmentRAII env{{{"AWS_NEW_RETRIES_2026", ""}, + {"AWS_RETRY_MODE", ""}, + {"AWS_MAX_ATTEMPTS", ""}, + {"AWS_CONFIG_FILE", "/nonexistent/retry-strategy-test-config"}}}; + Aws::Config::ReloadCachedConfigFile(); + + auto retryStrategy = InitRetryStrategy(); + ASSERT_TRUE(retryStrategy); + ASSERT_STREQ("default", retryStrategy->GetStrategyName()); + ASSERT_EQ(11, retryStrategy->GetMaxAttempts()); +} + +TEST_F(InitRetryStrategyTest, ConfiguredMaxAttemptsOverridesServiceDefault) +{ + for (const auto& testCase : {std::make_pair("", 4L), std::make_pair("2", 2L), std::make_pair("0", 0L)}) + { + Aws::Environment::EnvironmentRAII env{{{"AWS_NEW_RETRIES_2026", "true"}, + {"AWS_RETRY_MODE", ""}, + {"AWS_MAX_ATTEMPTS", testCase.first}, + {"AWS_CONFIG_FILE", "/nonexistent/retry-strategy-test-config"}}}; + Aws::Config::ReloadCachedConfigFile(); + + auto retryStrategy = InitRetryStrategy(4, "", 0.025); + ASSERT_TRUE(retryStrategy); + ASSERT_EQ(testCase.second, retryStrategy->GetMaxAttempts()) << "AWS_MAX_ATTEMPTS=" << testCase.first; + } +} diff --git a/tests/aws-cpp-sdk-core-tests/smithy/client/AsyncSmithyClientTest.cpp b/tests/aws-cpp-sdk-core-tests/smithy/client/AsyncSmithyClientTest.cpp index b0b51c27430..6ca7f967377 100644 --- a/tests/aws-cpp-sdk-core-tests/smithy/client/AsyncSmithyClientTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/smithy/client/AsyncSmithyClientTest.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -300,4 +301,18 @@ namespace ASSERT_LT(elapsed, std::chrono::milliseconds(LONG_POLL_DELAY_MS)); #endif } + + TEST_F(AsyncSmithyClientTest, NewRetriesFlagIsCaseInsensitive) + { + Aws::Environment::EnvironmentRAII env{{{"AWS_NEW_RETRIES_2026", "TRUE"}}}; + client = MakeClient(Aws::MakeShared(ALLOCATION_TAG, 3)); + mockHttpClient->AddResponseToReturn(MakeSuccessResponse()); + + auto outcome = RunAndCapture(); + + ASSERT_TRUE(outcome.IsSuccess()); + const auto& requests = mockHttpClient->GetAllRequestsMade(); + ASSERT_EQ(1u, requests.size()); + ASSERT_NE(Aws::String::npos, requests[0].GetHeaders()[Aws::Http::SDK_REQUEST_HEADER].find("max=3")); + } }