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

Author: kinyoklion

apps/hello-cpp/main.cpp

@@ -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);

libs/client-sdk/include/launchdarkly/client_side/data_sources/data_source_status.hpp

@@ -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

libs/client-sdk/include/launchdarkly/client_side/data_sources/detail/data_source_status_manager.hpp

@@ -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);
 };
 

libs/client-sdk/src/data_sources/data_source_status.cpp

@@ -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()
@@ -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

libs/client-sdk/src/data_sources/polling_data_source.cpp

@@ -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());

libs/common/include/network/detail/asio_requester.hpp

@@ -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() {
@@ -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_);
@@ -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;
     }
 
@@ -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;
         }
 
@@ -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;
 
@@ -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);
 

libs/common/include/network/detail/http_requester.hpp

@@ -5,6 +5,7 @@
 #include <cstdint>
 #include <future>
 #include <map>
+#include <optional>
 #include <ostream>
 #include <string>
 
@@ -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,
@@ -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);

libs/common/src/data/evaluation_result.cpp

@@ -1,6 +1,8 @@
 #include "data/evaluation_result.hpp"
 
 #include <chrono>
+#include <iomanip>
+#include <utility>
 
 namespace launchdarkly {
 
@@ -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 << "}";

libs/common/src/network/http_requester.cpp

@@ -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();
@@ -79,6 +85,7 @@ HttpRequest::HttpRequest(std::string const& url,
     } else {
         port_ = is_https_ ? "443" : "80";
     }
+    valid_ = true;
 }
 
 HttpRequest::HttpRequest(HttpRequest& base_request,
@@ -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 {
@@ -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;