fix: client init state should not regress when data source interrupted (

#441)

The contract of `Initialized()` is that it's supposed to return true
forever once the client has received some data from LaunchDarkly.

This wasn't correct because `Initialized()` was delegating to
`IsInitializedSuccessfully()`. This function returned true only for the
`Valid` and `Offline` states. This is missing `Interrupted`, which the
SDK might be in after having once been `Valid`.


--------------------------

Besides causing an incorrect result for `Initialized()` when the SDK's
data source was interrupted, this affected logging when calling an
evaluation method, emitting an incorrect message :
> LaunchDarkly client has not yet been initialized. Returning cached
value

Although the action is correct (`returning cached value`), the reason
(`has not yet initialized`) is wrong. We don't need to log this message
at all as an interrupted SDK is normal.

To fix, I've deleted the confusing internal `IsInitialized` method,
which was really a proxy for checking `state != initializing`. I've
modified `IsInitializedSuccessfully` to return true for `Interrupted`.

libs/client-sdk/src/client_impl.cpp

@@ -134,19 +134,28 @@ ClientImpl::ClientImpl(Config in_cfg,
     run_thread_ = std::move(std::thread([&]() { ioc_.run(); }));
 }
 
-// Was an attempt made to initialize the data source, and did that attempt
-// succeed? The data source being connected, or not being connected due to
-// offline mode, both represent successful terminal states.
+// Returns true if the SDK can be considered initialized. We have defined
+// explicit configuration of offline mode as initialized.
+//
+// When online, we're initialized if we've obtained a payload and are healthy
+// (kValid) or obtained a payload and are unhealthy (kInterrupted).
+//
+// The purpose of this concept is to enable:
+//
+// (1) Resolving the StartAsync() promise. Once the SDK is no longer
+// initializing, this promise needs to indicate if the process was successful
+// or not.
+//
+// (2) Providing a getter for (1), if the user didn't check the promise or
+// otherwise need to poll the state. That's the Initialized() method.
+//
+// (3) As a diagnostic during evaluation, to log a message warning that a
+// cached (if persistence is being used) or default (if not) value will be
+// returned because the SDK is not yet initialized.
 static bool IsInitializedSuccessfully(DataSourceStatus::DataSourceState state) {
     return (state == DataSourceStatus::DataSourceState::kValid ||
-            state == DataSourceStatus::DataSourceState::kSetOffline);
-}
-
-// 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);
+            state == DataSourceStatus::DataSourceState::kSetOffline ||
+            state == DataSourceStatus::DataSourceState::kInterrupted);
 }
 
 std::future<bool> ClientImpl::IdentifyAsync(Context context) {
@@ -155,7 +164,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 +178,16 @@ 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) {
-            if (auto const state = status.State(); IsInitialized(state)) {
-                init_promise->set_value(result(status.State()));
+        [init_promise](data_sources::DataSourceStatus const& status) {
+            if (auto const state = status.State();
+                state != DataSourceStatus::DataSourceState::kInitializing) {
+                init_promise->set_value(
+                    IsInitializedSuccessfully(status.State()));
                 return true; /* delete this change listener since the desired
                                 state was reached */
             }
@@ -191,7 +200,7 @@ std::future<bool> ClientImpl::StartAsyncInternal(
 }
 
 std::future<bool> ClientImpl::StartAsync() {
-    return StartAsyncInternal(IsInitializedSuccessfully);
+    return StartAsyncInternal();
 }
 
 bool ClientImpl::Initialized() 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/tests/client_c_bindings_test.cpp

@@ -148,6 +148,8 @@ TEST(ClientBindings, GetStatusOfOfflineClient) {
     bool success = false;
     LDClientSDK_Start(sdk, 3000, &success);
 
+    EXPECT_TRUE(success);
+
     LDDataSourceStatus status_2 = LDClientSDK_DataSourceStatus_Status(sdk);
     EXPECT_EQ(LD_DATASOURCESTATUS_STATE_OFFLINE,
               LDDataSourceStatus_GetState(status_2));
@@ -156,6 +158,8 @@ TEST(ClientBindings, GetStatusOfOfflineClient) {
 
     EXPECT_NE(0, LDDataSourceStatus_StateSince(status_2));
 
+    EXPECT_TRUE(LDClientSDK_Initialized(sdk));
+
     LDDataSourceStatus_Free(status_1);
     LDDataSourceStatus_Free(status_2);
     LDClientSDK_Free(sdk);