fix: more helpful error messages for streaming connection failures (#419

)

This commit pulls the stringification logic of SSE errors into the SSE
package itself, and refactors the enum into a richer sum type using
`std::variant`. It also introduces a more helpful log message when SDK key is invalid.

contract-tests/sse-contract-tests/src/event_outbox.cpp

@@ -1,10 +1,12 @@
 #include "event_outbox.hpp"
 #include "definitions.hpp"
 
+#include <boost/beast/core/bind_handler.hpp>
 #include <boost/url/parse.hpp>
-#include <iostream>
 #include <nlohmann/json.hpp>
 
+#include <iostream>
+
 // Check the outbox at this interval. Normally a flush is triggered for
 // every event; this is just a failsafe in case a flush is happening
 // concurrently.
@@ -78,22 +80,8 @@ EventOutbox::RequestType EventOutbox::build_request(
                     json = EventMessage{"event", Event{std::move(arg)}};
                 }
             } else if constexpr (std::is_same_v<T, launchdarkly::sse::Error>) {
-                using launchdarkly::sse::Error;
-                auto msg = ErrorMessage{"error"};
-                switch (arg) {
-                    case Error::NoContent:
-                        msg.comment = "no content";
-                        break;
-                    case Error::InvalidRedirectLocation:
-                        msg.comment = "invalid redirect location";
-                        break;
-                    case Error::UnrecoverableClientError:
-                        msg.comment = "unrecoverable client error";
-                    case Error::ReadTimeout:
-                        msg.comment = "read timeout";
-                    default:
-                        msg.comment = "unspecified error";
-                }
+                auto msg = ErrorMessage{"error",
+                                        launchdarkly::sse::ErrorToString(arg)};
                 json = msg;
             }
         },

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

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

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

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}

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

libs/server-sent-events/src/error.cpp

@@ -0,0 +1,63 @@
+#include <launchdarkly/sse/error.hpp>
+
+#include <sstream>
+
+namespace launchdarkly::sse {
+namespace errors {
+
+std::ostream& operator<<(std::ostream& out, NoContent const&) {
+    out << "received HTTP error 204 (no content) - giving up "
+           "permanently";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         InvalidRedirectLocation const& invalid) {
+    out << "received invalid redirect from server, cannot follow ("
+        << invalid.location << ") - giving up permanently";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out, NotRedirectable const&) {
+    out << "received malformed redirect from server, cannot follow - giving up "
+           "permanently";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out, ReadTimeout const& err) {
+    out << "timed out reading response body (exceeded " << err.timeout->count()
+        << "ms) - will retry";
+    return out;
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         UnrecoverableClientError const& err) {
+    std::string explanation;
+    if (err.status == boost::beast::http::status::unauthorized ||
+        err.status == boost::beast::http::status::forbidden) {
+        explanation = " (invalid auth key)";
+    }
+    out << "received HTTP error " << static_cast<int>(err.status) << explanation
+        << " for streaming connection - giving up "
+           "permanently";
+    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