fix: improve handling of streaming error state changes/logging (#439)

The existing handling for data source state transitions when receiving
`sse` errors was incorrect. It was treating any error as a state
transition to `kOff/kShutdown`, and logging at the `error` level.

The correct behavior would be making that transition only if the error
is unrecoverable. Additionally, recoverable errors should be logged at
the debug level, in order to not overstate the impact of the issue.

This bug did not actually affect the backoff behavior of the
eventsource, but it did impact status listeners and any code that queried the existing status within the SDK.

libs/client-sdk/include/launchdarkly/client_side/bindings/c/sdk.h

@@ -504,6 +504,15 @@ enum LDDataSourceStatus_State {
     LD_DATASOURCESTATUS_STATE_SHUTDOWN = 4
 };
 
+/**
+ *
+ * @return Returns the name of the given LDDataSourceStatus_State as a string.
+ * If the enum value is not recognized, the default string value is returned.
+ */
+LD_EXPORT(char const*)
+LDDataSourceStatus_State_Name(enum LDDataSourceStatus_State,
+                              char const* default_if_unknown);
+
 /**
  * Get an enumerated value representing the overall current state of the data
  * source.

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

@@ -62,13 +62,7 @@ enum class DataSourceState {
      * SDK key will never become valid), or because the SDK client was
      * explicitly shut down.
      */
-    kShutdown = 4,
-
-    // BackgroundDisabled,
-
-    // TODO: A plugin of sorts would likely be required to implement
-    // network availability.
-    // kNetworkUnavailable,
+    kShutdown = 4
 };
 
 using DataSourceStatus =
@@ -116,6 +110,14 @@ class IDataSourceStatusProvider {
     IDataSourceStatusProvider() = default;
 };
 
+/**
+ *
+ * @return Returns the name of the given DataSourceState as a string. If
+ * the enum value is not recognized, the default string value is returned.
+ */
+char const* GetDataSourceStateName(DataSourceState state,
+                                   char const* default_if_unknown);
+
 std::ostream& operator<<(std::ostream& out,
                          DataSourceStatus::DataSourceState const& state);
 

libs/client-sdk/src/bindings/c/sdk.cpp

@@ -352,6 +352,14 @@ LDDataSourceStatus_GetState(LDDataSourceStatus status) {
         TO_DATASOURCESTATUS(status)->State());
 }
 
+LD_EXPORT(char const*)
+LDDataSourceStatus_State_Name(enum LDDataSourceStatus_State state,
+                              char const* default_if_unknown) {
+    return GetDataSourceStateName(
+        static_cast<data_sources::DataSourceStatus::DataSourceState>(state),
+        default_if_unknown);
+}
+
 LD_EXPORT(LDDataSourceStatus_ErrorInfo)
 LDDataSourceStatus_GetLastError(LDDataSourceStatus status) {
     LD_ASSERT_NOT_NULL(status);

libs/client-sdk/src/client_impl.cpp

@@ -145,8 +145,7 @@ static bool IsInitializedSuccessfully(DataSourceStatus::DataSourceState state) {
 // Was any attempt made to initialize the data source (with a successful or
 // permanent failure outcome?)
 static bool IsInitialized(DataSourceStatus::DataSourceState state) {
-    return IsInitializedSuccessfully(state) ||
-           (state == DataSourceStatus::DataSourceState::kShutdown);
+    return state != DataSourceStatus::DataSourceState::kInitializing;
 }
 
 std::future<bool> ClientImpl::IdentifyAsync(Context context) {
@@ -155,7 +154,7 @@ std::future<bool> ClientImpl::IdentifyAsync(Context context) {
     event_processor_->SendAsync(events::IdentifyEventParams{
         std::chrono::system_clock::now(), std::move(context)});
 
-    return StartAsyncInternal(IsInitializedSuccessfully);
+    return StartAsyncInternal();
 }
 
 void ClientImpl::RestartDataSource() {
@@ -169,16 +168,15 @@ void ClientImpl::RestartDataSource() {
     data_source_->ShutdownAsync(start_op);
 }
 
-std::future<bool> ClientImpl::StartAsyncInternal(
-    std::function<bool(DataSourceStatus::DataSourceState)> result_predicate) {
+std::future<bool> ClientImpl::StartAsyncInternal() {
     auto init_promise = std::make_shared<std::promise<bool>>();
     auto init_future = init_promise->get_future();
 
     status_manager_.OnDataSourceStatusChangeEx(
-        [result = std::move(result_predicate),
-         init_promise](data_sources::DataSourceStatus const& status) {
+        [init_promise](data_sources::DataSourceStatus const& status) {
             if (auto const state = status.State(); IsInitialized(state)) {
-                init_promise->set_value(result(status.State()));
+                init_promise->set_value(
+                    IsInitializedSuccessfully(status.State()));
                 return true; /* delete this change listener since the desired
                                 state was reached */
             }
@@ -191,11 +189,11 @@ std::future<bool> ClientImpl::StartAsyncInternal(
 }
 
 std::future<bool> ClientImpl::StartAsync() {
-    return StartAsyncInternal(IsInitializedSuccessfully);
+    return StartAsyncInternal();
 }
 
 bool ClientImpl::Initialized() const {
-    return IsInitializedSuccessfully(status_manager_.Status().State());
+    return IsInitialized(status_manager_.Status().State());
 }
 
 std::unordered_map<Client::FlagKey, Value> ClientImpl::AllFlags() const {

libs/client-sdk/src/client_impl.hpp

@@ -109,10 +109,7 @@ class ClientImpl : public IClient {
 
     void RestartDataSource();
 
-    std::future<bool> StartAsyncInternal(
-        std::function<bool(data_sources::DataSourceStatus::DataSourceState)>
-            predicate);
-
+    std::future<bool> StartAsyncInternal();
     Config config_;
     Logger logger_;
 

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

@@ -4,26 +4,27 @@
 
 namespace launchdarkly::client_side::data_sources {
 
-std::ostream& operator<<(std::ostream& out,
-                         DataSourceStatus::DataSourceState const& state) {
+char const* GetDataSourceStateName(DataSourceState state,
+                                   char const* default_if_unknown) {
     switch (state) {
         case DataSourceStatus::DataSourceState::kInitializing:
-            out << "INITIALIZING";
-            break;
+            return "INITIALIZING";
         case DataSourceStatus::DataSourceState::kValid:
-            out << "VALID";
-            break;
+            return "VALID";
         case DataSourceStatus::DataSourceState::kInterrupted:
-            out << "INTERRUPTED";
-            break;
+            return "INTERRUPTED";
         case DataSourceStatus::DataSourceState::kSetOffline:
-            out << "OFFLINE";
-            break;
+            return "OFFLINE";
         case DataSourceStatus::DataSourceState::kShutdown:
-            out << "SHUTDOWN";
-            break;
+            return "SHUTDOWN";
+        default:
+            return default_if_unknown;
     }
+}
 
+std::ostream& operator<<(std::ostream& out,
+                         DataSourceStatus::DataSourceState const& state) {
+    out << GetDataSourceStateName(state, "UNKNOWN");
     return out;
 }