Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions olp-cpp-sdk-core/include/olp/core/client/RetrySettings.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2021-2024 HERE Europe B.V.
* Copyright (C) 2021-2026 HERE Europe B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -71,8 +71,9 @@ struct CORE_API RetrySettings {
*
* The default value is 60.
*
* @note Connection or data transfer will be interrupted after specified
* period of time ignoring `connection_timeout` and `transfer_timeout` values.
* @note Connection, data transfer, or retry attempts will be interrupted
* after specified period of time ignoring `connection_timeout` and
* `transfer_timeout` values.
*/
int timeout = 60;

Expand Down
38 changes: 34 additions & 4 deletions olp-cpp-sdk-core/src/client/OlpClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include <cctype>
#include <chrono>
#include <climits>
#include <future>
#ifdef OLP_SDK_NETWORK_IOS_BACKGROUND_DOWNLOAD
#include <list>
Expand Down Expand Up @@ -182,6 +183,32 @@
});
}

std::chrono::seconds GetRetryAfterWaitTime(const HttpResponse& response) {
for (const auto& header : response.GetHeaders()) {
if (!CaseInsensitiveCompare(header.first, "Retry-After")) {
continue;

Check warning on line 189 in olp-cpp-sdk-core/src/client/OlpClient.cpp

View check run for this annotation

Codecov / codecov/patch

olp-cpp-sdk-core/src/client/OlpClient.cpp#L189

Added line #L189 was not covered by tests
}

if (header.second.empty()) {
return std::chrono::seconds::zero();
}

using Rep = std::chrono::seconds::rep;
const Rep seconds_from_header =
std::strtol(header.second.c_str(), nullptr, 10);

if (seconds_from_header == std::numeric_limits<Rep>::max() ||
seconds_from_header == std::numeric_limits<Rep>::min()) {
return std::chrono::seconds::zero();

Check warning on line 202 in olp-cpp-sdk-core/src/client/OlpClient.cpp

View check run for this annotation

Codecov / codecov/patch

olp-cpp-sdk-core/src/client/OlpClient.cpp#L202

Added line #L202 was not covered by tests
}

// In case of other issues with conversion - return zero seconds
return std::chrono::seconds(seconds_from_header);
}

return std::chrono::seconds::zero();
}

RequestSettingsPtr GetRequestSettings(const RetrySettings& retry_settings) {
return std::make_shared<RequestSettings>(
retry_settings.initial_backdown_period, retry_settings.timeout);
Expand Down Expand Up @@ -286,9 +313,10 @@

// TODO: Do not block thread but instead implement an event queue that will
// trigger next retry once the time expired!
const auto actual_wait_time =
std::min(settings->current_backdown_period,
settings->max_wait_time - settings->accumulated_wait_time);
auto wait_time = settings->current_backdown_period;
wait_time += GetRetryAfterWaitTime(response);
const auto actual_wait_time = std::min(
wait_time, settings->max_wait_time - settings->accumulated_wait_time);
Comment thread
mykolamalik marked this conversation as resolved.
std::this_thread::sleep_for(actual_wait_time);

settings->accumulated_wait_time += actual_wait_time;
Expand Down Expand Up @@ -843,8 +871,10 @@
}

// do the periodical sleep and check for cancellation status in between.
auto wait_time = backdown_period;
wait_time += GetRetryAfterWaitTime(response);
auto duration_to_sleep =
std::min(backdown_period, max_wait_time - accumulated_wait_time);
std::min(wait_time, max_wait_time - accumulated_wait_time);
accumulated_wait_time += duration_to_sleep;

while (duration_to_sleep.count() > 0 && !context.IsCancelled()) {
Expand Down
110 changes: 110 additions & 0 deletions olp-cpp-sdk-core/tests/client/OlpClientTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,116 @@ TEST_P(OlpClientTest, RetryWithExponentialBackdownStrategy) {
testing::Mock::VerifyAndClearExpectations(network.get());
}

TEST_P(OlpClientTest, RetryAfterHeaderDelaysRetry) {
constexpr auto kRetryAfterDelay = std::chrono::seconds(1);
auto network = network_;
auto& retry_settings = client_settings_.retry_settings;
retry_settings.max_attempts = 1;
retry_settings.initial_backdown_period = 0;
retry_settings.backdown_strategy = [](std::chrono::milliseconds, size_t) {
return std::chrono::milliseconds::zero();
};

olp::client::OlpClient client(client_settings_, kEmptyBaseUrl);
std::vector<std::chrono::steady_clock::time_point> timestamps;
std::vector<std::future<void>> futures;
olp::http::RequestId request_id = 5;
int attempt = 0;

EXPECT_CALL(*network, Send(_, _, _, _, _))
.Times(2)
.WillRepeatedly([&](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*/) {
timestamps.push_back(std::chrono::steady_clock::now());
const auto current_request_id = request_id++;
const bool retryable_response = attempt++ == 0;
futures.emplace_back(std::async(std::launch::async, [=]() {
std::this_thread::sleep_for(kCallbackSleepTime);
if (retryable_response) {
header_callback("retry-after", "1");
} else {
header_callback("not-retry-after", "0");
}
callback(olp::http::NetworkResponse()
.WithStatus(retryable_response
? http::HttpStatusCode::TOO_MANY_REQUESTS
: http::HttpStatusCode::OK)
.WithRequestId(current_request_id));
}));
return olp::http::SendOutcome(current_request_id);
});

auto call_wrapper = MakeCallWrapper(client);
const auto response =
call_wrapper->CallApi({}, "GET", {}, {}, {}, nullptr, {});

for (auto& future : futures) {
future.wait();
}

ASSERT_EQ(http::HttpStatusCode::OK, response.GetStatus());
ASSERT_EQ(2u, timestamps.size());
EXPECT_GE(timestamps[1] - timestamps[0], kRetryAfterDelay);
testing::Mock::VerifyAndClearExpectations(network.get());
}

TEST_P(OlpClientTest, InvalidRetryAfterHeaderUsesBackdownStrategy) {
constexpr auto kBackdownDelay = std::chrono::milliseconds(100);
auto network = network_;
auto& retry_settings = client_settings_.retry_settings;
retry_settings.max_attempts = 1;
retry_settings.initial_backdown_period = kBackdownDelay.count();
retry_settings.backdown_strategy = [](std::chrono::milliseconds, size_t) {
return std::chrono::milliseconds::zero();
};

olp::client::OlpClient client(client_settings_, kEmptyBaseUrl);
std::vector<std::chrono::steady_clock::time_point> timestamps;
std::vector<std::future<void>> futures;
olp::http::RequestId request_id = 5;
int attempt = 0;

EXPECT_CALL(*network, Send(_, _, _, _, _))
.Times(2)
.WillRepeatedly([&](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*/) {
timestamps.push_back(std::chrono::steady_clock::now());
const auto current_request_id = request_id++;
const bool retryable_response = attempt++ == 0;
futures.emplace_back(std::async(std::launch::async, [=]() {
std::this_thread::sleep_for(kCallbackSleepTime);
if (retryable_response) {
header_callback("Retry-After", "not-a-delay");
}
callback(olp::http::NetworkResponse()
.WithStatus(retryable_response
? http::HttpStatusCode::TOO_MANY_REQUESTS
: http::HttpStatusCode::OK)
.WithRequestId(current_request_id));
}));
return olp::http::SendOutcome(current_request_id);
});

auto call_wrapper = MakeCallWrapper(client);
const auto response =
call_wrapper->CallApi({}, "GET", {}, {}, {}, nullptr, {});

for (auto& future : futures) {
future.wait();
}

ASSERT_EQ(http::HttpStatusCode::OK, response.GetStatus());
ASSERT_EQ(2u, timestamps.size());
EXPECT_GE(timestamps[1] - timestamps[0], kBackdownDelay);
testing::Mock::VerifyAndClearExpectations(network.get());
}

TEST_P(OlpClientTest, RetryTimeout) {
const size_t kMaxRetries = 3;
const size_t kSuccessfulAttempt = kMaxRetries + 1;
Expand Down
Loading