search

wordpress-mobile / WordPress-FluxC-Android

WordPress Network and Persistence layer based on the Flux architecture
GNU General Public License v2.0
57 stars 37 forks source link

NullPointerException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkNotNullParame... #2499

Open sentry-io[bot] opened 2 years ago

sentry-io[bot] commented 2 years ago

Sentry Issue: WOOCOMMERCE-ANDROID-4M2

NullPointerException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkNotNullParameter, parameter <this>
    at kotlin.text.StringsKt__StringsKt.startsWith
    at kotlin.text.StringsKt__StringsKt.startsWith$default(Strings.kt:835)
    at kotlin.text.StringsKt.startsWith$default
    at kotlin.text.StringsKt.startsWith$default
    at kotlin.text.StringsKt.startsWith$default
...
(33 additional frame(s) were not displayed)
nbradbury commented 2 years ago

Crash is happening when stripping order metadata here. Additional stack trace:

 at org.wordpress.android.fluxc.network.rest.wpcom.wc.order.OrderMappingConst.isInternalAttribute$woocommerce_release(OrderMappingConst.kt:9)
    at org.wordpress.android.fluxc.network.rest.wpcom.wc.order.StripOrderMetaData$invoke$1.invoke(StripOrderMetaData.kt:30)
    at org.wordpress.android.fluxc.network.rest.wpcom.wc.order.StripOrderMetaData$invoke$1.invoke(StripOrderMetaData.kt:30)
    at org.wordpress.android.fluxc.network.rest.wpcom.wc.order.StripOrderMetaData$invoke$1.invoke(StripOrderMetaData.kt:30)
    at org.wordpress.android.fluxc.network.rest.wpcom.wc.order.StripOrderMetaData$invoke$1.invoke(StripOrderMetaData.kt:30)
    at kotlin.sequences.FilteringSequence$iterator$1.calcNext(Sequences.kt:171)
    at kotlin.sequences.FilteringSequence$iterator$1.hasNext(Sequences.kt:194)
    at kotlin.sequences.TransformingSequence$iterator$1.hasNext(Sequences.kt:214)
    at kotlin.sequences.FilteringSequence$iterator$1.calcNext(Sequences.kt:169)
    at kotlin.sequences.FilteringSequence$iterator$1.hasNext(Sequences.kt:194)

Update: the crash appears to happen when calling isInternalAttribute here which suggests the WCMetaData.key is null even though it's defined as non-null. Perhaps this GSON limitation is the problem?

ThomazFB commented 2 years ago

Oh, interesting, thanks for the ping @nbradbury. We verified this same problem during the Product Add-ons project a year ago, some metadata does show up with invalid data like null keys sometimes. Considering that any Woo plugin can generate metadata, I believe it would be best if we always expect the worst and don't mark anything coming from the API as non-null, and use the Kotlin null safety features to filter out invalid metadata info. WDYT?

nbradbury commented 2 years ago

always expect the worst and don't mark anything coming from the API as non-null, and use the Kotlin null safety features to filter out invalid metadata info. WDYT?

That would work but it sure complicates things! I'm wondering if setting a default value for the WCMetaData.key would help, eg: @SerializedName("key") val key: String = ""?

ThomazFB commented 2 years ago

I think it's worth trying. The only problem I see is that the issue you posted mentions that default values are also getting ignored by Gson. Maybe they already fixed it?

nbradbury commented 2 years ago

Well 💩 . I read somewhere that the constructor isn't called when the GSON key doesn't exist and was hoping a default value would fix it, but reading further I believe you're right - the initialization is skipped entirely. What a mess.

ThomazFB commented 2 years ago

Yeah, sounds like we're in a bad situation about trusting the Gson lib 😓, I decided to tackle this fix as we discussed here in this PR: https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2500

Let me know if you think this works. Thanks!