Retry unsuccessful HTTP requests on 500 and 503 errors by default. (#623)

The error code 500 Internal Error indicates that a server is unable
to handle the request at that time. The error code 503 Slow Down
typically means that the request rate to the server bucket is
too high. Hence, it makes sense to resolve such errors by retrying
the request.

Related-To: OLPEDGE-1398

Signed-off-by: Kirill Zhuchkov <[email protected]>

Author: r0busta

olp-cpp-sdk-core/src/client/OlpClientSettings.cpp

@@ -19,14 +19,23 @@
 
 #include "olp/core/client/OlpClientSettings.h"
 #include "olp/core/client/HttpResponse.h"
+#include "olp/core/http/HttpStatusCode.h"
 
 namespace olp {
 namespace client {
 unsigned int DefaultBackdownPolicy(unsigned int milliseconds) {
   return milliseconds;
 }
 
-bool DefaultRetryCondition(const HttpResponse&) { return false; }
+bool DefaultRetryCondition(const HttpResponse& response) {
+  switch (response.status) {
+    case http::HttpStatusCode::INTERNAL_SERVER_ERROR:
+    case http::HttpStatusCode::SERVICE_UNAVAILABLE:
+      return true;
+    default:
+      return false;
+  }
+}
 
 }  // namespace client
 }  // namespace olp

olp-cpp-sdk-core/tests/client/OlpClientTest.cpp

@@ -23,6 +23,7 @@
 #include <chrono>
 #include <future>
 #include <string>
+#include <queue>
 
 #include <olp/core/client/ApiError.h>
 #include <olp/core/client/OlpClient.h>
@@ -216,17 +217,29 @@ TEST_P(OlpClientTest, ZeroAttempts) {
 }
 
 TEST_P(OlpClientTest, DefaultRetryCondition) {
-  client_settings_.retry_settings.max_attempts = 6;
+  client_settings_.retry_settings.max_attempts = 3;
 
   auto network = std::make_shared<NetworkMock>();
   client_settings_.network_request_handler = network;
+
+  auto attempt_statuses = std::queue<int>{{500, 503, 200}};
   EXPECT_CALL(*network, Send(_, _, _, _, _))
-      .WillOnce([&](olp::http::NetworkRequest request,
-                    olp::http::Network::Payload payload,
-                    olp::http::Network::Callback callback,
-                    olp::http::Network::HeaderCallback header_callback,
-                    olp::http::Network::DataCallback data_callback) {
-        callback(olp::http::NetworkResponse().WithStatus(429));
+      .Times(attempt_statuses.size())
+      .WillRepeatedly([&attempt_statuses](
+                          olp::http::NetworkRequest request,
+                          olp::http::Network::Payload payload,
+                          olp::http::Network::Callback callback,
+                          olp::http::Network::HeaderCallback header_callback,
+                          olp::http::Network::DataCallback data_callback) {
+        if (attempt_statuses.empty()) {
+          ADD_FAILURE_AT(__FILE__, __LINE__) << "Unexpected retry attempt";
+          return olp::http::SendOutcome(olp::http::ErrorCode::UNKNOWN_ERROR);
+        }
+
+        auto status = attempt_statuses.front();
+        attempt_statuses.pop();
+        callback(olp::http::NetworkResponse().WithStatus(status));
+
         return olp::http::SendOutcome(olp::http::RequestId(5));
       });
 
@@ -237,7 +250,7 @@ TEST_P(OlpClientTest, DefaultRetryCondition) {
       std::multimap<std::string, std::string>(),
       std::multimap<std::string, std::string>(), nullptr, std::string());
 
-  ASSERT_EQ(429, response.status);
+  ASSERT_EQ(200, response.status);
 }
 
 TEST_P(OlpClientTest, RetryCondition) {