Crashes in production app for local notifications

[fauno]

Hi, I've used this package for an app that only shows local notifications and everything works fine, but Play Console is telling me 10% of the users (~1000 people) are experiencing this crash:

java.lang.NumberFormatException: 
  at java.lang.Integer.parseInt (Integer.java:608)
  at java.lang.Integer.parseInt (Integer.java:643)
  at com.dieam.reactnativepushnotification.modules.RNPushNotificationHelper.toScheduleNotificationIntent (RNPushNotificationHelper.java:70)
  at com.dieam.reactnativepushnotification.modules.RNPushNotificationHelper.cancelScheduledNotification (RNPushNotificationHelper.java:501)
  at com.dieam.reactnativepushnotification.modules.RNPushNotificationHelper.cancelAllScheduledNotifications (RNPushNotificationHelper.java:475)
  at com.dieam.reactnativepushnotification.modules.RNPushNotification.cancelAllLocalNotifications (RNPushNotification.java:210)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.facebook.react.bridge.JavaMethodWrapper.invoke (JavaMethodWrapper.java:372)
  at com.facebook.react.bridge.JavaModuleWrapper.invoke (JavaModuleWrapper.java:158)
  at com.facebook.react.bridge.queue.NativeRunnable.run (Native Method)
  at android.os.Handler.handleCallback (Handler.java:808)
  at android.os.Handler.dispatchMessage (Handler.java:101)
  at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage (MessageQueueThreadHandler.java:29)
  at android.os.Looper.loop (Looper.java:166)
  at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run (MessageQueueThreadImpl.java:232)
  at java.lang.Thread.run (Thread.java:784)

I'm running cancelAllLocalNotifications() only once in the code and made sure every notification is created with numeric string ids as the doc says, but I'm not sure why would it want to parseInt() them. Since I can't reproduce it on development I'm not sure how should I debug it. Any ideas? :)

[david-saint]

This is urgent

[fauno]

Finally discovered what was it! Some users had scheduled notifications from an ancient version that used hashes for IDs and they could never be canceled because the cancellation crashed. Not sure if I should submit this as a PR, but here's the patch. It tries to remove the notification but if the parsing fails it only removes it from the persistence database.

--- node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotificationHelper.java.orig	2020-05-13 14:11:17.749444849 -0300
+++ node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotificationHelper.java	2020-05-13 14:17:01.652609184 -0300
@@ -517,11 +517,20 @@
 
     private void cancelScheduledNotification(String notificationIDString) {
         Log.i(LOG_TAG, "Cancelling notification: " + notificationIDString);
+        try {
+          int notificationID = Integer.parseInt(notificationIDString);
 
-        // remove it from the alarm manger schedule
-        Bundle b = new Bundle();
-        b.putString("id", notificationIDString);
-        getAlarmManager().cancel(toScheduleNotificationIntent(b));
+          // remove it from the alarm manger schedule
+          Bundle b = new Bundle();
+          b.putString("id", notificationIDString);
+          getAlarmManager().cancel(toScheduleNotificationIntent(b));
+
+          // removed it from the notification center
+          NotificationManager notificationManager = notificationManager();
+
+          notificationManager.cancel(notificationID);
+        } catch (NumberFormatException e) {
+          e.printStackTrace();
+        }
 
         if (scheduledNotificationsPersistence.contains(notificationIDString)) {
             // remove it from local storage
@@ -531,11 +540,6 @@
         } else {
             Log.w(LOG_TAG, "Unable to find notification " + notificationIDString);
         }
-
-        // removed it from the notification center
-        NotificationManager notificationManager = notificationManager();
-
-        notificationManager.cancel(Integer.parseInt(notificationIDString));
     }
 
     private NotificationManager notificationManager() {

[Dallas62]

Hi @fauno
Did you tried the latest version ?
This should be fixed

[fauno]

hi @Dallas62 no, i don't usually upgrade to latest versions because it usually means i have to spend several weeks checking if they're still compatible to the whole app, but i did upgrade from 3.1.2 to 3.1.9 last week because if i recall correctly there were some commits i wanted.

anyway i checked the current code as is the same i patched, can you point me to where it should be fixed? the issue was that when i changed to numerical string ids the previous scheduled notifications couldn't be cancelled anymore (i think it was when i upgraded from 2.2.1 to 3.1.2 according to git log, and that was on 2018!). it may be too specific to my case though. it was a known bug to me but only recently users started to report about it and i could get the information required to fix it (play console was showing the exception but not the failing string) :)

[Dallas62]

Actually latest version is close to the initial interface. A breaking change which wasn't identify occurs when a user open the Application. Initially data where directly included in the notification object. Now they are in notification.data.
It has been pointed out here:
#1212 (comment)
I know it can take time to update, but there is a huge bug in version 3.1.9, where there is no channel. So notifications are not triggered... 😄

Changes for this bug is here:
7347b8e
ad3888b

The latest is in case of invalid value (like NaN).

[fauno]

thanks! these notifications are only local though. i'll take a look anyway!

[Dallas62]

This also impact scheduled notifications, I notice that on my Application. iOS is like 🚀 but Android is stuck 👎 after the change, the Android retention is growing again.
There is a last bug with sound, I think it's ok on dev branch but I'm waiting for confirmation.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.