aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.43k stars 2.12k forks source link

Receiving Notifications in Android 31+ fails because of PendingIntent Flag in RNPushNotificationHelper #10252

Closed janal3 closed 1 year ago

janal3 commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

Push Notifications

Amplify Categories

notifications

Environment information

Device: Samsung Galaxy S21 Android 12 TargetSdk 31

Describe the bug

If I receive a notification in Android 12 (version 31 and above) while the app is in the background I get an exception and the app fails to display the notification.

08-23 10:08:55.709 13341 13341 I RNPushNotificationHelper: sendNotification: Intent { cmp=com.aurora_beta/com.amazonaws.amplify.pushnotification.modules.RNPushNotificationBroadcastReceiver (has extras) }

08-23 10:08:55.713 13341 13341 E RNPushNotificationHelper: java.lang.IllegalArgumentException: com.aurora_beta: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.

Expected behavior

Pinpoint notifications work fine on Android 12.

Reproduction steps

Additional information and screenshots

Works when Flag of PendingIntent in RNPushNotificationHelper is set to FLAG_IMMUTABLE or FLAG_MUTABLE https://github.com/aws-amplify/aws-sdk-android/pull/2729

bnviki commented 2 years ago

+1 this is a blocker, following is the error from logcat

RNPushNotificationHelper: failed to send push notification RNPushNotificationHelper: java.lang.IllegalArgumentException: com.follop.app: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent. RNPushNotificationHelper: Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles. RNPushNotificationHelper: at android.app.PendingIntent.checkFlags(PendingIntent.java:382) RNPushNotificationHelper: at android.app.PendingIntent.getBroadcastAsUser(PendingIntent.java:673) RNPushNotificationHelper: at android.app.PendingIntent.getBroadcast(PendingIntent.java:660) RNPushNotificationHelper: at com.amazonaws.amplify.pushnotification.modules.RNPushNotificationHelper.sendToNotificationCentre(RNPushNotificationHelper.java:318) RNPushNotificationHelper: at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService.sendNotification(RNPushNotificationMessagingService.java:110) RNPushNotificationHelper: at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService.handleFCMMessagePush(RNPushNotificationMessagingService.java:90) RNPushNotificationHelper: at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService.access$000(RNPushNotificationMessagingService.java:39)

mtsymlov commented 2 years ago

Hi guys!

I have same problem on any Android 12 :(

Logs

failed to send push notification
java.lang.IllegalArgumentException: com.schoolymobile: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
    at android.app.PendingIntent.checkFlags(PendingIntent.java:375)
    at android.app.PendingIntent.getBroadcastAsUser(PendingIntent.java:645)
    at android.app.PendingIntent.getBroadcast(PendingIntent.java:632)
    at com.amazonaws.amplify.pushnotification.modules.RNPushNotificationHelper.sendToNotificationCentre(RNPushNotificationHelper.java:318)
    at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService.sendNotification(RNPushNotificationMessagingService.java:110)
    at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService.handleFCMMessagePush(RNPushNotificationMessagingService.java:90)
    at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService.access$000(RNPushNotificationMessagingService.java:39)
    at com.amazonaws.amplify.pushnotification.RNPushNotificationMessagingService$1.run(RNPushNotificationMessagingService.java:65)
    at android.os.Handler.handleCallback(Handler.java:938)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:201)
    at android.os.Looper.loop(Looper.java:288)
    at android.app.ActivityThread.main(ActivityThread.java:7842)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

Dependencies

"@aws-amplify/pushnotification": "4.3.24",
"aws-amplify": "4.3.27",
"aws-amplify-react-native": "6.0.5",
"react-native": "0.68.2"
mtsymlov commented 2 years ago

I've rollbacked to previous for me version of React Native — 0.67.4, and it's work fine again. But I'm sad without upgrade React Native versions 🙁

abdallahshaban557 commented 2 years ago

Hello everyone, we are aware of this issue, and we have some refactoring work for our push notifications category that we believe will resolve these issues with the newer versions of Android.

delighteckv commented 2 years ago

When can we expect this?

ChrisLFieldsII commented 2 years ago

I went through a lot trying to get amplify pushnotification to work w/ android 12.

i left my patch on this issue here

Once you get past this IMMUTABILITY flag error, you then (on android 12/targetSdk 31) end up running into another error due to a notification trampoline.

It will not show in local dev though!!! Beware. You have to toggle this behavior on which most devs will not be aware of and wonder why it works locally but PMs say it doesnt work when the app is built XD

tannerabread commented 2 years ago

@delighteckv I'm curious if @ChrisLFieldsII's patch helps you while we are waiting for the refactor of the push notifications category?

jim-jigx commented 2 years ago

I've implemented a similar patch and it seems to resolve the issue. This seems to have happened after we upgraded to RN 0.68.

tannerabread commented 2 years ago

@jim-jigx If that's the case, do you know if Fabric was enabled when upgrading?

jim-jigx commented 2 years ago

@jim-jigx If that's the case, do you know if Fabric was enabled when upgrading?

Not to my knowledge. We have newArchEnabled=false in our gradle.properties. I hope that answers your question.

markl-vesper commented 2 years ago

I also have implemented @ChrisLFieldsII patch to resolve the issue. Great work and thanks for helping out the community!!

ChrisLFieldsII commented 2 years ago

I also have implemented @ChrisLFieldsII patch to resolve the issue. Great work and thanks for helping out the community!!

Glad it could help!

I just updated the comment i left the patch on to include the updates I made for our projects MainActivity.kt (yours might be MainActivity.java) to handle the notification bundle that is attached to the notification PendingIntent.

One requirement that didn't work for us out-of-the-box was deep linking from cold start (the app would just open on home), but the changes I made in our MainActivity.kt seemed to resolve this for us by making sure onNotificationOpened is always emitted on app open so our JS side can handle deep linking.

markl-vesper commented 2 years ago

Hi @ChrisLFieldsII - Yes mine is MainActivity.java and unfortunately the additonal changes to MainActivity.java actually break our build. I suspect your using Expo whereas we are not.

Anyways the main issue I am facing now seems to be more of an intermittent receiving of the PN's at the O/S level but I suspect it might be some AWS limits on the sandboxed Pinpoint applications vs PROD that limit sends per day (they certainly do for emails) so I'm going to carry on testing in PROD as we have some test devices and user accounts I can play with.

At first I though it was an Android 9 issue but soon worked out it wasn't as the android 9 emulator did end up receiving some and then on another test an Android 9 physical device also got one (but one only) but certainly right now its VERY intermittent.

My only change to your patch was to put back in the conditional check on !isForeground as we have a seperate Appsync in app notification when the app is open

What fun this is ;)

delighteckv commented 2 years ago

@tannerabread Verified the patch works fine on Android 12(API 31).

Thanks for contribution @ChrisLFieldsII

markl-vesper commented 2 years ago

I tested again on Android 9 device emulator and what I found was that initially no PN came thru so I went and heated up lunch and 5 mins later when I got back they had arrived. Android 12 device emulator seems to either receive them immediately from google servers or process them immediately and send on to app

agNishith commented 1 year ago

Any updates on this issue? We are stuck at the notification part in our app due to this.

bonnmh commented 1 year ago

        "@amplitude/react-native": "^2.4.0",
        "@aws-amplify/pushnotification": "^5.0.5",
        "aws-amplify": "^5.0.5",
        "react": "17.0.1",
        "react-native": "0.64.4",

I got app crash when trying to run android file (this came after I installed @aws-amplify/pushnotification) , I get below error code can someone tell me what should I do next

Error: INSTALL_PARSE_FAILED_MANIFEST_MALFORMED: Failed parse during installPackageLI: /data/app/vmdl550519623.tmp/base.apk (at Binary XML file line #165): com.dieam.reactnativepushnotification.modules.RNPushNotificationBootEventReceiver: Targeting S+ (version 31 and above) requires that an explicit value for android:exported be defined when intent filters are present'

ChrisLFieldsII commented 1 year ago

@bonnmh

This is whats causing the crash.

Targeting S+ (version 31 and above) requires that an explicit value for android:exported be defined when intent filters are present

Here is a reference to the fix on stackoverflow: https://stackoverflow.com/a/70333589/5434172

In short, you need to add android:exported="true|false" to your activity due to a change Android made in Android 12.

Whether the value is true or false depends on your needs. Reference the official android docs on android:exported here: https://developer.android.com/guide/topics/manifest/receiver-element.html#exported

Hope that helps.

tannerabread commented 1 year ago

Thank you @ChrisLFieldsII, while we are still working on the root cause of this issue, the docs were updated today (12/9) to show the updates with android:exported, I appreciate the comment helping others

dylan-westbury commented 1 year ago

Any update on a fix?

tannerabread commented 1 year ago

@tannerabread unfortunately it fails (rn 0.67.5 ) because of PendingIntent Flag in RNPushNotificationHelper without patch mentioned above, does not fail with patch, onRegister is not called at all.( 0.67.4 does not work so version 0.67.5 is special release from fb team targeted to fix android issue ) I know that onRegister is called only once if device token is already assigned. I'have separate postman call to amazon pin point and I've did next steps

cleaned all enpoints by my userId (sub), so If I call get those endpoints, I have empty result. run app on real android device with packets mentioned above. call endpoint and look what happens on amazon side, and I have in response configuration object which miss address, wrong optOut and channel type { "EndpointStatus": "ACTIVE", "OptOut": "ALL", "RequestId": "9827c1c8-a9ce-445c-8835-2b7800e5681f", "Location": {}, "Demographic": { "ModelVersion": "31", "AppVersion": "android/31", "Platform": "android" }, "EffectiveDate": "2023-01-27T18:49:30.736Z", "Attributes": {}, "Metrics": {}, "User": { "UserId": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" }, "ApplicationId": "xxxxxxxxxxxxxxx", "Id": "xxxxxxxxxxxxxxxxxxxx", "CohortId": "26", "CreationDate": "2023-01-27T10:48:57.777Z" } Is there a way to get this token directly from react native js code ?, so I can make a workaround

@LYevhen, I wanted to bring the conversation about Android SDK 31+ back to the issue it is about. It was pointed out to me that the React Native version is not so important and rather it is with Android 31+ that the PendingIntent Flag starts to show up and the patch listed on the other issue starts being necessary.

I believe your onRegister not firing is a separate issue than either of these at this point, but as far as handling it yourself I wouldn't be able to help with that implementation. Being able to call it repeatedly is a current feature request seen in #9496. If you have used PushNotification.updateEndpoint to add your device token to Pinpoint then the "unofficial way" in the comments there does work to return the token from AsyncStorage, but that wouldn't be officially a supported way to retrieve the token.

We do have major updates coming to the Push Notification category (CC @dylan-westbury) but I don't have an exact timeline, just the knowledge that it is currently in development. This update should solve this issue and some others listed in our repo. On top of that, there is also a patch in the works for this specific issue with the PendingIntent flag

Thank you all for your patience while we improve this category

LYevhen commented 1 year ago

@tannerabread Thank You a lot for Your response. It is first time I have response in this repo, so quick, so valuable. For some reason token was not set to store either. I took it from ReactNative.getToken module, unofficially but it worked for me ( updateEndpoit with sub, token and optOut)

LYevhen commented 1 year ago

@cshfang tested with my config @aws-amplify/pushnotification: ^5.0.12 aws-amplify: ^5.0.12 aws-amplify-react-native: ^7.0.2 react-native: 0.67.5 react: 17.0.2

fix did not help

02-01 23:03:38.317 20610 20610 E RNPushNotificationHelper: failed to send push notification
02-01 23:03:38.317 20610 20610 E RNPushNotificationHelper: java.lang.IllegalArgumentException: com.app.name: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
02-01 23:03:38.317 20610 20610 E RNPushNotificationHelper: Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
cshfang commented 1 year ago

Hi @LYevhen. How did you incorporate the fix into your app? The fix has so far been merged to our main branch but not yet released.

LYevhen commented 1 year ago

@cshfang Maybe I did something wrong but I take a look here https://github.com/aws-amplify/amplify-js/releases/tag/aws-amplify%405.0.12 by this link there is mentioned 2 commits, and one from You with fix for sdk 31 https://github.com/aws-amplify/amplify-js/compare/aws-amplify%405.0.12...main

cshfang commented 1 year ago

Ah. The current released version of aws-amplify is 5.0.12 and although the fix is merged at this version, the release tag still refers to the commit at the time of release. When we release the next version of aws-amplify (e.g. 5.0.13) the fix should be picked up and the package version bumped.

As we don't have a tag for every commit that gets merged, the options to consume the fix will have to be to wait for a release, patch your local installed version of the module or use a local npm registry like Verdaccio to do a local version bump yourself.

LYevhen commented 1 year ago

@cshfang My appologies, I interpreted it wrong. Thanks for details

tannerabread commented 1 year ago

This fix is released! I tested it on my project and it seems to be working as expected

@LYevhen can you try aws-amplify: 5.0.14 and @aws-amplify/pushnotification: 5.0.14 and verify that it works for you as well without the patch submitted by @ChrisLFieldsII

LYevhen commented 1 year ago

@tannerabread I've reverted all git patches, changes to MainActivity.java, removed all node modules, updated package.json with mentioned versions 5.0.14, successfully received push notification on my android device. Thanks

tannerabread commented 1 year ago

Thank you for testing it so quickly @LYevhen!

I will close this now but if anyone runs into this problem and the fix in 5.0.14 doesn't work, comment here so we can investigate further