pusher / push-notifications-android

Android SDK for Pusher Beams
https://www.pusher.com/beams
MIT License
22 stars 22 forks source link

Proguard issue #115

Closed karmaobserver closed 3 years ago

karmaobserver commented 3 years ago

Hello,

When proguard is enabled, when first time application is launched after installation (only first time, every other time it is working fine) application crashes. I have tried to add in proguard rule as follow but it did not help: -keep class com.pusher.* { ; }

Error is as follow: java.lang.NullPointerException: Parameter specified as non-null is null: method kotlin.jvm.internal.t.e, parameter $this$toMutableSet at kotlin.collections.CollectionsKt___CollectionsKt.toMutableSet(Unknown Source:2) at kotlin.collections.CollectionsKt.toMutableSet(Unknown Source:0) at com.pusher.pushnotifications.internal.ServerSyncProcessHandler.processStartJob(Unknown Source:31) at com.pusher.pushnotifications.internal.ServerSyncProcessHandler.handleMessage(Unknown Source:28) at android.os.Handler.dispatchMessage(Handler.java:107) at android.os.Looper.loop(Looper.java:237) at android.os.HandlerThread.run(HandlerThread.java:67)

I have standard rules for Kotlin in proguard rules as well.

benw-pusher commented 3 years ago

Could you confirm the library version in use? The latest version v1.6.2 includes some changes to Proguard compatibility.

karmaobserver commented 3 years ago

@ben-pusher Thank you for quick response. Yes, I am using lastest version v1.6.2 as follows: implementation("com.pusher:push-notifications-android:1.6.2")

karmaobserver commented 3 years ago

@ben-pusher Any progress with the issue? In case you need some additional info, let me know.

benw-pusher commented 3 years ago

I've run a build with Proguard enabled and haven't been able to replicate the behaviour you describe - I'm continuing to look into this.

karmaobserver commented 3 years ago

It may help, the crash occurs during initialization as in code shown below: (Application starts and just after screen is previewed, crash occurs)

@HiltAndroidApp
class SomeApp : Application() {

    override fun onCreate() {
        super.onCreate()
        initPusher()
    }

    private fun initPusher() {
        PushNotifications.start(applicationContext, getString(R.string.config_pusher_instance_id))
    }
}
reul commented 3 years ago

I suddenly started getting this crash on several apps on March 30, some of them were released back in december and had no history of it.

image

Are you sure it's related to ProGuard?

This is the stacktrace I get.

kotlin.collections.CollectionsKt___CollectionsKt.toMutableSet (CollectionsKt___CollectionsKt.java:1665)
kotlin.collections.CollectionsKt.toMutableSet (CollectionsKt.java)
com.pusher.pushnotifications.internal.ServerSyncProcessHandler.processStartJob (ServerSyncProcessHandler.java:210)
com.pusher.pushnotifications.internal.ServerSyncProcessHandler.handleMessage (ServerSyncProcessHandler.java:491)
android.os.Handler.dispatchMessage (Handler.java:106)
android.os.Looper.loop (Looper.java:214)
android.os.HandlerThread.run (HandlerThread.java:65) 
karmaobserver commented 3 years ago

Without Proguard, application working as intended. But with Proguard crash occurs. But keep in mind, it is only when user installs application and runs for the first time.

buildTypes {
        getByName("debug") {
            applicationIdSuffix = ".debug"
            debuggable(true)
            isMinifyEnabled = false
            isShrinkResources = false
            proguardFiles(getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro")
            signingConfig = signingConfigs.getByName("config")
        }

        create("stage") {
            applicationIdSuffix = ".stage"
            debuggable(false)
            isMinifyEnabled = true
            isShrinkResources = true
            proguardFiles(getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro")
            signingConfig = signingConfigs.getByName("config")
        }

        getByName("release") {
            debuggable(false)
            isMinifyEnabled = true
            isShrinkResources = true
            proguardFiles(getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro")
            signingConfig = signingConfigs.getByName("config")
        }
    }

For some reason, in case I enable Proguard and set debuggable on true, crash does not occur. But that is not the case I want to use for production.

karmaobserver commented 3 years ago

By removing below part of code, application working as intended with Proguard enabled.

 PushNotifications.start(applicationContext, getString(R.string.config_pusher_instance_id))
reul commented 3 years ago

@karmaobserver Yeah, you're absolutely right about first run as well.

reul commented 3 years ago

So I tried each version from 1.4.6 t o 1.6.2, and I can reproduce the crash in all of them FYI.

karmaobserver commented 3 years ago

@ben-pusher Any progress with the issue? This is critical issue for production since it prevents us to use Proguard in production.

benw-pusher commented 3 years ago

I'm still not able to replicate this internally, I wonder if it is related to some Proguard or other app config/behaviour.

karmaobserver commented 3 years ago

@ben-pusher @reul Hey guys, i found out what is the issue. In case following rule in Proguard is used the crash occurs:

-assumenosideeffects class kotlin.jvm.internal.Intrinsics {
    static void checkParameterIsNotNull(java.lang.Object, java.lang.String);
}

-Rule improves some performances, it affects null checks at runtime. That is actually what is crash description about as well.

-How to reproduce crash: Simply create new project with added rule mentioned above for Proguard and crash will be reproduced.

reul commented 3 years ago

Hey, @karmaobserver! I can confirm removing that rule fixes the issue. Thank you so much!

benw-pusher commented 3 years ago

Hi, we pusher a serverside change to stop sending the null values that were triggering this error - instead we now send an empty array. As this is a server side change there are no updates or configuration changes required but you should hopefully see that this error is resolved. I'll close this out but if you do see the error continue please reopen.