launchdarkly / ios-client-sdk

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

Monitoring offline not working #403

Open thomas-cassany opened 2 months ago

thomas-cassany commented 2 months ago

Describe the bug When the SDK lost the network connection, the observers registered with observeCurrentConnectionMode are not called.

client.connectionInformation has currentConnectionMode set to streaming. when the network connection is lost or any SynchronizingError except unauthorized one happens, the SDK detects the issue, update LDClient.connectionInformation.lastConnectionFailureReason, but never informs the observers registered with observeCurrentConnectionMode

It's due to ConnectionInformation.synchronizingErrorCheck that updates connectionInformation but doesn't change currentConnectionMode. The switch of currentConnectionMode is needed to trigger the call to the observers. For some SynchronizingError (maybe all), currentConnectionMode should be set to offline until the connection is back. Or it can be set to establishingStreamingConnection, if the error is temporary.

To reproduce

Expected behavior Observer of observeCurrentConnectionMode called with the mode offline or establishingStreamingConnection

Logs

Library version Reproduced with the version 9.8.2.

XCode and Swift version XCode 15.4 Swift 5

Platform the issue occurs on Found on iPhone but likely to be all other platform too( iPad, macOS, tvOS, or watchOS)

Additional context

tanderson-ld commented 2 months ago

HI @thomas-cassany . Thank you for reporting this issue and providing the results of your investigation. That should help us reproduce/fix this quickly.

Filed internally as 252639.

thomas-cassany commented 2 months ago

@tanderson-ld Thanks. Is it possible to have an idea how does it prioritize? We are waiting for this change but if it takes long, we will use another approach in between.

tanderson-ld commented 2 months ago

It is currently the 4th item in my backlog and I think will get looked at within the next week or two.

thomas-cassany commented 2 months ago

Excellent. Thanks for the update.

tanderson-ld commented 2 months ago

@thomas-cassany , we've been able to reproduce the issue and we see a related issue in the other direction of going from offline to online. Currently investigating potential fixes.

tanderson-ld commented 2 months ago

@thomas-cassany, the scope of the change to fix this is larger than we expected and will take more time to evaluate the impact to other portions of the SDK. In the interest of getting you unblocked, could you describe how you are using observeCurrentConnectionMode? Perhaps we can find a workaround in the mean time.

thomas-cassany commented 2 months ago

Actually, we want to monitor which users have issue to get access to our features due to a problem with the LD access. We want to avoid sending the full flag list to minimise the volume of data send for monitoring.

tanderson-ld commented 2 months ago

Is there another issue you are seeing related to customers not seeing the right flag configurations when expected? That would be a much bigger issue that we would want to investigate as at the moment we don't have any known bugs affecting evaluations of flags on iOS.

The root cause of the observability API issue is as you said, but the code is not currently set up to modify the mode from that synchronizingErrorCheck function so the fix is more involved. The flag fetching and evaluation logic is functioning properly and independently of the observability API. We have plans in the near future to refactor the iOS SDK significantly and are discussing if this observability fix will be done as part of that refactoring.

Is there another event in your application that triggers wanting to know the freshness of the flags / flag introspection? Perhaps a customer opening a support ticket?

You could query the connection information at that time and record if the last failed connection time is recent since that is being properly updated when errors are encountered. Perhaps even just the difference between current time and now.

let lastFail = LDClient.get()!.getConnectionInformation().lastFailedConnection

However the phone being offline would also lead to the failed connection, so this doesn't definitively prove an issue in the flagging system itself without knowing other networking information.

thomas-cassany commented 2 months ago

Thanks @tanderson-ld I appreciate the effort and the quality to give me feedback. I was a bit short on what we want to measure in my last message. We are confident that LD is super reliable, but we know that mobile network connection is not 100% sure. We expect to have some sync issue due to VPN, network restriction or anything else. Even if it's a small percentage, we want to have a clear vision on it and then decided what to do.

As you mentioned, LDClient.get()!.getConnectionInformation() and date difference are enough. My tests show we can have a workaround using the connection information when the client goes to background. It needs to keep in memory some data when currentConnectionMode is set to streaming. It should be ok because our users don't do a long session and so shouldn't have connection up and down in the same session.

On different note, I'll be happy to help you more on the iOS SDK evolution.