search

pusher / push-notifications-android

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

Make notification reporting more resilient #97

Closed luismfonseca closed 4 years ago

luismfonseca commented 4 years ago

Why?

We had a customer report with the following stack trace:

Fatal Exception: java.lang.IllegalStateException: bundle.getString(BUNDLE_INSTANCE_ID_KEY) must not be null
       at com.pusher.pushnotifications.reporting.ReportingJobService$Companion.fromBundle(ReportingJobService.java:71)
       at com.pusher.pushnotifications.reporting.ReportingJobService.onStartJob(ReportingJobService.java:100)
       at com.firebase.jobdispatcher.JobService$2.run(JobService.java:164)
       at android.os.Handler.handleCallback(Handler.java:873)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loop(Looper.java:201)
       at android.app.ActivityThread.main(ActivityThread.java:6810)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:547)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:873)

I'm not sure when this can happen, but these operations are best-effort, so we should fail silently if something is occasionally wrong for some reason.

luismfonseca commented 4 years ago

@marekoid as discussed in person: the assumption on where the null pointer was, was correct but I've added a test to test it explicitly. With regards to the overly-defensive code, I've remove the extraneous return null for cases we don't expect and added the global try .. catch().

daniellevass commented 4 years ago

Closing this one out as we've decided to keep #98.