universal-tools / UTNotificationsFeedback

7 stars 0 forks source link

Crash experienced for Notification on Android 8+ with targetSDKVersion as 26 #90

Open ghost opened 5 years ago

ghost commented 5 years ago

Hi,

We have upgraded our UTNotifications plugin to 1.7.3 and currently, we are seeing an increasing crash after upgrading both UTNotifications(to 1.7.3) and Android targetSDKVersion to 26.

Attaching the crash log as follows: java.lang.RuntimeException: at android.app.ActivityThread.handleReceiver (ActivityThread.java:3399) at android.app.ActivityThread.-wrap18 (Unknown Source) at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1780) at android.os.Handler.dispatchMessage (Handler.java:105) at android.os.Looper.loop (Looper.java:164) at android.app.ActivityThread.main (ActivityThread.java:6944) at java.lang.reflect.Method.invoke (Native Method) at com.android.internal.os.Zygote$MethodAndArgsCaller.run (Zygote.java:327) at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1374) Caused by: java.lang.IllegalStateException: at android.app.ContextImpl.startServiceCommon (ContextImpl.java:1538) at android.app.ContextImpl.startService (ContextImpl.java:1484) at android.content.ContextWrapper.startService (ContextWrapper.java:663) at android.content.ContextWrapper.startService (ContextWrapper.java:663) at android.support.v4.content.WakefulBroadcastReceiver.startWakefulService (WakefulBroadcastReceiver.java:89) at com.google.android.gms.gcm.GcmReceiver.zzi (Unknown Source) at com.google.android.gms.gcm.GcmReceiver.onReceive (Unknown Source:98) at android.app.ActivityThread.handleReceiver (ActivityThread.java:3392)

When I went into the source code of UTNotifications library for android, I found that the method PendingIntent.getService() is used. Should it be replaced with PendingIntent.getForegroundService() ?

yuriy-universal-ivanov commented 5 years ago

Hi @myselfAB10 ,

PendingIntent.getForegroundService is to be used only for foreground services, i.e. services that show a notification WHILE they are running (like a download job and similar stuff). Unlike Context.startService, PendingIntent.getService doesn't require from the app to be in foreground.

And of course we tested receiving push and local notifications for apps being in background and foreground (and closed altogether), including in Android 8 and 8.1: it works totally fine in UTNotifications 1.7.3/1.7.4.

You can also notice that in the stack trace there is no UTNotifications code, just FCM/GCM stuff. I'm afraid I don't know the reason of this error. Do you know any way to reproduce it? At least, could you maybe check what devices are affected and what specific versions of Android they're running?

Thank you and best regards, Yuriy, Universal Tools team.

ghost commented 5 years ago

Hi @yuriy-universal-ivanov ,

As per the guidelines of Android, IntentService is not recommended to use anymore. Instead, it should be replaced with JobIntentService. And that is the reason why I was inquiring about the PendingIntent.getForegroundService. And in the log, there is no UTNotifications. I agree to that but only UTNotifications library uses the FCM/GCM. Nowhere else, this is used. And when you happen to click on a notification, an IntentService is started. So going by that, is getForegroundService completely unnecessary? Also, are there any specific reasons that the IntentServices have not been replaced?

Also, just an enquiry, is FCM still to be implemented in UTNotifications 1.7.3? Because I still see the GCM code there. The process of receiving notifications and registering tokens are done through GCM. Would you be rolling out an update for the same? as GCM will not be available to use from April, 2019.

yuriy-universal-ivanov commented 5 years ago

Hi @myselfAB10 ,

The problem with JobIntentService is that it can significantly delay executing a job, so a user can see a push notification too late. We might make it configurable in the future whether to use IntentService or JobIntentService. Still, getForegroundService absolutely doesn't work for notifications. It shows a temporary "work in progress"-like app notification, but not a one that informs a user on some news/updates in your app. As soon as the services finishes its work, it hides the temporary "work in progress" notification. See also https://developer.android.com/guide/components/services#Foreground:

Caution: Limit your app's use of foreground services.

You should only use a foreground service when your app needs to perform a task that is noticeable by the user even when they're not directly interacting with the app. For this reason, foreground services must show a status bar notification with a priority of PRIORITY_LOW or higher, which helps ensure that the user is aware of what your app is doing. If the action is of low enough importance that you want to use a minimum-priority notification, you probably shouldn't be using a service; instead, consider using a scheduled job.

Every app that runs a service places an additional load on the system, consuming system resources. If an app tries to hide its services by using a low-priority notification, this can impair the performance of the app the user is actively interacting with. For this reason, if an app tries to run a service with a minimum-priority notification, the system calls out the app's behavior in the notification drawer's bottom section.

Best regards, Yuriy, Universal Tools team.

yuriy-universal-ivanov commented 5 years ago

@myselfAB10 One idea here: does the push notification you're sending have a high priority (you can configure it in our notification profiles settings in Unity)? As it might be an issue with Android 8+ indeed, if a low-priority notification invokes a background service...

ghost commented 5 years ago

Hi @yuriy-universal-ivanov , Thanks for providing the detailed description. It was indeed helpful. Coming to the bug, I looked into our Notification profile. It was not sending as High Priority. So, that might be an issue. Also, according to Google, FCM combined with High priority notifications shouldn't be an issue anymore for the background notifications. And this log was taken from playstore. And unfortunately, there are not any exact reproducible steps. The notifications were tested even from our end. We'll move to high priority notifications to see if it works.

Coming to the FCM Migration, in the file called GcmInstanceIDListenerService, UTNotifications is using the InstanceID to get the token. And that InstanceID is actually part of the google-play-services-gms library. And we are using the play services version of 11.8.0. So, in short, we are even getting the token from GCM. I just want to know how you can claim to be using FCM.

Reference Screenshot:

screen shot 2018-11-14 at 6 46 34 pm

And InstanceID even got deprecated as per the Google developer docs.

In the FCM Migration Guide , it claims that all GCM client and server APIs will be unusable. Still, don't we need to change anything on that? Even in this guide Link , it says to use FCM or JobIntentService for token refreshes. But UTNotifications still happen to be using IntentService for token refreshes. It would be very helpful if you provide some clarity on the above.

Also, for the notification handling, the following class GcmIntentService is used. Reference ScreenShot:

screen shot 2018-11-14 at 6 50 33 pm

Even this is using the GCM. And again, as per the docs, this is likely to be un-usable from April, 2019.

Lastly, for the changes in AndroidManifest.xml, shall we consider removing the GCM specific intent filters? In the update, there were no changes mentioned for the AndroidManifest. Do we need to change anything on that? Reference screenshot:

screen shot 2018-11-14 at 6 57 12 pm

Requesting to clarify on these above so that we can have a clear-cut idea on what needs to be implemented.

yuriy-universal-ivanov commented 5 years ago

Hi @myselfAB10 ,

So, in short, we are even getting the token from GCM. I just want to know how you can claim to be using FCM.

It's simple: in fact all GCM classes work absolutely fine with FCM servers: it's based on the sender id - if it belongs to FCM, GCM classes work with FCM. So UTNotifications really work with FCM, not GCM, though still using old classes. Don't worry for your applications: old GCM API will remain functioning even after April 2019, though it might get incompatible with newer versions of Google Play Services Libs if you choose to use them in your project instead of current versions. We're gonna release an update working only with FCM classes soon, but as it requires some functionality not supported by old versions of Unity Android build system (5.x) we intentionally didn't introduce it earlier to support people using old versions of Unity. Maybe we'll find a way to make it work with Unity 5 as well, but so far we failed to.

I hope it helps.

ghost commented 5 years ago

Hi @yuriy-universal-ivanov , Thanks for the clarification.

it's based on the sender id - if it belongs to FCM, GCM classes work with FCM

How to be sure if the sender id is using FCM in this case? I checked our code. In the SettingsEditor, we are actually sending the sender id.

old GCM API will remain functioning even after April 2019, though it might get incompatible with newer versions of Google Play Services Libs.

Again for this, it means that we won't be able to update the play services lib, as long as we don't receive an update from the UTNotifications, right?

yuriy-universal-ivanov commented 5 years ago

How to be sure if the sender id is using FCM in this case? I checked our code. In the SettingsEditor, we are actually sending the sender id.

If it was received from Firebase Console in google-services.json file, it's FCM.

Again for this, it means that we won't be able to update the play services lib, as long as we don't receive an update from the UTNotifications, right?

Likely so, yes.

Nyankoo commented 5 years ago

I'm getting the same crash on the Google Play Console:

java.lang.RuntimeException: at android.app.ActivityThread.handleReceiver (ActivityThread.java:3197) at android.app.ActivityThread.-wrap17 (Unknown Source) at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1675) at android.os.Handler.dispatchMessage (Handler.java:106) at android.os.Looper.loop (Looper.java:164) at android.app.ActivityThread.main (ActivityThread.java:6518) at java.lang.reflect.Method.invoke (Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:438) at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:807) Caused by: java.lang.IllegalStateException: at android.app.ContextImpl.startServiceCommon (ContextImpl.java:1521) at android.app.ContextImpl.startService (ContextImpl.java:1477) at android.content.ContextWrapper.startService (ContextWrapper.java:650) at android.content.ContextWrapper.startService (ContextWrapper.java:650) at com.google.android.gcm.GCMBaseIntentService.runIntentInService (GCMBaseIntentService.java:282) at com.google.android.gcm.GCMBroadcastReceiver.onReceive (GCMBroadcastReceiver.java:55) at android.app.ActivityThread.handleReceiver (ActivityThread.java:3190)

Was the above mentioned update using only FCM already released?

ghost commented 5 years ago

Hi @Nyankoo , So, the crash was happening from GCM Registration. We made some custom changes to the library(basically migrating the GCM client calls with FCM calls) and the crash was gone after that. Also, we made the targetSDKVersion to 27.

Nyankoo commented 5 years ago

Hi @myselfAB10 , thank you for writing back! which libraries did you change and would it be possible to take a look at your changes, or even post them here so @yuriy-universal-ivanov could maybe include them in an official release?

yuriy-universal-ivanov commented 5 years ago

@myselfAB10 @Nyankoo ,

UTNotifications 1.8 is using FCM-only libraries, and supports the very latest versions of Play Services Libs and Android Support library (and also Unity 2019.1.2f1, Android 10 (Q) preview, Play Services Resolver and other latest things). It's still in beta though, as we're testing lots of configurations and use cases, but it's pretty stable on most configurations and environments (much better than 1.7.4 with latest Unity and other components). You can send a copy of your Asset Store purchase receipt to universal.tools.contact@gmail.com and I'll send you the latest 1.8.beta build.

Best regards, Yuriy, Universal Tools team.