launchdarkly / android-client-sdk

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

LDContext.builderFromContext() throws NPE when trying to set a custom attribute #229

Closed gzhu closed 4 months ago

gzhu commented 9 months ago

Is this a support request? No

Describe the bug When updating the LDContext using builderFromContext(), if the original context doesn't have any custom attributes, trying to set a custom attribute gives a null pointer error. The copyOnWriteAttributes flag gets set and seems to cause a HashMap to be constructed from a null hashmap attributes = new HashMap<>(attributes);

To reproduce I started with a plain context:

ldContext = LDContext.create(UUID.randomUUID().toString())

and then later when I need to update the context:

val updatedContext: LDContext = LDContext.builderFromContext(ldContext)
            .anonymous(false)
            .name(name)
            .set("email", email)
            .build()

It crashes on the .set("email", email) call

Expected behavior It should not crash

Logs

java.lang.NullPointerException: Attempt to invoke interface method 'int java.util.Map.size()' on a null object reference
                                                                                                        at java.util.HashMap.putMapEntries(HashMap.java:493)
                                                                                                        at java.util.HashMap.<init>(HashMap.java:482)
                                                                                                        at com.launchdarkly.sdk.ContextBuilder.trySet(ContextBuilder.java:306)
                                                                                                        at com.launchdarkly.sdk.ContextBuilder.set(ContextBuilder.java:212)
                                                                                                        at com.launchdarkly.sdk.ContextBuilder.set(ContextBuilder.java:261)

SDK version 5.0.2

Language version, developer tools Kotlin 1.8.21

OS/platform Android API 34

Additional context The HashMap constructor provided by android sdk34 seems to check the existing map's size when it is passed in.

tanderson-ld commented 9 months ago

Thank you @gzhu for bringing this to our attention. It has been filed internally as 219990. Have you been able to find a workaround?

gzhu commented 9 months ago

Yes, I was able to find a workaround of just creating an entirely new context: LDContext.builder(ContextKind.DEFAULT, id)

kevincianfarini commented 4 months ago

I'm also seeing this behavior, except with privateAttributes.

 Caused by: java.lang.NullPointerException: Attempt to invoke interface method 'java.lang.Object[] java.util.Collection.toArray()' on a null object reference
      at java.util.ArrayList.<init>(ArrayList.java:191)
      at com.launchdarkly.sdk.ContextBuilder.prepareToChangePrivate(ContextBuilder.java:390)
      at com.launchdarkly.sdk.ContextBuilder.privateAttributes(ContextBuilder.java:337)

For the following code:

        return LDContext.builderFromContext(commonContext)
            .privateAttributes("email", userEmail)
            .build()
kevincianfarini commented 4 months ago

I'm unsure of any possible workaround for this but am happy to hear if anyone has found one.

tanderson-ld commented 4 months ago

Thank you for adding your feedback. I'll dig in to this today and hopefully find a clean fix.

tanderson-ld commented 4 months ago

As it turns out, this issue was already fixed in our Java common code, but the dependency bump didn't propagate into our Android SDK code base. We are in the process of updating our release system to use Github Actions and I will incorporate this with that. A release should come within the next few days.

kevincianfarini commented 4 months ago

I see that version 5.0.5 which fixes this bug was released on Github ~3 hours ago. I'm still seeing the latest version on maven central as 5.0.4. You mentioned that this would be released with an overhaul to your release system. Perhaps there's a bug in publishing?

kevincianfarini commented 4 months ago

Yup, looks like secrets aren't configured properly in the repo perhaps? https://github.com/launchdarkly/android-client-sdk/actions/runs/8193444567/job/22407214215

tanderson-ld commented 4 months ago

@kevincianfarini, yes, we are working through internal configurations to get that resolved and it should be up by EOD.

kevincianfarini commented 4 months ago

Hi. Is there anything I can do to help get 5.0.5 published?

tanderson-ld commented 4 months ago

@kevincianfarini, facing intermittency with integration tests and a signing issue. Sorry for the delay.

tanderson-ld commented 4 months ago

Sorry @kevincianfarini, uncovered another Github Action issue and currently we're after hours. May not be able to keep iterating until Monday due to needing a reviewer before deploying the action changes.

kevincianfarini commented 4 months ago

All good. Have a good weekend

tanderson-ld commented 4 months ago

Got v5.0.5 published. Sorry for all that headache. It should be propagating through the different repository systems.

kevincianfarini commented 4 months ago

Fixed the issue! Thank you so much.

tanderson-ld commented 4 months ago

@gzhu, this issue should be fixed now and available in version 5.0.5. I'm going to close this issue as another user has confirmed the fix for the same issue. Please reopen this issue if it is not fixed for you.