-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The app crashes when an exception occurs inside OneSignal in SDK 4.4.1+ #1444
Comments
@ushi3 Thanks for reporting with all this detail. Could you also include the stacktraces of the crashes you are seeing? We would like to know which exceptions we should be correctly handling instead catching everything here. |
OK, Below is examples we've receiving.
|
According to our findings, when calling By the way, I think the implementation of androidx.Worker will be helpful us. Worker catches all exceptions and marks them as Failed. |
Maybe, fixed by v4.6.3 🎉 🎉 🎉 |
@ushi3, please reopen or comment if you're having any more issues. I closed it for now with our new release. |
Description:
In version 4.4.1 and above, when OneSignal receives a push notification, an exception that occurs within OneSignal SDK to crashes the app.
We have updated the SDK from 4.4.0 to 4.6.0 and released the app. Since then, we've been receiving a lot of crash reports.
After investigating the cause, it was found that it was due to the changes of #1356. And SDK containing it was released in version 4.4.1.
What's wrong with #1356?
TLDR;
After 4.4.1, The thread being OneSignal notification process is not catching the exception properly.
In #1356, implementation of
OSNotificationWorkerManager.NotificationWorker
changedWorker
toListenableWorker
.Worker
'sdoWork()
runs on worker-thread, whileListenableWorker
'sstartWork()
runs on main-thread.That difference causes a change in the subsequent processing. (probably not intended)
Both processes call
processNotificationData()
, and then reachesOSNotificationReceivedEvent.complete()
.OSNotificationReceivedEvent.complete()
is the part that causes this issue.Here exist code that if call on main-thread, execute process on other thread. code
Below 4.4.1, this if statement is
false
. So, continue processing on current-thread (= worker-thread).After 4.4.1, this statement is
true
, run in other thread made on instant.If the method
OSNotificationReceivedEvent.processNotification()
is execute on worker-thread, the exception thrown here will be caught byWorker.startWorker()
. However, if the method is execute on instant thread, the exception will not be caught. App crashes when actual exception thrown.Do you really throw some exception in this method? to anwer YES.
Many of the exceptions are caused by Android NotificationManager API and device platform bugs.
For example, on some devices such as Samsung, LG, the system notification service was killed (reason don't know why), NotificationManager throw DeadSystemException.
Environment
Device OS : Android 7.0, 7.1.1, 8.0, 8.1 (at least)
OneSignal SDK : 4.4.1+
Steps to Reproduce Issue:
Confirm that execution-thread has changed around 4.4.1 before-after.
OSNotificationReceivedEvent.complete()#L71~
Looper.getMainLooper().isCurrentThread()
false
, above 4.4.1 istrue
It is difficult to raise an exception, but you can see the app behavior by throw exception using the debugger during this process.
Anything else:
A simple fix is to try-catch exception into the Runnable at
OSNotificationReceivedEvent.complete()#L81-88
.The text was updated successfully, but these errors were encountered: