launchdarkly / ios-client-sdk

LaunchDarkly Client-side SDK for iOS (Swift and Obj-C)
https://docs.launchdarkly.com/sdk/client-side/ios
Other
68 stars 82 forks source link

Calling Identify With Same Context Passed In Start Doesn't Notify Observers #331

Closed brianmichel closed 5 months ago

brianmichel commented 6 months ago

Describe the bug

We rely on the observe functionality to power reactive UI in our application. However when migrating to the v9 (we were on v5) version of the SDK it seems that how contexts get identified and the downstream behavior has changed. It seems that if you pass in a user context to the start function causes the later identify call to short circuit https://github.com/launchdarkly/ios-client-sdk/blob/e1585eced3f2c1079b45e0a3183b527569fa8a9b/LaunchDarkly/LaunchDarkly/LDClient.swift#L296-L300 which means none of the observers seem to get triggered.

To reproduce

  1. Create a static user context
  2. Pass that context into the start function of the SDK
  3. Subscribe to a feature flag
  4. Pass the same context to the identify call

It seems that passing the context into start will set it as the client's context value which is the same when the identify check subsequently tries to check the new context so the short circuit takes place

Expected behavior I would expect that the code in the identify call to run completely at least once either as part of the start routine or on the first identify call.

Logs If applicable, add any log output related to your problem.

Library version v9.2.1

XCode and Swift version Version 15.1 (15C65)

Platform the issue occurs on macOS

Additional context Add any other context about the problem here.

brianmichel commented 6 months ago

We construct an LDContext with really basic information so if someone is already signed in it won't change between setting up the LD SDK and making the first identify call. Should we be using an anonymous context to start our SDK?

Our user context protocol:

public protocol FeatureFlagClientUser {
    var identifier: String { get }
    var createdAt: Date { get }
}
keelerm84 commented 5 months ago

It seems that passing the context into start will set it as the client's context value

Yes, this is correct. When calling start, you can optionally provide a context. If you don't, the SDK initializes the SDK with an anonymous context. And as you say, this sets it as the internal context for the SDK, the same as identify.

I would expect that the code in the identify call to run completely at least once either as part of the start routine or on the first identify call.

The issue here isn't whether or not they share the same call flow (they actually do share quite a bit of downstream functionality). The real problem is that you cannot register observers on the SDK until you have created it, and you can't create the SDK without initializing it to start with (which always requires a context).

Should we be using an anonymous context to start our SDK?

That would solve the problem, but at the cost of tearing down and setting back up fresh network connections during the context switch.

Alternatively, the start method of the SDK provides for a completion parameter. This completion parameter is fired once the SDK has come online and has received flag updates. You could use this completion handler to signal when initial start up is finished, and then react to changes via the observers. Does that sound acceptable?

brianmichel commented 5 months ago

It seems that passing the context into start will set it as the client's context value

Yes, this is correct. When calling start, you can optionally provide a context. If you don't, the SDK initializes the SDK with an anonymous context. And as you say, this sets it as the internal context for the SDK, the same as identify.

I would expect that the code in the identify call to run completely at least once either as part of the start routine or on the first identify call.

The issue here isn't whether or not they share the same call flow (they actually do share quite a bit of downstream functionality). The real problem is that you cannot register observers on the SDK until you have created it, and you can't create the SDK without initializing it to start with (which always requires a context).

Should we be using an anonymous context to start our SDK?

That would solve the problem, but at the cost of tearing down and setting back up fresh network connections during the context switch.

Alternatively, the start method of the SDK provides for a completion parameter. This completion parameter is fired once the SDK has come online and has received flag updates. You could use this completion handler to signal when initial start up is finished, and then react to changes via the observers. Does that sound acceptable?

Yeah I toyed around with using the completion parameter to do the identification call we need. Ultimately, I think jumping so many version (5 to 9) there are just naturally going to be a few places where we need to do some adjusting. I guess you can close this issue for now since it just seems like a fact of life for how the SDK now functions.