From 51f14da53517f520201f543358f4685685289a11 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 29 Apr 2024 09:33:39 -1000 Subject: [PATCH 1/7] Add a virtual ShouldTry method to the RetryPolicy for customization. --- .../inc/azure/core/http/policies/policy.hpp | 60 +- sdk/core/azure-core/src/http/retry_policy.cpp | 18 +- .../azure-core/test/ut/retry_policy_test.cpp | 1326 +++++++++-------- 3 files changed, 725 insertions(+), 679 deletions(-) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 511a1c3a7a..2e0a796d9c 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -39,6 +39,13 @@ */ extern std::shared_ptr AzureSdkGetCustomHttpTransport(); +#if defined(_azure_TESTING_BUILD) +namespace Azure { namespace Core { namespace Http { namespace Policies { namespace Tests { + class RetryPolicyTest; + class RetryLogic; +}}}}} // namespace Azure::Core::Http::Policies::Tests +#endif // _azure_TESTING_BUILD + namespace Azure { namespace Core { namespace Http { namespace Policies { struct TransportOptions; @@ -363,14 +370,28 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { /** * @brief HTTP retry policy. */ - class RetryPolicy -#if !defined(_azure_TESTING_BUILD) - final + class RetryPolicy : public HttpPolicy { +#if defined(_azure_TESTING_BUILD) + friend class Azure::Core::Http::Policies::Tests::RetryPolicyTest; + friend class Azure::Core::Http::Policies::Tests::RetryLogic; #endif - : public HttpPolicy { + private: RetryOptions m_retryOptions; + bool ShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor = -1) const; + + bool ShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor = -1) const; + public: /** * Constructs HTTP retry policy with the provided #Azure::Core::Http::Policies::RetryOptions. @@ -403,18 +424,25 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { static int32_t GetRetryCount(Context const& context); protected: - virtual bool ShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor = -1) const; - - virtual bool ShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor = -1) const; + /** + * @brief Overriding this method customizes the logic of when the RetryPolicy will re-attempt + * a request, based on the returned HTTP response. + * + * @remark A null response pointer means there was no response received from the corresponding + * request. Custom implementations of this method that override the retry behavior, should + * handle that error case, if that needs to be customized. + * + * @remark Unless overriden, the default implementation is to always return true. The + * non-retriable errors, including those specified in the RetryOptions, remain evaluated + * before calling ShouldRetry. + * + * @param response An HTTP response returned corresponding to the request sent by the policy. + * @param retryOptions The set of options provided to the RetryPolicy. + * @return Whether or not the HTTP request should be sent againg through the pipeline. + */ + virtual bool ShouldRetry( + std::unique_ptr const& response, + RetryOptions const& retryOptions) const; }; /** diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index 217e0bd898..c6533d0395 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -118,6 +118,15 @@ int32_t RetryPolicy::GetRetryCount(Context const& context) return *ptr; } +bool RetryPolicy::ShouldRetry( + std::unique_ptr const& response, + RetryOptions const& retryOptions) const +{ + (void)response; + (void)retryOptions; + return true; +} + std::unique_ptr RetryPolicy::Send( Request& request, NextHttpPolicy nextPolicy, @@ -141,13 +150,20 @@ std::unique_ptr RetryPolicy::Send( auto response = nextPolicy.Send(request, retryContext); // If we are out of retry attempts, if a response is non-retriable (or simply 200 OK, i.e - // doesn't need to be retried), then ShouldRetry returns false. + // doesn't need to be retried), then ShouldRetryOnResponse returns false. if (!ShouldRetryOnResponse(*response.get(), m_retryOptions, attempt, retryAfter)) { // If this is the second attempt and StartTry was called, we need to stop it. Otherwise // trying to perform same request would use last retry query/headers return response; } + + // Service SDKs can inject custom logic to define whether the request should be retried, + // based on the response. The default is true. + if (!ShouldRetry(response, m_retryOptions)) + { + return response; + } } catch (const TransportException& e) { diff --git a/sdk/core/azure-core/test/ut/retry_policy_test.cpp b/sdk/core/azure-core/test/ut/retry_policy_test.cpp index b482418ce5..278b72eec9 100644 --- a/sdk/core/azure-core/test/ut/retry_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/retry_policy_test.cpp @@ -13,823 +13,825 @@ using namespace Azure::Core::Http; using namespace Azure::Core::Http::Policies; using namespace Azure::Core::Http::Policies::_internal; -namespace { -class TestTransportPolicy final : public HttpPolicy { -private: - std::function()> m_send; - -public: - TestTransportPolicy(std::function()> send) : m_send(send) {} - - std::unique_ptr Send( - Request&, - NextHttpPolicy, - Azure::Core::Context const&) const override - { - return m_send(); - } +namespace Azure { namespace Core { namespace Http { namespace Policies { namespace Tests { + class TestTransportPolicy final : public HttpPolicy { + private: + std::function()> m_send; + + public: + TestTransportPolicy(std::function()> send) : m_send(send) {} + + std::unique_ptr Send( + Request&, + NextHttpPolicy, + Azure::Core::Context const&) const override + { + return m_send(); + } - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } -}; - -class RetryPolicyTest final : public RetryPolicy { -private: - std::function - m_shouldRetryOnTransportFailure; - - std::function< - bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> - m_shouldRetryOnResponse; - -public: - bool BaseShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } + }; + + class RetryPolicyTest final : public RetryPolicy { + private: + std::function + m_shouldRetryOnTransportFailure; + + std::function< + bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> + m_shouldRetryOnResponse; + + public: + bool BaseShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } - bool BaseShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } + bool BaseShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } - RetryPolicyTest( - RetryOptions const& retryOptions, - decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, - decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) - : RetryPolicy(retryOptions), - m_shouldRetryOnTransportFailure( - shouldRetryOnTransportFailure != nullptr // - ? shouldRetryOnTransportFailure - : static_cast( // - [this](auto options, auto attempt, auto retryAfter, auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnTransportFailure( - options, attempt, ignore, jitter); - })), - m_shouldRetryOnResponse( - shouldRetryOnResponse != nullptr // - ? shouldRetryOnResponse - : static_cast( // - [this]( - RawResponse const& response, - auto options, - auto attempt, - auto retryAfter, - auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnResponse( - response, options, attempt, ignore, jitter); - })) - { - } + RetryPolicyTest( + RetryOptions const& retryOptions, + decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, + decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) + : RetryPolicy(retryOptions), + m_shouldRetryOnTransportFailure( + shouldRetryOnTransportFailure != nullptr // + ? shouldRetryOnTransportFailure + : static_cast( // + [this](auto options, auto attempt, auto retryAfter, auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnTransportFailure( + options, attempt, ignore, jitter); + })), + m_shouldRetryOnResponse( + shouldRetryOnResponse != nullptr // + ? shouldRetryOnResponse + : static_cast( // + [this]( + RawResponse const& response, + auto options, + auto attempt, + auto retryAfter, + auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnResponse( + response, options, attempt, ignore, jitter); + })) + { + } - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } -protected: - bool ShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const override - { - return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); - } + bool ShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); + } + + bool ShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); + } + }; - bool ShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const override + TEST(RetryPolicy, ShouldRetryOnResponse) { - return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); - } -}; -} // namespace + using namespace std::chrono_literals; + RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; -TEST(RetryPolicy, ShouldRetryOnResponse) -{ - using namespace std::chrono_literals; - RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; + RawResponse const* responsePtrSent = nullptr; - RawResponse const* responsePtrSent = nullptr; + RawResponse const* responsePtrReceived = nullptr; + RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; + int32_t attemptReceived = -1234; + double jitterReceived = -5678; - RawResponse const* responsePtrReceived = nullptr; - RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; - int32_t attemptReceived = -1234; - double jitterReceived = -5678; + int onTransportFailureInvoked = 0; + int onResponseInvoked = 0; - int onTransportFailureInvoked = 0; - int onResponseInvoked = 0; + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + }, + [&](RawResponse const& response, auto options, auto attempt, auto, auto jitter) { + ++onResponseInvoked; + responsePtrReceived = &response; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + })); + + policies.emplace_back(std::make_unique([&]() { + auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); + + responsePtrSent = response.get(); + + return response; + })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, Azure::Core::Context()); + } - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - }, - [&](RawResponse const& response, auto options, auto attempt, auto, auto jitter) { - ++onResponseInvoked; - responsePtrReceived = &response; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - })); - - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); - - responsePtrSent = response.get(); - - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, Azure::Core::Context()); - } + EXPECT_EQ(onTransportFailureInvoked, 0); + EXPECT_EQ(onResponseInvoked, 1); - EXPECT_EQ(onTransportFailureInvoked, 0); - EXPECT_EQ(onResponseInvoked, 1); + EXPECT_NE(responsePtrSent, nullptr); + EXPECT_EQ(responsePtrSent, responsePtrReceived); - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(responsePtrSent, responsePtrReceived); + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + EXPECT_EQ(attemptReceived, 1); + EXPECT_EQ(jitterReceived, -1); - EXPECT_EQ(attemptReceived, 1); - EXPECT_EQ(jitterReceived, -1); + // 3 attempts + responsePtrSent = nullptr; - // 3 attempts - responsePtrSent = nullptr; + responsePtrReceived = nullptr; + retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; + attemptReceived = -1234; + jitterReceived = -5678; - responsePtrReceived = nullptr; - retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; - attemptReceived = -1234; - jitterReceived = -5678; + onTransportFailureInvoked = 0; + onResponseInvoked = 0; - onTransportFailureInvoked = 0; - onResponseInvoked = 0; + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + }, + [&](RawResponse const& response, + auto options, + auto attempt, + auto retryAfter, + auto jitter) { + ++onResponseInvoked; + responsePtrReceived = &response; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + retryAfter = 1ms; + return onResponseInvoked < 3; + })); + + policies.emplace_back(std::make_unique([&]() { + auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); + + responsePtrSent = response.get(); + + return response; + })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, Azure::Core::Context()); + } - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - }, - [&](RawResponse const& response, auto options, auto attempt, auto retryAfter, auto jitter) { - ++onResponseInvoked; - responsePtrReceived = &response; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - retryAfter = 1ms; - return onResponseInvoked < 3; - })); - - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); - - responsePtrSent = response.get(); - - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, Azure::Core::Context()); - } + EXPECT_EQ(onTransportFailureInvoked, 0); + EXPECT_EQ(onResponseInvoked, 3); - EXPECT_EQ(onTransportFailureInvoked, 0); - EXPECT_EQ(onResponseInvoked, 3); + EXPECT_NE(responsePtrSent, nullptr); + EXPECT_EQ(responsePtrSent, responsePtrReceived); - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(responsePtrSent, responsePtrReceived); + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + EXPECT_EQ(attemptReceived, 3); + EXPECT_EQ(jitterReceived, -1); + } - EXPECT_EQ(attemptReceived, 3); - EXPECT_EQ(jitterReceived, -1); -} + TEST(RetryPolicy, ShouldRetryOnTransportFailure) + { + using namespace std::chrono_literals; + RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; -TEST(RetryPolicy, ShouldRetryOnTransportFailure) -{ - using namespace std::chrono_literals; - RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; + RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; + int32_t attemptReceived = -1234; + double jitterReceived = -5678; - RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; - int32_t attemptReceived = -1234; - double jitterReceived = -5678; + int onTransportFailureInvoked = 0; + int onResponseInvoked = 0; - int onTransportFailureInvoked = 0; - int onResponseInvoked = 0; + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + }, + [&](auto, auto options, auto attempt, auto, auto jitter) { + ++onResponseInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + })); + + policies.emplace_back(std::make_unique( + []() -> std::unique_ptr { throw TransportException("Test"); })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); + } - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - }, - [&](auto, auto options, auto attempt, auto, auto jitter) { - ++onResponseInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - })); - - policies.emplace_back(std::make_unique( - []() -> std::unique_ptr { throw TransportException("Test"); })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); - } + EXPECT_EQ(onTransportFailureInvoked, 1); + EXPECT_EQ(onResponseInvoked, 0); - EXPECT_EQ(onTransportFailureInvoked, 1); - EXPECT_EQ(onResponseInvoked, 0); + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + EXPECT_EQ(attemptReceived, 1); + EXPECT_EQ(jitterReceived, -1); - EXPECT_EQ(attemptReceived, 1); - EXPECT_EQ(jitterReceived, -1); + // 3 attempts + retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; + attemptReceived = -1234; + jitterReceived = -5678; - // 3 attempts - retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; - attemptReceived = -1234; - jitterReceived = -5678; + onTransportFailureInvoked = 0; + onResponseInvoked = 0; - onTransportFailureInvoked = 0; - onResponseInvoked = 0; + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return onTransportFailureInvoked < 3; + }, + [&](auto, auto options, auto attempt, auto retryAfter, auto jitter) { + ++onResponseInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + retryAfter = 1ms; + return false; + })); + + policies.emplace_back(std::make_unique( + []() -> std::unique_ptr { throw TransportException("Test"); })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); + } - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return onTransportFailureInvoked < 3; - }, - [&](auto, auto options, auto attempt, auto retryAfter, auto jitter) { - ++onResponseInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - retryAfter = 1ms; - return false; - })); - - policies.emplace_back(std::make_unique( - []() -> std::unique_ptr { throw TransportException("Test"); })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); + EXPECT_EQ(onTransportFailureInvoked, 3); + EXPECT_EQ(onResponseInvoked, 0); + + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + + EXPECT_EQ(attemptReceived, 3); + EXPECT_EQ(jitterReceived, -1); } - EXPECT_EQ(onTransportFailureInvoked, 3); - EXPECT_EQ(onResponseInvoked, 0); + class RetryLogic final : private RetryPolicy { + RetryLogic() : RetryPolicy(RetryOptions()) {} + ~RetryLogic() {} - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + static RetryLogic const g_retryPolicy; - EXPECT_EQ(attemptReceived, 3); - EXPECT_EQ(jitterReceived, -1); -} + public: + static bool TestShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } -namespace { -class RetryLogic final : private RetryPolicy { - RetryLogic() : RetryPolicy(RetryOptions()) {} - ~RetryLogic() {} + static bool TestShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } + }; - static RetryLogic const g_retryPolicy; + RetryLogic const RetryLogic::g_retryPolicy; -public: - static bool TestShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) + TEST(RetryPolicy, Exponential) { - return g_retryPolicy.ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } + using namespace std::chrono_literals; - static bool TestShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } -}; + RetryOptions const options{3, 1s, 2min, {}}; -RetryLogic const RetryLogic::g_retryPolicy; -} // namespace + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); -TEST(RetryPolicy, Exponential) -{ - using namespace std::chrono_literals; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1s); + } - RetryOptions const options{3, 1s, 2min, {}}; + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2s); + } - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 4s); + } + + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2s); + EXPECT_EQ(shouldRetry, false); + } } + TEST(RetryPolicy, LessThan2Retries) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 4s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({1, 1s, 2min, {}}, 1, retryAfter, 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1s); + } - EXPECT_EQ(shouldRetry, false); - } -} + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({0, 1s, 2min, {}}, 1, retryAfter, 1.0); -TEST(RetryPolicy, LessThan2Retries) -{ - using namespace std::chrono_literals; + EXPECT_EQ(shouldRetry, false); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({1, 1s, 2min, {}}, 1, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({-1, 1s, 2min, {}}, 1, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1s); + EXPECT_EQ(shouldRetry, false); + } } + TEST(RetryPolicy, NotExceedingMaxRetryDelay) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({0, 1s, 2min, {}}, 1, retryAfter, 1.0); + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, false); - } + RetryOptions const options{7, 1s, 20s, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({-1, 1s, 2min, {}}, 1, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, false); - } -} + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1s); + } -TEST(RetryPolicy, NotExceedingMaxRetryDelay) -{ - using namespace std::chrono_literals; + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); - RetryOptions const options{7, 1s, 20s, {}}; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 4s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 8s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 5, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 4s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 16s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 6, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 8s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 20s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 5, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 7, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 16s); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 20s); + } } + TEST(RetryPolicy, NotExceedingInt32Max) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 6, retryAfter, 1.0); + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 20s); - } + RetryOptions const options{35, 1s, 9999999999999s, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 7, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 31, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 20s); - } -} + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1073741824s); + } -TEST(RetryPolicy, NotExceedingInt32Max) -{ - using namespace std::chrono_literals; + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 32, retryAfter, 1.0); - RetryOptions const options{35, 1s, 9999999999999s, {}}; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2147483647s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 31, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 33, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1073741824s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2147483647s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 32, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 34, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2147483647s); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2147483647s); + } } + TEST(RetryPolicy, Jitter) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 33, retryAfter, 1.0); + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2147483647s); - } + RetryOptions const options{3, 10s, 20min, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 34, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 0.8); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2147483647s); - } -} + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 8s); + } -TEST(RetryPolicy, Jitter) -{ - using namespace std::chrono_literals; + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.3); - RetryOptions const options{3, 10s, 20min, {}}; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 13s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 0.8); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 0.8); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 8s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 16s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.3); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 13s); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 26s); + } } + TEST(RetryPolicy, JitterExtremes) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 0.8); + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 16s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 1ms, 2min, {}}, 1, retryAfter, 0.8); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.3); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 0ms); + } - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 26s); - } -} + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 2ms, 2min, {}}, 1, retryAfter, 0.8); -TEST(RetryPolicy, JitterExtremes) -{ - using namespace std::chrono_literals; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1ms); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 1ms, 2min, {}}, 1, retryAfter, 0.8); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 2, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 0ms); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 21s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 2ms, 2min, {}}, 1, retryAfter, 0.8); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 3, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1ms); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 21s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 2, retryAfter, 1.3); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnTransportFailure( + {35, 1s, 9999999999999s, {}}, 33, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 21s); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2791728741100ms); + } } + TEST(RetryPolicy, HttpStatusCode) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 3, retryAfter, 1.3); + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 21s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), + {3, 3210s, 3h, {HttpStatusCode::RequestTimeout}}, + 1, + retryAfter, + 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnTransportFailure( - {35, 1s, 9999999999999s, {}}, 33, retryAfter, 1.3); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 3210s); + } - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2791728741100ms); - } -} + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), + {3, 654s, 3h, {HttpStatusCode::Ok}}, + 1, + retryAfter, + 1.0); -TEST(RetryPolicy, HttpStatusCode) -{ - using namespace std::chrono_literals; + EXPECT_EQ(shouldRetry, false); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), - {3, 3210s, 3h, {HttpStatusCode::RequestTimeout}}, - 1, - retryAfter, - 1.0); - - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 3210s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + RawResponse(1, 1, HttpStatusCode::Ok, ""), + {3, 987s, 3h, {HttpStatusCode::Ok}}, + 1, + retryAfter, + 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), - {3, 654s, 3h, {HttpStatusCode::Ok}}, - 1, - retryAfter, - 1.0); - - EXPECT_EQ(shouldRetry, false); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 987s); + } } + TEST(RetryPolicy, RetryAfterMs) { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - RawResponse(1, 1, HttpStatusCode::Ok, ""), - {3, 987s, 3h, {HttpStatusCode::Ok}}, - 1, - retryAfter, - 1.0); - - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 987s); - } -} - -TEST(RetryPolicy, RetryAfterMs) -{ - using namespace std::chrono_literals; + using namespace std::chrono_literals; - { - RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); - response.SetHeader("rEtRy-aFtEr-mS", "1234"); + { + RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); + response.SetHeader("rEtRy-aFtEr-mS", "1234"); - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.3); + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1234ms); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1234ms); + } - { - RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); - response.SetHeader("X-mS-ReTrY-aFtEr-MS", "5678"); + { + RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); + response.SetHeader("X-mS-ReTrY-aFtEr-MS", "5678"); - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 0.8); + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 0.8); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 5678ms); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 5678ms); + } } -} - -TEST(RetryPolicy, RetryAfter) -{ - using namespace std::chrono_literals; + TEST(RetryPolicy, RetryAfter) { - RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); - response.SetHeader("rEtRy-aFtEr", "90"); + using namespace std::chrono_literals; - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.1); + { + RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); + response.SetHeader("rEtRy-aFtEr", "90"); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 90s); - } -} + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.1); -TEST(RetryPolicy, LogMessages) -{ - using Azure::Core::Diagnostics::Logger; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 90s); + } + } - struct Log + TEST(RetryPolicy, LogMessages) { - struct Entry + using Azure::Core::Diagnostics::Logger; + + struct Log { - Logger::Level Level; - std::string Message; - }; + struct Entry + { + Logger::Level Level; + std::string Message; + }; - std::vector Entries; + std::vector Entries; - Log() - { - Logger::SetLevel(Logger::Level::Informational); - Logger::SetListener([&](auto lvl, auto msg) { Entries.emplace_back(Entry{lvl, msg}); }); - } + Log() + { + Logger::SetLevel(Logger::Level::Informational); + Logger::SetListener([&](auto lvl, auto msg) { Entries.emplace_back(Entry{lvl, msg}); }); + } - ~Log() - { - Logger::SetListener(nullptr); - Logger::SetLevel(Logger::Level::Warning); - } + ~Log() + { + Logger::SetListener(nullptr); + Logger::SetLevel(Logger::Level::Warning); + } - } log; + } log; - { - using namespace std::chrono_literals; - RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::InternalServerError}}; + { + using namespace std::chrono_literals; + RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::InternalServerError}}; - auto requestNumber = 0; + auto requestNumber = 0; - std::vector> policies; - policies.emplace_back(std::make_unique(retryOptions, nullptr, nullptr)); - policies.emplace_back(std::make_unique([&]() { - ++requestNumber; + std::vector> policies; + policies.emplace_back(std::make_unique(retryOptions, nullptr, nullptr)); + policies.emplace_back(std::make_unique([&]() { + ++requestNumber; - if (requestNumber == 1) - { - throw TransportException("Cable Unplugged"); - } + if (requestNumber == 1) + { + throw TransportException("Cable Unplugged"); + } - return std::make_unique( - 1, - 1, - requestNumber == 2 ? HttpStatusCode::InternalServerError - : HttpStatusCode::ServiceUnavailable, - "Test"); - })); + return std::make_unique( + 1, + 1, + requestNumber == 2 ? HttpStatusCode::InternalServerError + : HttpStatusCode::ServiceUnavailable, + "Test"); + })); - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, Azure::Core::Context()); - } + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, Azure::Core::Context()); + } + + EXPECT_EQ(log.Entries.size(), 5); - EXPECT_EQ(log.Entries.size(), 5); + EXPECT_EQ(log.Entries[0].Level, Logger::Level::Warning); + EXPECT_EQ(log.Entries[0].Message, "HTTP Transport error: Cable Unplugged"); - EXPECT_EQ(log.Entries[0].Level, Logger::Level::Warning); - EXPECT_EQ(log.Entries[0].Message, "HTTP Transport error: Cable Unplugged"); + EXPECT_EQ(log.Entries[1].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[1].Message, "HTTP Retry attempt #1 will be made in 0ms."); - EXPECT_EQ(log.Entries[1].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[1].Message, "HTTP Retry attempt #1 will be made in 0ms."); + EXPECT_EQ(log.Entries[2].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[2].Message, "HTTP status code 500 will be retried."); - EXPECT_EQ(log.Entries[2].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[2].Message, "HTTP status code 500 will be retried."); + EXPECT_EQ(log.Entries[3].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[3].Message, "HTTP Retry attempt #2 will be made in 0ms."); - EXPECT_EQ(log.Entries[3].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[3].Message, "HTTP Retry attempt #2 will be made in 0ms."); + EXPECT_EQ(log.Entries[4].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[4].Message, "HTTP status code 503 won't be retried."); + } - EXPECT_EQ(log.Entries[4].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[4].Message, "HTTP status code 503 won't be retried."); -} +}}}}} // namespace Azure::Core::Http::Policies::Tests \ No newline at end of file From 319c503e39d685e5e15b1ea3a594d7b4439ed721 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 29 Apr 2024 09:54:00 -1000 Subject: [PATCH 2/7] Make test hooks virtual again. --- .../inc/azure/core/http/policies/policy.hpp | 21 +- .../azure-core/test/ut/retry_policy_test.cpp | 1326 ++++++++--------- 2 files changed, 669 insertions(+), 678 deletions(-) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 2e0a796d9c..e7a2702e78 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -39,13 +39,6 @@ */ extern std::shared_ptr AzureSdkGetCustomHttpTransport(); -#if defined(_azure_TESTING_BUILD) -namespace Azure { namespace Core { namespace Http { namespace Policies { namespace Tests { - class RetryPolicyTest; - class RetryLogic; -}}}}} // namespace Azure::Core::Http::Policies::Tests -#endif // _azure_TESTING_BUILD - namespace Azure { namespace Core { namespace Http { namespace Policies { struct TransportOptions; @@ -371,21 +364,21 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { * @brief HTTP retry policy. */ class RetryPolicy : public HttpPolicy { -#if defined(_azure_TESTING_BUILD) - friend class Azure::Core::Http::Policies::Tests::RetryPolicyTest; - friend class Azure::Core::Http::Policies::Tests::RetryLogic; -#endif - private: RetryOptions m_retryOptions; - bool ShouldRetryOnTransportFailure( +#if defined(_azure_TESTING_BUILD) + protected: +#else + private: +#endif + virtual bool ShouldRetryOnTransportFailure( RetryOptions const& retryOptions, int32_t attempt, std::chrono::milliseconds& retryAfter, double jitterFactor = -1) const; - bool ShouldRetryOnResponse( + virtual bool ShouldRetryOnResponse( RawResponse const& response, RetryOptions const& retryOptions, int32_t attempt, diff --git a/sdk/core/azure-core/test/ut/retry_policy_test.cpp b/sdk/core/azure-core/test/ut/retry_policy_test.cpp index 278b72eec9..b482418ce5 100644 --- a/sdk/core/azure-core/test/ut/retry_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/retry_policy_test.cpp @@ -13,825 +13,823 @@ using namespace Azure::Core::Http; using namespace Azure::Core::Http::Policies; using namespace Azure::Core::Http::Policies::_internal; -namespace Azure { namespace Core { namespace Http { namespace Policies { namespace Tests { - class TestTransportPolicy final : public HttpPolicy { - private: - std::function()> m_send; - - public: - TestTransportPolicy(std::function()> send) : m_send(send) {} - - std::unique_ptr Send( - Request&, - NextHttpPolicy, - Azure::Core::Context const&) const override - { - return m_send(); - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } - }; - - class RetryPolicyTest final : public RetryPolicy { - private: - std::function - m_shouldRetryOnTransportFailure; - - std::function< - bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> - m_shouldRetryOnResponse; - - public: - bool BaseShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } - - bool BaseShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } - - RetryPolicyTest( - RetryOptions const& retryOptions, - decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, - decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) - : RetryPolicy(retryOptions), - m_shouldRetryOnTransportFailure( - shouldRetryOnTransportFailure != nullptr // - ? shouldRetryOnTransportFailure - : static_cast( // - [this](auto options, auto attempt, auto retryAfter, auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnTransportFailure( - options, attempt, ignore, jitter); - })), - m_shouldRetryOnResponse( - shouldRetryOnResponse != nullptr // - ? shouldRetryOnResponse - : static_cast( // - [this]( - RawResponse const& response, - auto options, - auto attempt, - auto retryAfter, - auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnResponse( - response, options, attempt, ignore, jitter); - })) - { - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } +namespace { +class TestTransportPolicy final : public HttpPolicy { +private: + std::function()> m_send; + +public: + TestTransportPolicy(std::function()> send) : m_send(send) {} + + std::unique_ptr Send( + Request&, + NextHttpPolicy, + Azure::Core::Context const&) const override + { + return m_send(); + } - bool ShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); - } + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } +}; + +class RetryPolicyTest final : public RetryPolicy { +private: + std::function + m_shouldRetryOnTransportFailure; + + std::function< + bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> + m_shouldRetryOnResponse; + +public: + bool BaseShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } - bool ShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); - } - }; + bool BaseShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } - TEST(RetryPolicy, ShouldRetryOnResponse) + RetryPolicyTest( + RetryOptions const& retryOptions, + decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, + decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) + : RetryPolicy(retryOptions), + m_shouldRetryOnTransportFailure( + shouldRetryOnTransportFailure != nullptr // + ? shouldRetryOnTransportFailure + : static_cast( // + [this](auto options, auto attempt, auto retryAfter, auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnTransportFailure( + options, attempt, ignore, jitter); + })), + m_shouldRetryOnResponse( + shouldRetryOnResponse != nullptr // + ? shouldRetryOnResponse + : static_cast( // + [this]( + RawResponse const& response, + auto options, + auto attempt, + auto retryAfter, + auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnResponse( + response, options, attempt, ignore, jitter); + })) { - using namespace std::chrono_literals; - RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; + } - RawResponse const* responsePtrSent = nullptr; + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } - RawResponse const* responsePtrReceived = nullptr; - RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; - int32_t attemptReceived = -1234; - double jitterReceived = -5678; +protected: + bool ShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const override + { + return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); + } - int onTransportFailureInvoked = 0; - int onResponseInvoked = 0; + bool ShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const override + { + return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); + } +}; +} // namespace - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - }, - [&](RawResponse const& response, auto options, auto attempt, auto, auto jitter) { - ++onResponseInvoked; - responsePtrReceived = &response; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - })); - - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); - - responsePtrSent = response.get(); - - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, Azure::Core::Context()); - } +TEST(RetryPolicy, ShouldRetryOnResponse) +{ + using namespace std::chrono_literals; + RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; - EXPECT_EQ(onTransportFailureInvoked, 0); - EXPECT_EQ(onResponseInvoked, 1); + RawResponse const* responsePtrSent = nullptr; - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(responsePtrSent, responsePtrReceived); + RawResponse const* responsePtrReceived = nullptr; + RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; + int32_t attemptReceived = -1234; + double jitterReceived = -5678; - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + int onTransportFailureInvoked = 0; + int onResponseInvoked = 0; - EXPECT_EQ(attemptReceived, 1); - EXPECT_EQ(jitterReceived, -1); + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + }, + [&](RawResponse const& response, auto options, auto attempt, auto, auto jitter) { + ++onResponseInvoked; + responsePtrReceived = &response; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + })); + + policies.emplace_back(std::make_unique([&]() { + auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); + + responsePtrSent = response.get(); + + return response; + })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, Azure::Core::Context()); + } - // 3 attempts - responsePtrSent = nullptr; + EXPECT_EQ(onTransportFailureInvoked, 0); + EXPECT_EQ(onResponseInvoked, 1); - responsePtrReceived = nullptr; - retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; - attemptReceived = -1234; - jitterReceived = -5678; + EXPECT_NE(responsePtrSent, nullptr); + EXPECT_EQ(responsePtrSent, responsePtrReceived); - onTransportFailureInvoked = 0; - onResponseInvoked = 0; + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - }, - [&](RawResponse const& response, - auto options, - auto attempt, - auto retryAfter, - auto jitter) { - ++onResponseInvoked; - responsePtrReceived = &response; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - retryAfter = 1ms; - return onResponseInvoked < 3; - })); - - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); - - responsePtrSent = response.get(); - - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, Azure::Core::Context()); - } + EXPECT_EQ(attemptReceived, 1); + EXPECT_EQ(jitterReceived, -1); - EXPECT_EQ(onTransportFailureInvoked, 0); - EXPECT_EQ(onResponseInvoked, 3); + // 3 attempts + responsePtrSent = nullptr; - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(responsePtrSent, responsePtrReceived); + responsePtrReceived = nullptr; + retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; + attemptReceived = -1234; + jitterReceived = -5678; - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + onTransportFailureInvoked = 0; + onResponseInvoked = 0; - EXPECT_EQ(attemptReceived, 3); - EXPECT_EQ(jitterReceived, -1); + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + }, + [&](RawResponse const& response, auto options, auto attempt, auto retryAfter, auto jitter) { + ++onResponseInvoked; + responsePtrReceived = &response; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + retryAfter = 1ms; + return onResponseInvoked < 3; + })); + + policies.emplace_back(std::make_unique([&]() { + auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); + + responsePtrSent = response.get(); + + return response; + })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, Azure::Core::Context()); } - TEST(RetryPolicy, ShouldRetryOnTransportFailure) - { - using namespace std::chrono_literals; - RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; + EXPECT_EQ(onTransportFailureInvoked, 0); + EXPECT_EQ(onResponseInvoked, 3); - RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; - int32_t attemptReceived = -1234; - double jitterReceived = -5678; + EXPECT_NE(responsePtrSent, nullptr); + EXPECT_EQ(responsePtrSent, responsePtrReceived); - int onTransportFailureInvoked = 0; - int onResponseInvoked = 0; + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - }, - [&](auto, auto options, auto attempt, auto, auto jitter) { - ++onResponseInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return false; - })); - - policies.emplace_back(std::make_unique( - []() -> std::unique_ptr { throw TransportException("Test"); })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); - } + EXPECT_EQ(attemptReceived, 3); + EXPECT_EQ(jitterReceived, -1); +} - EXPECT_EQ(onTransportFailureInvoked, 1); - EXPECT_EQ(onResponseInvoked, 0); +TEST(RetryPolicy, ShouldRetryOnTransportFailure) +{ + using namespace std::chrono_literals; + RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::Ok}}; - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + RetryOptions retryOptionsReceived{0, 0ms, 0ms, {}}; + int32_t attemptReceived = -1234; + double jitterReceived = -5678; - EXPECT_EQ(attemptReceived, 1); - EXPECT_EQ(jitterReceived, -1); + int onTransportFailureInvoked = 0; + int onResponseInvoked = 0; - // 3 attempts - retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; - attemptReceived = -1234; - jitterReceived = -5678; + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + }, + [&](auto, auto options, auto attempt, auto, auto jitter) { + ++onResponseInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return false; + })); + + policies.emplace_back(std::make_unique( + []() -> std::unique_ptr { throw TransportException("Test"); })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); + } - onTransportFailureInvoked = 0; - onResponseInvoked = 0; + EXPECT_EQ(onTransportFailureInvoked, 1); + EXPECT_EQ(onResponseInvoked, 0); - { - std::vector> policies; - policies.emplace_back(std::make_unique( - retryOptions, - [&](auto options, auto attempt, auto, auto jitter) { - ++onTransportFailureInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - return onTransportFailureInvoked < 3; - }, - [&](auto, auto options, auto attempt, auto retryAfter, auto jitter) { - ++onResponseInvoked; - retryOptionsReceived = options; - attemptReceived = attempt; - jitterReceived = jitter; - - retryAfter = 1ms; - return false; - })); - - policies.emplace_back(std::make_unique( - []() -> std::unique_ptr { throw TransportException("Test"); })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); - } + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - EXPECT_EQ(onTransportFailureInvoked, 3); - EXPECT_EQ(onResponseInvoked, 0); + EXPECT_EQ(attemptReceived, 1); + EXPECT_EQ(jitterReceived, -1); - EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); - EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); - EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); - EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); + // 3 attempts + retryOptionsReceived = RetryOptions{0, 0ms, 0ms, {}}; + attemptReceived = -1234; + jitterReceived = -5678; - EXPECT_EQ(attemptReceived, 3); - EXPECT_EQ(jitterReceived, -1); + onTransportFailureInvoked = 0; + onResponseInvoked = 0; + + { + std::vector> policies; + policies.emplace_back(std::make_unique( + retryOptions, + [&](auto options, auto attempt, auto, auto jitter) { + ++onTransportFailureInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + return onTransportFailureInvoked < 3; + }, + [&](auto, auto options, auto attempt, auto retryAfter, auto jitter) { + ++onResponseInvoked; + retryOptionsReceived = options; + attemptReceived = attempt; + jitterReceived = jitter; + + retryAfter = 1ms; + return false; + })); + + policies.emplace_back(std::make_unique( + []() -> std::unique_ptr { throw TransportException("Test"); })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + EXPECT_THROW(pipeline.Send(request, Azure::Core::Context()), TransportException); } - class RetryLogic final : private RetryPolicy { - RetryLogic() : RetryPolicy(RetryOptions()) {} - ~RetryLogic() {} + EXPECT_EQ(onTransportFailureInvoked, 3); + EXPECT_EQ(onResponseInvoked, 0); - static RetryLogic const g_retryPolicy; + EXPECT_EQ(retryOptionsReceived.MaxRetries, retryOptions.MaxRetries); + EXPECT_EQ(retryOptionsReceived.RetryDelay, retryOptions.RetryDelay); + EXPECT_EQ(retryOptionsReceived.MaxRetryDelay, retryOptions.MaxRetryDelay); + EXPECT_EQ(retryOptionsReceived.StatusCodes, retryOptions.StatusCodes); - public: - static bool TestShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } + EXPECT_EQ(attemptReceived, 3); + EXPECT_EQ(jitterReceived, -1); +} - static bool TestShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } - }; +namespace { +class RetryLogic final : private RetryPolicy { + RetryLogic() : RetryPolicy(RetryOptions()) {} + ~RetryLogic() {} - RetryLogic const RetryLogic::g_retryPolicy; + static RetryLogic const g_retryPolicy; - TEST(RetryPolicy, Exponential) +public: + static bool TestShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) { - using namespace std::chrono_literals; - - RetryOptions const options{3, 1s, 2min, {}}; + return g_retryPolicy.ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); + static bool TestShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } +}; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1s); - } +RetryLogic const RetryLogic::g_retryPolicy; +} // namespace - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); +TEST(RetryPolicy, Exponential) +{ + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2s); - } + RetryOptions const options{3, 1s, 2min, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 4s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, false); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2s); } - TEST(RetryPolicy, LessThan2Retries) { - using namespace std::chrono_literals; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({1, 1s, 2min, {}}, 1, retryAfter, 1.0); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 4s); + } - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({0, 1s, 2min, {}}, 1, retryAfter, 1.0); + EXPECT_EQ(shouldRetry, false); + } +} - EXPECT_EQ(shouldRetry, false); - } +TEST(RetryPolicy, LessThan2Retries) +{ + using namespace std::chrono_literals; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({-1, 1s, 2min, {}}, 1, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({1, 1s, 2min, {}}, 1, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, false); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1s); } - TEST(RetryPolicy, NotExceedingMaxRetryDelay) { - using namespace std::chrono_literals; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({0, 1s, 2min, {}}, 1, retryAfter, 1.0); - RetryOptions const options{7, 1s, 20s, {}}; + EXPECT_EQ(shouldRetry, false); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({-1, 1s, 2min, {}}, 1, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1s); - } + EXPECT_EQ(shouldRetry, false); + } +} - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); +TEST(RetryPolicy, NotExceedingMaxRetryDelay) +{ + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2s); - } + RetryOptions const options{7, 1s, 20s, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 4s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 8s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 5, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 3, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 16s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 4s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 6, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 4, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 20s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 8s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 7, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 5, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 20s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 16s); } - TEST(RetryPolicy, NotExceedingInt32Max) { - using namespace std::chrono_literals; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 6, retryAfter, 1.0); - RetryOptions const options{35, 1s, 9999999999999s, {}}; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 20s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 31, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 7, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1073741824s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 20s); + } +} - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 32, retryAfter, 1.0); +TEST(RetryPolicy, NotExceedingInt32Max) +{ + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2147483647s); - } + RetryOptions const options{35, 1s, 9999999999999s, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 33, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 31, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2147483647s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1073741824s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 34, retryAfter, 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 32, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2147483647s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2147483647s); } - TEST(RetryPolicy, Jitter) { - using namespace std::chrono_literals; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 33, retryAfter, 1.0); - RetryOptions const options{3, 10s, 20min, {}}; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2147483647s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 0.8); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 34, retryAfter, 1.0); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 8s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2147483647s); + } +} - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.3); +TEST(RetryPolicy, Jitter) +{ + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 13s); - } + RetryOptions const options{3, 10s, 20min, {}}; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 0.8); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 0.8); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 16s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 8s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.3); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 1, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 26s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 13s); } - TEST(RetryPolicy, JitterExtremes) { - using namespace std::chrono_literals; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 0.8); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 1ms, 2min, {}}, 1, retryAfter, 0.8); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 16s); + } - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 0ms); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure(options, 2, retryAfter, 1.3); - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 2ms, 2min, {}}, 1, retryAfter, 0.8); + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 26s); + } +} - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1ms); - } +TEST(RetryPolicy, JitterExtremes) +{ + using namespace std::chrono_literals; - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 2, retryAfter, 1.3); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 1ms, 2min, {}}, 1, retryAfter, 0.8); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 21s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 0ms); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry - = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 3, retryAfter, 1.3); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 2ms, 2min, {}}, 1, retryAfter, 0.8); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 21s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1ms); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnTransportFailure( - {35, 1s, 9999999999999s, {}}, 33, retryAfter, 1.3); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 2, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 2791728741100ms); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 21s); } - TEST(RetryPolicy, HttpStatusCode) { - using namespace std::chrono_literals; - - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), - {3, 3210s, 3h, {HttpStatusCode::RequestTimeout}}, - 1, - retryAfter, - 1.0); + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry + = RetryLogic::TestShouldRetryOnTransportFailure({3, 10s, 21s, {}}, 3, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 3210s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 21s); + } - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), - {3, 654s, 3h, {HttpStatusCode::Ok}}, - 1, - retryAfter, - 1.0); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnTransportFailure( + {35, 1s, 9999999999999s, {}}, 33, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, false); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 2791728741100ms); + } +} - { - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - RawResponse(1, 1, HttpStatusCode::Ok, ""), - {3, 987s, 3h, {HttpStatusCode::Ok}}, - 1, - retryAfter, - 1.0); +TEST(RetryPolicy, HttpStatusCode) +{ + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 987s); - } + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), + {3, 3210s, 3h, {HttpStatusCode::RequestTimeout}}, + 1, + retryAfter, + 1.0); + + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 3210s); } - TEST(RetryPolicy, RetryAfterMs) { - using namespace std::chrono_literals; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + RawResponse(1, 1, HttpStatusCode::RequestTimeout, ""), + {3, 654s, 3h, {HttpStatusCode::Ok}}, + 1, + retryAfter, + 1.0); + + EXPECT_EQ(shouldRetry, false); + } - { - RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); - response.SetHeader("rEtRy-aFtEr-mS", "1234"); + { + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + RawResponse(1, 1, HttpStatusCode::Ok, ""), + {3, 987s, 3h, {HttpStatusCode::Ok}}, + 1, + retryAfter, + 1.0); + + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 987s); + } +} - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.3); +TEST(RetryPolicy, RetryAfterMs) +{ + using namespace std::chrono_literals; - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 1234ms); - } - - { - RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); - response.SetHeader("X-mS-ReTrY-aFtEr-MS", "5678"); + { + RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); + response.SetHeader("rEtRy-aFtEr-mS", "1234"); - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 0.8); + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.3); - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 5678ms); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 1234ms); } - TEST(RetryPolicy, RetryAfter) { - using namespace std::chrono_literals; + RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); + response.SetHeader("X-mS-ReTrY-aFtEr-MS", "5678"); - { - RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); - response.SetHeader("rEtRy-aFtEr", "90"); + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 0.8); - std::chrono::milliseconds retryAfter{}; - bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( - response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.1); - - EXPECT_EQ(shouldRetry, true); - EXPECT_EQ(retryAfter, 90s); - } + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 5678ms); } +} + +TEST(RetryPolicy, RetryAfter) +{ + using namespace std::chrono_literals; - TEST(RetryPolicy, LogMessages) { - using Azure::Core::Diagnostics::Logger; + RawResponse response(1, 1, HttpStatusCode::RequestTimeout, ""); + response.SetHeader("rEtRy-aFtEr", "90"); - struct Log - { - struct Entry - { - Logger::Level Level; - std::string Message; - }; + std::chrono::milliseconds retryAfter{}; + bool const shouldRetry = RetryLogic::TestShouldRetryOnResponse( + response, {3, 1s, 2min, {HttpStatusCode::RequestTimeout}}, 1, retryAfter, 1.1); - std::vector Entries; + EXPECT_EQ(shouldRetry, true); + EXPECT_EQ(retryAfter, 90s); + } +} - Log() - { - Logger::SetLevel(Logger::Level::Informational); - Logger::SetListener([&](auto lvl, auto msg) { Entries.emplace_back(Entry{lvl, msg}); }); - } +TEST(RetryPolicy, LogMessages) +{ + using Azure::Core::Diagnostics::Logger; - ~Log() - { - Logger::SetListener(nullptr); - Logger::SetLevel(Logger::Level::Warning); - } + struct Log + { + struct Entry + { + Logger::Level Level; + std::string Message; + }; + + std::vector Entries; - } log; + Log() + { + Logger::SetLevel(Logger::Level::Informational); + Logger::SetListener([&](auto lvl, auto msg) { Entries.emplace_back(Entry{lvl, msg}); }); + } + ~Log() { - using namespace std::chrono_literals; - RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::InternalServerError}}; + Logger::SetListener(nullptr); + Logger::SetLevel(Logger::Level::Warning); + } - auto requestNumber = 0; + } log; - std::vector> policies; - policies.emplace_back(std::make_unique(retryOptions, nullptr, nullptr)); - policies.emplace_back(std::make_unique([&]() { - ++requestNumber; + { + using namespace std::chrono_literals; + RetryOptions const retryOptions{5, 10s, 5min, {HttpStatusCode::InternalServerError}}; - if (requestNumber == 1) - { - throw TransportException("Cable Unplugged"); - } + auto requestNumber = 0; - return std::make_unique( - 1, - 1, - requestNumber == 2 ? HttpStatusCode::InternalServerError - : HttpStatusCode::ServiceUnavailable, - "Test"); - })); + std::vector> policies; + policies.emplace_back(std::make_unique(retryOptions, nullptr, nullptr)); + policies.emplace_back(std::make_unique([&]() { + ++requestNumber; - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + if (requestNumber == 1) + { + throw TransportException("Cable Unplugged"); + } - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, Azure::Core::Context()); - } + return std::make_unique( + 1, + 1, + requestNumber == 2 ? HttpStatusCode::InternalServerError + : HttpStatusCode::ServiceUnavailable, + "Test"); + })); - EXPECT_EQ(log.Entries.size(), 5); + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - EXPECT_EQ(log.Entries[0].Level, Logger::Level::Warning); - EXPECT_EQ(log.Entries[0].Message, "HTTP Transport error: Cable Unplugged"); + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, Azure::Core::Context()); + } - EXPECT_EQ(log.Entries[1].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[1].Message, "HTTP Retry attempt #1 will be made in 0ms."); + EXPECT_EQ(log.Entries.size(), 5); - EXPECT_EQ(log.Entries[2].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[2].Message, "HTTP status code 500 will be retried."); + EXPECT_EQ(log.Entries[0].Level, Logger::Level::Warning); + EXPECT_EQ(log.Entries[0].Message, "HTTP Transport error: Cable Unplugged"); - EXPECT_EQ(log.Entries[3].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[3].Message, "HTTP Retry attempt #2 will be made in 0ms."); + EXPECT_EQ(log.Entries[1].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[1].Message, "HTTP Retry attempt #1 will be made in 0ms."); - EXPECT_EQ(log.Entries[4].Level, Logger::Level::Informational); - EXPECT_EQ(log.Entries[4].Message, "HTTP status code 503 won't be retried."); - } + EXPECT_EQ(log.Entries[2].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[2].Message, "HTTP status code 500 will be retried."); + + EXPECT_EQ(log.Entries[3].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[3].Message, "HTTP Retry attempt #2 will be made in 0ms."); -}}}}} // namespace Azure::Core::Http::Policies::Tests \ No newline at end of file + EXPECT_EQ(log.Entries[4].Level, Logger::Level::Informational); + EXPECT_EQ(log.Entries[4].Message, "HTTP status code 503 won't be retried."); +} From a0905b508cdf33dfc06aa8601016cf94296b7f1a Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 29 Apr 2024 12:10:49 -1000 Subject: [PATCH 3/7] Add unit test for ShouldRetry. --- .../azure-core/test/ut/retry_policy_test.cpp | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/sdk/core/azure-core/test/ut/retry_policy_test.cpp b/sdk/core/azure-core/test/ut/retry_policy_test.cpp index b482418ce5..2679d78e0a 100644 --- a/sdk/core/azure-core/test/ut/retry_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/retry_policy_test.cpp @@ -124,8 +124,89 @@ class RetryPolicyTest final : public RetryPolicy { return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); } }; + +class RetryPolicyTest_CustomRetry_False final : public RetryPolicy { +public: + RetryPolicyTest_CustomRetry_False(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) {} + + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } + +protected: + bool ShouldRetry(std::unique_ptr const& response, RetryOptions const& retryOptions) + const override + { + (void)response; + (void)retryOptions; + return false; + } +}; } // namespace +TEST(RetryPolicy, ShouldRetry) +{ + using namespace std::chrono_literals; + RetryOptions const retryOptions{3, 10ms, 60s, {HttpStatusCode::Ok}}; + + // The default ShouldRetry implementation is to always return true, + // which means we will retry up to the retry count in the options. + { + Azure::Core::Context context = Azure::Core::Context::ApplicationContext; + auto policy = RetryPolicy(retryOptions); + + RawResponse const* responsePtrSent = nullptr; + int32_t retryCounter = -1; + + std::vector> policies; + policies.emplace_back(std::make_unique(policy)); + policies.emplace_back(std::make_unique([&]() { + auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); + + responsePtrSent = response.get(); + retryCounter++; + return response; + })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, context); + + EXPECT_NE(responsePtrSent, nullptr); + EXPECT_EQ(retryCounter, 3); + } + + // Overriding ShouldRetry to return false will mean we won't retry. + { + Azure::Core::Context context = Azure::Core::Context(); + auto policy = RetryPolicyTest_CustomRetry_False(retryOptions); + + RawResponse const* responsePtrSent = nullptr; + int32_t retryCounter = -1; + + std::vector> policies; + policies.emplace_back(std::make_unique(policy)); + + policies.emplace_back(std::make_unique([&]() { + auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); + + responsePtrSent = response.get(); + retryCounter++; + return response; + })); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + + Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); + pipeline.Send(request, context); + + EXPECT_NE(responsePtrSent, nullptr); + EXPECT_EQ(retryCounter, 0); + } +} + TEST(RetryPolicy, ShouldRetryOnResponse) { using namespace std::chrono_literals; From bfec526c28a37ceb47972a557f99b24f25828eb7 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 29 Apr 2024 12:22:19 -1000 Subject: [PATCH 4/7] Fix typo. --- sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index e7a2702e78..b54d2a0391 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -431,7 +431,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { * * @param response An HTTP response returned corresponding to the request sent by the policy. * @param retryOptions The set of options provided to the RetryPolicy. - * @return Whether or not the HTTP request should be sent againg through the pipeline. + * @return Whether or not the HTTP request should be sent again through the pipeline. */ virtual bool ShouldRetry( std::unique_ptr const& response, From 1030fbeab5dfebca2a33a5c611101f2d1e699ec2 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 13 May 2024 16:41:09 -0700 Subject: [PATCH 5/7] Address PR feedback. --- sdk/core/azure-core/src/http/retry_policy.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index c6533d0395..29eeae8e4a 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -118,12 +118,8 @@ int32_t RetryPolicy::GetRetryCount(Context const& context) return *ptr; } -bool RetryPolicy::ShouldRetry( - std::unique_ptr const& response, - RetryOptions const& retryOptions) const +bool RetryPolicy::ShouldRetry(std::unique_ptr const&, RetryOptions const&) const { - (void)response; - (void)retryOptions; return true; } From 36ff9da91824df8dd8cf2a804d4775e9e03d1ac9 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 13 May 2024 16:43:35 -0700 Subject: [PATCH 6/7] Revert making the mock'd functions private for non-test builds. --- .../inc/azure/core/http/policies/policy.hpp | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index b54d2a0391..54f3d7208e 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -367,24 +367,6 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { private: RetryOptions m_retryOptions; -#if defined(_azure_TESTING_BUILD) - protected: -#else - private: -#endif - virtual bool ShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor = -1) const; - - virtual bool ShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor = -1) const; - public: /** * Constructs HTTP retry policy with the provided #Azure::Core::Http::Policies::RetryOptions. @@ -417,6 +399,18 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { static int32_t GetRetryCount(Context const& context); protected: + virtual bool ShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor = -1) const; + + virtual bool ShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor = -1) const; /** * @brief Overriding this method customizes the logic of when the RetryPolicy will re-attempt * a request, based on the returned HTTP response. From 3294a2708566e0338eed5313cac028438aa7de01 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 13 May 2024 16:44:56 -0700 Subject: [PATCH 7/7] Add new line. --- sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 54f3d7208e..e626dbc48b 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -411,6 +411,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { int32_t attempt, std::chrono::milliseconds& retryAfter, double jitterFactor = -1) const; + /** * @brief Overriding this method customizes the logic of when the RetryPolicy will re-attempt * a request, based on the returned HTTP response.