fix: more helpful error messages for streaming connection failures

launchdarkly · Jun 26, 2024 · 2b379f3 · 2b379f3
1 parent f719d78
commit 2b379f3
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 44 deletions.
diff --git a/libs/client-sdk/src/data_sources/streaming_data_source.cpp b/libs/client-sdk/src/data_sources/streaming_data_source.cpp
@@ -19,21 +19,6 @@ namespace launchdarkly::client_side::data_sources {
 static char const* const kCouldNotParseEndpoint =
     "Could not parse streaming endpoint URL.";
 
-static char const* DataSourceErrorToString(launchdarkly::sse::Error error) {
-    switch (error) {
-        case sse::Error::NoContent:
-            return "server responded 204 (No Content), will not attempt to "
-                   "reconnect";
-        case sse::Error::InvalidRedirectLocation:
-            return "server responded with an invalid redirection";
-        case sse::Error::UnrecoverableClientError:
-            return "unrecoverable client-side error";
-        case sse::Error::ReadTimeout:
-            return "read timeout reached";
-    }
-    launchdarkly::detail::unreachable();
-}
-
 StreamingDataSource::StreamingDataSource(
     config::shared::built::ServiceEndpoints const& endpoints,
     config::shared::built::DataSourceConfig<config::shared::ClientSDK> const&
@@ -155,12 +140,12 @@ void StreamingDataSource::Start() {
 
     client_builder.errors([weak_self](auto error) {
         if (auto self = weak_self.lock()) {
-            auto error_string = DataSourceErrorToString(error);
+            auto error_string = launchdarkly::sse::ErrorToString(error);
             LD_LOG(self->logger_, LogLevel::kError) << error_string;
             self->status_manager_.SetState(
                 DataSourceStatus::DataSourceState::kShutdown,
                 DataSourceStatus::ErrorInfo::ErrorKind::kErrorResponse,
-                error_string);
+                std::move(error_string));
         }
     });
 

diff --git a/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp b/libs/server-sdk/src/data_systems/background_sync/sources/streaming/streaming_data_source.cpp
@@ -15,20 +15,6 @@ namespace launchdarkly::server_side::data_systems {
 static char const* const kCouldNotParseEndpoint =
     "Could not parse streaming endpoint URL";
 
-static char const* DataSourceErrorToString(sse::Error const error) {
-    switch (error) {
-        case sse::Error::NoContent:
-            return "server responded 204 (No Content), will not attempt to "
-                   "reconnect";
-        case sse::Error::InvalidRedirectLocation:
-            return "server responded with an invalid redirection";
-        case sse::Error::UnrecoverableClientError:
-            return "unrecoverable client-side error";
-        default:
-            return "unrecognized error";
-    }
-}
-
 std::string const& StreamingDataSource::Identity() const {
     static std::string const identity = "streaming data source";
     return identity;
@@ -132,12 +118,12 @@ void StreamingDataSource::StartAsync(
 
     client_builder.errors([weak_self](auto error) {
         if (auto self = weak_self.lock()) {
-            auto error_string = DataSourceErrorToString(error);
+            std::string error_string = launchdarkly::sse::ErrorToString(error);
             LD_LOG(self->logger_, LogLevel::kError) << error_string;
             self->status_manager_.SetState(
                 DataSourceStatus::DataSourceState::kOff,
                 DataSourceStatus::ErrorInfo::ErrorKind::kErrorResponse,
-                error_string);
+                std::move(error_string));
         }
     });
 

diff --git a/libs/server-sent-events/include/launchdarkly/sse/error.hpp b/libs/server-sent-events/include/launchdarkly/sse/error.hpp
@@ -1,11 +1,44 @@
 #pragma once
+#include <chrono>
+#include <optional>
+#include <ostream>
+#include <variant>
+
+#include <boost/beast/http/status.hpp>
 
 namespace launchdarkly::sse {
+namespace errors {
+
+struct NoContent {};
+std::ostream& operator<<(std::ostream& out, NoContent const&);
+
+struct InvalidRedirectLocation {
+    std::string location;
+};
+std::ostream& operator<<(std::ostream& out, InvalidRedirectLocation const&);
+
+struct NotRedirectable {};
+std::ostream& operator<<(std::ostream& out, NotRedirectable const&);
 
-enum class Error {
-    NoContent = 1,
-    InvalidRedirectLocation = 2,
-    UnrecoverableClientError = 3,
-    ReadTimeout = 4,
+struct ReadTimeout {
+    std::optional<std::chrono::milliseconds> timeout;
 };
-}
+std::ostream& operator<<(std::ostream& out, ReadTimeout const&);
+
+struct UnrecoverableClientError {
+    boost::beast::http::status status;
+};
+std::ostream& operator<<(std::ostream& out, UnrecoverableClientError const&);
+
+}  // namespace errors
+
+using Error = std::variant<errors::NoContent,
+                           errors::InvalidRedirectLocation,
+                           errors::NotRedirectable,
+                           errors::ReadTimeout,
+                           errors::UnrecoverableClientError>;
+
+std::ostream& operator<<(std::ostream& out, Error const& error);
+
+std::string ErrorToString(Error const& error);
+}  // namespace launchdarkly::sse
diff --git a/libs/server-sent-events/src/CMakeLists.txt b/libs/server-sent-events/src/CMakeLists.txt
@@ -10,6 +10,7 @@ add_library(${LIBNAME} OBJECT
         client.cpp
         parser.cpp
         event.cpp
+        error.cpp
         backoff.cpp)
 
 target_link_libraries(${LIBNAME}

diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp
@@ -222,7 +222,7 @@ class FoxyClient : public Client,
 
         if (status_class == beast::http::status_class::successful) {
             if (response.result() == beast::http::status::no_content) {
-                errors_(Error::NoContent);
+                errors_(errors::NoContent{});
                 return;
             }
             if (!correct_content_type(response)) {
@@ -247,14 +247,14 @@ class FoxyClient : public Client,
                 auto new_url = redirect_url("base", location_header);
 
                 if (!new_url) {
-                    errors_(Error::InvalidRedirectLocation);
+                    errors_(errors::InvalidRedirectLocation{location_header});
                     return;
                 }
 
                 req_.set(http::field::host, new_url->host());
                 req_.target(new_url->encoded_target());
             } else {
-                errors_(Error::InvalidRedirectLocation);
+                errors_(errors::NotRedirectable{});
                 return;
             }
         }
@@ -264,7 +264,7 @@ class FoxyClient : public Client,
                 return async_backoff(backoff_reason(response.result()));
             }
 
-            errors_(Error::UnrecoverableClientError);
+            errors_(errors::UnrecoverableClientError{response.result()});
             return;
         }
 
@@ -284,7 +284,7 @@ class FoxyClient : public Client,
                 return;
             }
             if (ec == boost::asio::error::operation_aborted) {
-                errors_(Error::ReadTimeout);
+                errors_(errors::ReadTimeout{read_timeout_});
                 return async_backoff(
                     "aborting read of response body (timeout)");
             }

diff --git a/libs/server-sent-events/src/error.cpp b/libs/server-sent-events/src/error.cpp
@@ -0,0 +1,61 @@
+#include <launchdarkly/sse/error.hpp>
+
+#include <sstream>
+
+namespace launchdarkly::sse {
+namespace errors {
+
+std::ostream& operator<<(std::ostream& out, NoContent const&) {
+    out << "no content, will not attempt to reconnect (HTTP 204)";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         InvalidRedirectLocation const& invalid) {
+    out << "server responded with an invalid redirect (" << invalid.location
+        << ")";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out, NotRedirectable const&) {
+    out << "cannot follow server redirect";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out, ReadTimeout const& err) {
+    out << "timed out reading response body (exceeded " << err.timeout->count()
+        << "ms)";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         UnrecoverableClientError const& err) {
+    if (err.status == boost::beast::http::status::unauthorized ||
+        err.status == boost::beast::http::status::forbidden) {
+        out << "invalid auth key (HTTP " << static_cast<int>(err.status) << ")";
+
+    } else {
+        out << "unrecoverable client-side error (HTTP "
+            << static_cast<int>(err.status) << ")";
+    }
+    return out;
+}
+}  // namespace errors
+
+std::ostream& operator<<(std::ostream& out, Error const& error) {
+    std::visit(
+        [&](auto&& arg) {
+            using T = std::decay_t<decltype(arg)>;
+            out << arg;
+        },
+        error);
+    return out;
+}
+
+std::string ErrorToString(Error const& error) {
+    std::stringstream ss;
+    ss << error;
+    return ss.str();
+}
+
+}  // namespace launchdarkly::sse