OneSignal-Android-SDK icon indicating copy to clipboard operation
OneSignal-Android-SDK copied to clipboard

Fix HMS register crash on old Huawei Devices

Open alirezaafkar opened this issue 3 years ago β€’ 4 comments

After updating to the recent version of OneSignalSDK, we have some issues on old Huawei phones. I found the problem and reason for the error is that you have ApiException before checking HMS libraries on device. Thus I moved this part of code, and the error has resolved.

For reproducing the crash call this method directly on old Huawei devices(Without HMS).

new PushRegistratorHMS().registerForPush(this, "123", new PushRegistrator.RegisteredHandler() {
        @Override
    public void complete(String id, int status) {

    }
});

com.onesignal.PushRegistratorHMS.registerForPush (PushRegistratorHMS.java:35) com.onesignal.OneSignal.registerForPushToken (OneSignal.java:1002) com.onesignal.OneSignal.access$1800 (OneSignal.java:86) com.onesignal.OneSignal$5.complete (OneSignal.java:1075) com.onesignal.OneSignalRemoteParams.processJson (OneSignalRemoteParams.java:188) com.onesignal.OneSignalRemoteParams.access$100 (OneSignalRemoteParams.java:12) com.onesignal.OneSignalRemoteParams$1.onSuccess (OneSignalRemoteParams.java:140) com.onesignal.OneSignalRestClient$5.run (OneSignalRestClient.java:270) java.lang.Thread.run (Thread.java:841)

This change is Reviewable

alirezaafkar avatar Sep 28 '20 09:09 alirezaafkar

@jkasten2 I've attached an image from Crashlytics that shows crashes on our users' device. About logic, I think the crash cause is ApiException; You use it before checking HMS libraries on the user's device. Yes, getHMSTokenTask is private, but I changed it to the public for testing in the example project.

photo_2020-10-01_11-20-50

alirezaafkar avatar Oct 01 '20 08:10 alirezaafkar

@alirezaafkar All HMS API being called in com.onesignal.PushRegistratorHMS.registerForPush is wrappered in a try-catch with ApiException exception so it isn't possible for this specific exception to create a crash.

Is the stacktrace you reported here from your device? Can you extract the full stacktrace with the exact exception from the Google Play console to ensure it is the same one? Or from any other crash reporting tool you are using the track production crash.

jkasten2 avatar Oct 07 '20 19:10 jkasten2

why don't you just test it? I've fixed it in my fork and we've no more crashes.

alirezaafkar avatar Oct 08 '20 06:10 alirezaafkar

@alirezaafkar The devices in your screenshot are devices from around 2014 it seems. We can get one of these devices to test with but don't have one on hand so this will delay this PR. However if you can provide the exception name and message it should provide the details we need to catch it and take the correct action. Your original stacktrace is missing the first few lines where this would normally be included.

jkasten2 avatar Oct 30 '20 00:10 jkasten2

Closing due to no other reports of this issue, as well as the age of the devices. Closing this PR as stale.

jkasten2 avatar Jan 17 '24 17:01 jkasten2

@jkasten2 Good job! πŸ‘πŸ‘πŸ‘

moallemi avatar Jan 17 '24 18:01 moallemi