Skip to content

feat: Support handling invalid URLs for asio requests. #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 1, 2023
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: 7 additions & 0 deletions apps/hello-cpp/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ int main() {
.value(),
ContextBuilder().kind("user", "ryan").build());

LD_LOG(logger, LogLevel::kInfo)
<< "Initial Status: " << client.DataSourceStatus().Status();

client.DataSourceStatus().OnDataSourceStatusChange([&logger](auto status) {
LD_LOG(logger, LogLevel::kInfo) << "Got status: " << status;
});

client.WaitForReadySync(std::chrono::seconds(30));

auto value = client.BoolVariation("my-boolean-flag", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,4 +238,10 @@ std::ostream& operator<<(std::ostream& out,
std::ostream& operator<<(std::ostream& out,
DataSourceStatus::ErrorInfo::ErrorKind const& kind);

std::ostream& operator<<(std::ostream& out,
DataSourceStatus const& status);

std::ostream& operator<<(std::ostream& out,
DataSourceStatus::ErrorInfo const& error);

} // namespace launchdarkly::client_side::data_sources
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class DataSourceStatusManager : public IDataSourceStatusProvider {

boost::signals2::signal<void(data_sources::DataSourceStatus status)>
data_source_status_signal_;
mutable std::mutex status_mutex_;
mutable std::recursive_mutex status_mutex_;
bool UpdateState(DataSourceStatus::DataSourceState const& requested_state);
};

Expand Down
23 changes: 23 additions & 0 deletions libs/client-sdk/src/data_sources/data_source_status.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@

#include <iomanip>
#include <utility>

#include "launchdarkly/client_side/data_sources/data_source_status.hpp"

namespace launchdarkly::client_side::data_sources {

DataSourceStatus::ErrorInfo::ErrorKind DataSourceStatus::ErrorInfo::Kind()
Expand Down Expand Up @@ -92,4 +94,25 @@ std::ostream& operator<<(std::ostream& out,
return out;
}

std::ostream& operator<<(std::ostream& out, DataSourceStatus const& status) {
std::time_t as_time_t =
std::chrono::system_clock::to_time_t(status.StateSince());
out << "Status(" << status.State() << ", Since("
<< std::put_time(std::gmtime(&as_time_t), "%Y-%m-%d %H:%M:%S") << ")";
if (status.LastError()) {
out << ", " << status.LastError().value();
}
out << ")";
return out;
}

std::ostream& operator<<(std::ostream& out,
DataSourceStatus::ErrorInfo const& error) {
std::time_t as_time_t = std::chrono::system_clock::to_time_t(error.Time());
out << "Error(" << error.Kind() << ", " << error.Message()
<< ", StatusCode(" << error.StatusCode() << "), Since("
<< std::put_time(std::gmtime(&as_time_t), "%Y-%m-%d %H:%M:%S") << "))";
return out;
}

} // namespace launchdarkly::client_side::data_sources
2 changes: 1 addition & 1 deletion libs/client-sdk/src/data_sources/polling_data_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ void PollingDataSource::DoPoll() {
DataSourceStatus::ErrorInfo::ErrorKind::kNetworkError,
res.ErrorMessage() ? *res.ErrorMessage() : "unknown error");
LD_LOG(logger_, LogLevel::kWarn)
<< "Polling for feature flag updates failed:"
<< "Polling for feature flag updates failed: "
<< (res.ErrorMessage() ? *res.ErrorMessage() : "unknown error");
} else if (res.Status() == 200) {
data_source_handler_.HandleMessage("put", res.Body().value());
Expand Down
29 changes: 24 additions & 5 deletions libs/common/include/network/detail/asio_requester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ class
void Fail(beast::error_code ec, char const* what) {
// TODO: Is it safe to cancel this if it has already failed?
DoClose();
handler_(HttpResult(std::string(what) + ": " + ec.message()));
std::optional<std::string> error_string =
std::string(what) + ": " + ec.message();
handler_(HttpResult(error_string));
}

void DoResolve() {
Expand All @@ -122,8 +124,9 @@ class
}

void OnResolve(beast::error_code ec, tcp::resolver::results_type results) {
if (ec)
if (ec) {
return Fail(ec, "resolve");
}

beast::get_lowest_layer(GetDerived().Stream())
.expires_after(connect_timeout_);
Expand Down Expand Up @@ -207,7 +210,8 @@ class
headers.insert_or_assign(field.name_string(), field.value());
}
auto result = HttpResult(parser_.get().result_int(),
parser_.get().body(), std::move(headers));
std::make_optional(parser_.get().body()),
std::move(headers));
return result;
}

Expand Down Expand Up @@ -283,8 +287,9 @@ class EncryptedClient : public Session<EncryptedClient>,

DoClose();
// TODO: Should this be treated as a terminal error for the request.
handler_(HttpResult("failed to set TLS host name extension: " +
ec.message()));
std::optional<std::string> error_string =
"failed to set TLS host name extension: " + ec.message();
handler_(HttpResult(error_string));
return;
}

Expand Down Expand Up @@ -364,6 +369,7 @@ class AsioRequester {
auto Request(HttpRequest request, CompletionToken&& token) {
// TODO: Clang-tidy wants to pass the request by reference, but I am not
// confident that lifetime would make sense.

namespace asio = boost::asio;
namespace system = boost::system;

Expand All @@ -375,6 +381,19 @@ class AsioRequester {
Result result(handler);

auto strand = net::make_strand(ctx_);

// The request is invalid and cannot be made, so produce an error
// result.
if (!request.Valid()) {
boost::asio::post(
strand, [strand, handler, request, this]() mutable {
std::optional<std::string> error_string =
"The request was malformed and could not be made.";
handler(HttpResult(error_string));
});
return;
}

boost::asio::post(strand, [strand, handler, request, this]() mutable {
auto beast_request = MakeBeastRequest(request);

Expand Down
10 changes: 10 additions & 0 deletions libs/common/include/network/detail/http_requester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <cstdint>
#include <future>
#include <map>
#include <optional>
#include <ostream>
#include <string>

Expand Down Expand Up @@ -92,6 +93,14 @@ class HttpRequest {
std::string const& Path() const;
bool Https() const;

/**
* Indicates if a request is valid. Meaning that it has correctly formed
* data that can be used to make an http request.
*
* @return True if the request is valid.
*/
bool Valid() const;

HttpRequest(std::string const& url,
HttpMethod method,
config::detail::built::HttpProperties properties,
Expand All @@ -115,6 +124,7 @@ class HttpRequest {
std::string path_;
std::map<std::string, std::string> params_;
bool is_https_;
bool valid_;
};

bool IsRecoverableStatus(HttpResult::StatusCode status);
Expand Down
5 changes: 4 additions & 1 deletion libs/common/src/data/evaluation_result.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "data/evaluation_result.hpp"

#include <chrono>
#include <iomanip>
#include <utility>

namespace launchdarkly {

Expand Down Expand Up @@ -53,7 +55,8 @@ std::ostream& operator<<(std::ostream& out, EvaluationResult const& result) {
if (result.debug_events_until_date_.has_value()) {
std::time_t as_time_t = std::chrono::system_clock::to_time_t(
result.debug_events_until_date_.value());
out << " debugEventsUntilDate: " << std::ctime(&as_time_t);
out << " debugEventsUntilDate: "
<< std::put_time(std::gmtime(&as_time_t), "%Y-%m-%d %H:%M:%S");
}
out << " detail: " << result.detail_;
out << "}";
Expand Down
12 changes: 12 additions & 0 deletions libs/common/src/network/http_requester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ HttpRequest::HttpRequest(std::string const& url,
body_(std::move(body)) {
auto uri_components = boost::urls::parse_uri(url);

// If the URI cannot be parsed, then the request is not valid.
if (!uri_components) {
valid_ = false;
return;
}

host_ = uri_components->host();
// For a boost beast request we need the query string in the path.
path_ = uri_components->path() + "?" + uri_components->query();
Expand All @@ -79,6 +85,7 @@ HttpRequest::HttpRequest(std::string const& url,
} else {
port_ = is_https_ ? "443" : "80";
}
valid_ = true;
}

HttpRequest::HttpRequest(HttpRequest& base_request,
Expand All @@ -90,6 +97,7 @@ HttpRequest::HttpRequest(HttpRequest& base_request,
host_ = base_request.host_;
port_ = base_request.port_;
is_https_ = base_request.is_https_;
valid_ = base_request.valid_;
}

std::string const& HttpRequest::Port() const {
Expand All @@ -99,6 +107,10 @@ bool HttpRequest::Https() const {
return is_https_;
}

bool HttpRequest::Valid() const {
return valid_;
}

bool IsRecoverableStatus(HttpResult::StatusCode status) {
return status < 400 || status > 499 || status == 400 || status == 408 ||
status == 429;
Expand Down