invertase / react-native-notifee

Moved to https://github.com/invertase/notifee
https://invertase.io/blog/open-sourcing-notifee
Other
466 stars 31 forks source link

NullPointerException in app.notifee.core.NotificationAlarmReceiver.onReceive #330

Closed mars-lan closed 3 years ago

mars-lan commented 3 years ago

Start seeing the following crash in production after enabling AlaramManager-based trigger introduced in 1.9.0.

Caused by java.lang.NullPointerException
Attempt to invoke virtual method 'java.lang.String android.os.BaseBundle.getString(java.lang.String)' on a null object reference

n.o.t.i.f.e.e.nZ_e.nz_n (SourceFile:20)
app.notifee.core.NotificationAlarmReceiver.onReceive (SourceFile:1)
android.app.ActivityThread.handleReceiver (ActivityThread.java:3390)
android.app.ActivityThread.-wrap18
android.app.ActivityThread$H.handleMessage (ActivityThread.java:1780)
android.os.Handler.dispatchMessage (Handler.java:105)
android.os.Looper.loop (Looper.java:164)
android.app.ActivityThread.main (ActivityThread.java:6938)
java.lang.reflect.Method.invoke (Method.java)
com.android.internal.os.Zygote$MethodAndArgsCaller.run (Zygote.java:327)
com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1374)

So far the number is low and limited to Android 7 & 8 only. The app was also 100% in background when it crashed.

mikehardy commented 3 years ago

Hey @mars-lan - no crash is good, sorry for that - however I thought you had test-integrated this already? Curious if this just started popping up now because you have a larger sample size or if there is something between the version you test-integrated (which was built pre-merge of the PR) and the version we released as 1.9.0

Investigating either way

mars-lan commented 3 years ago

The crash seemed random and wasn't reproducible locally. We've only just rolled out 1.9.0 to replace 1.9.0-alpha.0 so haven't seen any crahses with that version so far. Will report back in a few days.

mikehardy commented 3 years ago

Appears obvious in hindsight, as most NullPointerExceptions do. Even with the obfuscation (a necessary proprietary thing though it is irritating, yes!?) the stack trace pointed it out clearly thanks.

The error in the code appears quite clear but I have not reproduced it personally yet. I believe the use case is a notification an alarm that belongs to the app but is not a notifee notification is tapped alarm, and it triggers. Since it has no Bundle extras (where the Notifee items live) we crash. Ironically, we crash while attempting to test if it's a notifee notificationalarm, right before we just return if it is not. Solution is to check for null or (!null && no-notifee-key) and return.

The PR to fix it is in the core module and while I can build these things I've never actually published Notifee and don't imagine I can do so more quickly than @helenaford so I'll post the PR then kindly ask her to launch 1.9.1 :-). If for whatever reason that takes an unreasonable amount of time I'll go for publishing after the PR is reviewed / merged.

mikehardy commented 3 years ago

To be clear, I don't intend this as a solution and you may not even want to try this, however I promote the ability for anyone to test as a self-service thing.

So here's the AAR built with my PR in it in case you hit a local reproduction case that's obvious and want to verify it's fixed in a controlled way prior to merge+release. Just strip the .zip I added on the end to make it palatable to github

notifee_core_release.aar.zip

helenaford commented 3 years ago

Ah thanks @mars-lan for reporting it. @mikehardy thanks for the p/r. Feel free to publish if you can do it sooner than tomorrow. It's currently midnight here in the UK 😆🙈

mikehardy commented 3 years ago

I'm certain you'll produce a more supportable artifact tomorrow morning, it's evening here and I need to shut down for my night so you'll beat me to it.

helenaford commented 3 years ago

Fix published 1.9.1 🙏 I think we should reopen another issue for Android 8 & 7 to investigate why this happened 😞