launchdarkly / cpp-sdks

C++ Client/Server SDKs
Other
5 stars 2 forks source link

Setting the client offline does not get offline. #404

Closed ngangomsamananda closed 2 months ago

ngangomsamananda commented 2 months ago

Describe the bug Setting the client sdk offline using LDClientConfigBuilder_Offline does not go offline.

To reproduce `#include < iostream >

include <launchdarkly/client_side/bindings/c/sdk.h>

include <launchdarkly/bindings/c/context_builder.h>

include <launchdarkly/client_side/bindings/c/config/builder.h>

include <launchdarkly/bindings/c/object_builder.h>

include <launchdarkly/bindings/c/memory_routines.h>

LDClientConfigBuilder ConfigBuilder = nullptr; LDClientSDK g_pLDClient = nullptr;

void setOffline() { if (ConfigBuilder) { LDClientConfigBuilder_Offline(ConfigBuilder, true); } }

void setOnline() { if (ConfigBuilder) { LDClientConfigBuilder_Offline(ConfigBuilder, false); } }

bool isOffline() { if (g_pLDClient) { LDDataSourceStatus status = LDClientSDK_DataSourceStatus_Status(g_pLDClient); LDDataSourceStatus_State statusState = LDDataSourceStatus_GetState(status); if (statusState == LDDataSourceStatus_State::LD_DATASOURCESTATUS_STATE_OFFLINE || statusState == LDDataSourceStatus_State::LD_DATASOURCESTATUS_STATE_SHUTDOWN) { return true; } }

return false;

}

int main() { const char userid = "user-id"; const char mobile_key = "mob-key-abc-123"; ConfigBuilder = LDClientConfigBuilder_New(mobile_key); LDClientConfigBuilder_Events_PrivateAttribute(ConfigBuilder, "hubId");

LDClientConfig config;
LDStatus status = LDClientConfigBuilder_Build(ConfigBuilder, &config);
if (!LDStatus_Ok(status)) {
  std::cout<<"ldcsdk client config builder failed";
}

LDContextBuilder context_builder = LDContextBuilder_New();
LDContextBuilder_AddKind(context_builder, "user", userid);

LDContextBuilder_Attributes_SetPrivate(context_builder, "user", "email", LDValue_NewString("abc@gmail.com"));

LDContext context = LDContextBuilder_Build(context_builder);
g_pLDClient = LDClientSDK_New(config, context);
unsigned int maxwait = 10 * 1000; /* 10 seconds */

bool initialized_successfully;
if (LDClientSDK_Start(g_pLDClient, maxwait, &initialized_successfully)) 
{
    if (initialized_successfully) 
    {
        std::cout << "LaunchDarkly: client Initialization Succeded." << std::endl;
    }
    else 
    {
        std::cout<<"LaunchDarkly: client Initialization failed\n";
    }
} 
else {
        std::cout<<"LaunchDarkly: The client is still initializing.\n";
}

bool initialized = LDClientSDK_Initialized(g_pLDClient);

setOffline();
bool offline = isOffline();

}`

Here isOffline() returns false after setting sdk offline. Expected behavior Setting the client sdk offline should go offline.

Logs If applicable, add any log output related to your problem. To get more logs from the SDK, change the log level using environment variable LD_LOG_LEVEL. For example:

   LD_LOG_LEVEL=debug ./your-application

SDK version 3.5.0

Language version, developer tools For instance, C++17 or C11. If you are using a language that requires a separate compiler, such as C, please include the name and version of the compiler too.

OS/platform Windows 10

Additional context Add any other context about the problem here.

cwaldren-ld commented 2 months ago

Hi @ngangomsamananda, you must not call config methods after the configuration is built and passed into the SDK.

Your current code invokes undefined behavior. By the time you call setOffline(), the ConfigBuilder is already automatically freed by LDClientConfigBuilder_Build method.

See the docs here:

Creates an LDClientConfig. The LDClientConfigBuilder is consumed. On success, the config will be stored in out_config; otherwise, out_config will be set to NULL and the returned LDStatus will indicate the error.

To set Offline mode, move setOffline() call to somewhere after creating the ConfigBuilder, but before calling Build:

ConfigBuilder = LDClientConfigBuilder_New(mobile_key);
LDClientConfigBuilder_Events_PrivateAttribute(ConfigBuilder, "hubId");

setOffline(); // <---- move the call to here, before the LDClientConfigBuilder_Build() method is called

LDClientConfig config;
LDStatus status = LDClientConfigBuilder_Build(ConfigBuilder, &config);

I can see how you may have been confused. In the old c-client-sdk, there were two methods: (1) LDConfigSetOffline <-- this must be called at configuration time (2) LDClientSetOffline <-- this may be called at runtime

In the new SDK, LDClientConfigBuilder_Offline is equivalent to (1) LDConfigSetOffline. There is no replacement for (2).

Do you need to change offline mode after the SDK has already been setup and connected to LaunchDarkly?

ngangomsamananda commented 2 months ago

Hi @cwaldren-ld , I got confused withLDConfigSetOffline and LDClientSetOffline. What I need is LDClientSetOffline. Since you said there is no replacement for LDClientSetOfflinein new SDK. Could you please implement this feature for the upcoming release? With that we also need the replacement for LDClientSetOnline and LDClientIsOffline.

Yes, we need to change offline mode after the SDK has already setup and connected. When the Internet connectivity is down, we switch to offline mode.

cwaldren-ld commented 2 months ago

@ngangomsamananda I am wondering more about your use-case.

Normally, if internet is down, the SDK will automatically perform exponential backoff algorithm to reconnect when it is available again. For example, it will wait 1 second, then 2, then 4, etc. with jitter.

Is this behavior unacceptable for you? Or is the internet down for long periods of time, and you would rather the SDK does not try to connect because it causes log noise or similar?

ngangomsamananda commented 2 months ago

@cwaldren-ld It appears to me that we don't want SDK trying to connect when internet is down. I will get back to you with more informations on this.

ngangomsamananda commented 2 months ago

Hi @cwaldren-ld as you pointed out, we don't want SDK try to reconnect when the internet is down because it causes a lot of log noise. Is there any plan to implement this feature? If yes, how long would take to complete?

ngangomsamananda commented 2 months ago

@cwaldren-ld we are filtering the log that come when we try to reconnect. By doing that we prevent from logging excess information.

cwaldren-ld commented 2 months ago

Got it. I will file a feature request for this, but I cannot promise any timeline.

Filed internally as 246084.

ngangomsamananda commented 2 months ago

Thanks. We are able to find the work around. Anyway, in future we can change if the API's are available.