Expo 49/RN 0.72.1 support

euc-callum commented 10 months ago

Describe the bug I'm finding that an update to Expo 49 from Expo 48, which also involves a bump to RN 0.72.1 is causing the configure call to not resolve on Android.

I'm using version 7.1.6 of this library.

The following configuration is supplied to the configure call:

...

const deviceContext = {
  kind: 'device',
  anonymous: true,
  platform: Platform.OS,
  buildNumber: config.buildNumber,
} as const;

...
const ldConfig: LDConfig = { mobileKey: config.launchDarklyMobileKey };

const userContext = userId
  ? { kind: 'user', key: userId, role }
  : { kind: 'user', anonymous: true, role };

const context = {
  kind: 'multi',
  user: userContext,
  device: { ...FeatureFlagClient.deviceContext, region: selectedRegion },
} as const;

try {
  const ldConnTimeout = 5000;
  await this.client.configure(ldConfig, context, ldConnTimeout); // Freezes here
  ...

To reproduce

Use react-native-client-sdk 7.1.6
Use Expo 48
Include an awaited call to .configure as specified above
Update to Expo 49

Expected behavior The launch darkly client is configured correctly and the promise resolves.

SDK version 7.1.6

OS/platform Android

tanderson-ld commented 10 months ago

Hi @euc-callum , have you tested your project on iOS? What was the result?

euc-callum commented 10 months ago

Hey @tanderson-ld, on iOS the .configure function returns fine.

I suspect there's some case on Android in which the promise is not being resolved / rejected.

tanderson-ld commented 10 months ago

We do not currently support Expo explicitly. We have a ticket in our backlog to update to RN 0.72 and perhaps that will resolve this issue. Internally tracked as 212299.

euc-callum commented 10 months ago

Good to know, thanks @tanderson-ld!

Is there a means of viewing this internal ticket / understanding the ETA for it?

tanderson-ld commented 10 months ago

Unfortunately our ticket system is internal only. It isn't too deep in the backlog. We don't commit to dates publicly in case urgent work appears and priorities shift. Sorry I can't provide more specifics.

euc-callum commented 10 months ago

Fair enough.

Its probably worth flagging that this is making us reconsider utilising this SDK/LaunchDarkly, as for the last two RN version bumps it has been the one library we utilise that has taken the longest to update, and thus blocked us.

For example there is a bug in one of our dependencies that is impacting ~0.5% of sessions a day, that would be resolved if we moved to RN 0.72.1/Expo 49. This is the sole dependency blocking that for us currently.

https://github.com/launchdarkly/react-native-client-sdk/issues/173

euc-callum commented 10 months ago

Some further information on this.

We're seeing the following warning in the logs:

Target SDK mismatch: receiver ActivityInfo{10d0953 com.launchdarkly.sdk.android.ConnectivityReceiver} targets 33 but delivery restricted to [0, 23] broadcasting Intent { act=android.net.conn.CONNECTIVITY_CHANGE flg=0x4200010 (has extras) } from android (pid=575, uid=1000) to ..../com.launchdarkly.sdk.android.ConnectivityReceiver

Additionally, it is blocking on this line of code specifically:

https://github.com/launchdarkly/react-native-client-sdk/blob/main/android/src/main/java/com/launchdarkly/reactnative/LaunchdarklyReactNativeClientModule.java#L148

configureContext returns fine, so its something in the LDClient.init method.

tanderson-ld commented 10 months ago

For example there is a bug in one of our dependencies that is impacting ~0.5% of sessions a day, that would be resolved if we moved to RN 0.72.1/Expo 49. This is the sole dependency blocking that for us currently.

Is this related to the ConnectivityReceiver warning you mentioned in your other comment or is that independent? Does the ConnectivityReceiver warning appear 100% of the time?

tanderson-ld commented 10 months ago

I talked more with my team. We think we'll have 0.72 support in a few weeks, but since we do not support Expo explicitly, it may not fix the configure issue that's happening.

euc-callum commented 10 months ago

Is this related to the ConnectivityReceiver warning you mentioned in your other comment or is that independent? Does the ConnectivityReceiver warning appear 100% of the time?

Independent, I'm sharing that warning as its coming from LaunchDarkly at roughly the same time as the function blocks.

I talked more with my team. We think we'll have 0.72 support in a few weeks, but since we do not support Expo explicitly, it may not fix the configure issue that's happening.

I suspect it will, so 🤞

mikevercoelen commented 10 months ago

@tanderson-ld We're having the same issue on our Android development builds with Expo v49.

Not sure why you guys are not supporting expo, but it's the future, it's no longer crap like it used to be, so Expo support should be a must have.

@euc-callum Do you have this in production also? Production here seems to be fine

euc-callum commented 10 months ago

Interesting @mikevercoelen, thanks for the tip, will give that a look next week.

euc-callum commented 10 months ago

Did this go live four days ago https://github.com/launchdarkly/react-native-client-sdk/releases/tag/8.0.0 @tanderson-ld?

tanderson-ld commented 10 months ago

@euc-callum , yes! I meant to come through here and let you know and to ask you to give it a try. It includes RN 72.4 as a dependency and also a Android compile target bump. It should be pretty quick to test as the only major change is a config signature change requiring you to make a decision on whether Auto Env Attributes (a new feature) is turned on or off.

I could see this fixing your issue is if RN is doing something funky with broadcast intents based on the logs you sent previously. I'm just unsure if that is the case.

euc-callum commented 10 months ago

Thanks @tanderson-ld, its looking like its working so far locally on Android. I'm aiming to test it on an internal testing track build today/tomorrow, and will get back to you!

I appreciate the effort and timeliness on the change, thanks! 👍

tanderson-ld commented 10 months ago

Good to hear, I'm honestly a little surprised. This indicates something fishy was going on in the frameworks, which is a little disconcerting. Let us know what you find in the internal testing track.

@yusinto did all the work! Shout out to him.

GabrielDierks commented 8 months ago

Hey everyone just a heads up we managed to implement Launchdarkly successfully with Expo. Essentially there were no extra steps needed for our product that is using a Development build via prebuild. The only fix we had to do was to move the "expo-dev-client" package to the devDependencies because otherwise requests wouldn't go through for android!

"expo": "^49.0.11", "react-native": "^0.72.5", "launchdarkly-react-native-client-sdk": "^8.0.1",

rgommezz commented 7 months ago

@GabrielDierks I am still struggling with Android, despite having the same dependencies as you. I am also using Expo prebuild. Some of my findings based on my init call are as follows:

Edit: it works perfectly on release type builds, so the issue only happens when running a local development server.

await client.configure(
  {
    mobileKey: launchDarklyKey,
    enableAutoEnvAttributes: false,
  },
  {
    kind: 'user',
    key: 'someId',
    userId: '1'
  },
  Platform.OS === 'android' ? 2 : undefined
);

If I don't set a random timeout, the promise never resolves
Skipping enableAutoEnvAttributes also seems to make the promise hung forever
Interestingly the promise resolves right after the timeout fires. If I set it to 2, it resolves after 2 seconds, if I set to 5, after 5 seconds. However, the flags are never available, and when accessing them, the getters always return the default value set.

For instance, await ldClient.boolVariation('someKey', false) always resolves to false. The documentation states that the configure promise will be rejected.

The picture below represents 2 users with active sessions trying to access feature flags, 1 Android and 1 iOS. The highlighted part is the result of Android getting values, which always resort to the default value set in code, whereas iOS works as expected.

louis-launchdarkly commented 5 months ago

@euc-callum @mikevercoelen @GabrielDierks @rgommezz

We have taken the request in mind for the development of the next LaunchDarkly React Native SDK - https://github.com/launchdarkly/js-core/tree/main/packages/sdk/react-native is currently in alpha, written in pure JavaScript, and supports Expo 49/RN 0.72.x. If you are interested, we would love to get some feedback before moving this 10.x SDK to GA.

launchdarkly / react-native-client-sdk

Expo 49/RN 0.72.1 support #227