launchdarkly / android-client-sdk

LaunchDarkly Client-side SDK for Android
Other
45 stars 23 forks source link

EventStreamError if anon user key is empty string #209

Closed richardleggett closed 1 year ago

richardleggett commented 1 year ago

Describe the bug After upgrading from release 3.1.5 to 4.2.0 we noticed that flags were no longer being synced for anonymous users, and LogCat shows:

Serializing object: {
  "timestamp": "2023-02-24T11:18:01.244Z",
  "message": {
      "message": "Encountered EventStream error connecting to URI: https://clientstream.launchdarkly.com/meval/eyJra...= - com.launchdarkly.eventsource.UnsuccessfulResponseException: Unsuccessful response code received from stream: 400"
    } ...
  }

To reproduce Configure LD client with anon user, key is empty string:

private val ldConfig = LDConfig.Builder()
        .mobileKey(prodKey)
        .build()

private val anonUser = LDUser.Builder("")
        .anonymous(true)
        .build()

LDClient.init(applicationContext, ldConfig, anonUser, 5)

Expected behavior Previously it would auto-generate or otherwise retrieve a GUID context key. Flags would be synced for anon users.

Actual behavior No flags are synced (LD Client fails to connect to streaming).

Additional context The workaround appears to be either:

The problem we had was that this was a "silent" breaking change, perhaps it should treat an empty string key as an invalid argument if it results in failing to connect to LD for streaming?

louis-launchdarkly commented 1 year ago

Hello @richardleggett, thank you for reaching out. I see your point - while this breaking change is documented in the changelog: https://github.com/launchdarkly/android-client-sdk/blob/main/CHANGELOG.md#changed-breaking-changes-from-3x The documentation can potentially be improved and it is painful to get this as an EventStream error. The LDContext class has an isValid() method to check if the resulting Context is valid, I wonder if would that be helpful for your case?

Fando commented 1 year ago

The breaking changes note in the change-log for v4.0.0 seems inaccurate.

We are seeing similar behaviour in v3.2.3. This seems like a regression. We were lucky to notice this because there is no error when fetching flags. I'm reporting this in case this is a regression on the back-end, and other users might be unaware.

The Android SDK v3.2.3 is returning an empty flag map when user key = null and anonymous = true, which is contrary to the docs.

In client-side SDKs, if you don't provide a key or set it to null, and set anonymous to true, then the SDK generates a random key for you.

We circumvented the null key compiler warning via, LDUser.Builder(null as? String).anonymous(true).build()

We fixed the issue by specifying a non-empty user key for anon users, but the docs remain misleading and the issue seems like a regression. Maybe the SDK should default to some non-empty user key when anon=true?

louis-launchdarkly commented 1 year ago

I agree with your comment about the documentation, as that statement is misleading with the recently released major version SDKs.

We will need to separately look into the v3.2.3 behavior because the old logic should populate the key by the time the User is sent to LaunchDarkly's backend for evaluation.

louis-launchdarkly commented 1 year ago

Because the root cause is the same anonymous key generation behavior as https://github.com/launchdarkly/android-client-sdk/issues/193, while there is still no ETA yet, I am closing this one and if there is any new update, we will post in https://github.com/launchdarkly/android-client-sdk/issues/193