expo / expo

An open-source framework for making universal native apps with React. Expo runs on Android, iOS, and the web.
https://docs.expo.dev
MIT License
40.19k stars 6.93k forks source link

expo-notifications FCM V1 Migration Tracking Issue #28656

Closed christopherwalter closed 7 months ago

christopherwalter commented 1 year ago

This is a tracking issue for user-reported issues related to the Spring 2024 migration of Expo Push Notifications from FCM Legacy to FCM V1 for Android devices. There have been reported instances of brokenness in production when moving from FCM Legacy to FCM V1, and this issue will serve as a central place to track our investigation and resolution of these issues.

Recommend that users should now be on expo@51.0.24 and expo-notifications@0.28.15 or later.

The remaining issues (as of 2024-08-06) are as follows:

Fixed issues:

The deadline for migrating to FCM V1 is 2024-07-22. See this link for up to date info from Firebase on the legacy API shutdown process.

Update 2024-09-08: It appears that the FCM legacy service is now fully shut down. Later this week, we intend to close this issue and lock comments. Any remaining or new issues should be created as new issues in this repo, per the usual process.

fdelu commented 1 year ago

Something else I noticed that might be worth noting is that notification images (not icons) don't seem to be working when the app is in the foreground.

Edit: Just to clarify, I tested this by sending the request to the FCM V1 api directly

haplionheart commented 1 year ago

To add on to "the notification icon is not working", on Android I'm finding that the icon displays when the app is in the foreground. But I get a blank circle as an icon when the app is in the background

This is happening with a development build

jnoleau commented 1 year ago

No issue with the icon when we add the correct name/value pair on the Android manifest but it seems there is no "plugin" on the package expo-notification or this one is not working as expected.

But this issue can be fixed "simply" by adding a manual plugin specified here https://github.com/expo/expo/issues/24844#issuecomment-2011235153 in addition to the config.notification.icon in the app.config

zandvakiliramin commented 1 year ago

Hi, I am facing the same issue, I am using expo-server-sdk: 3.10.0 to send notifications. When I change the useFcmV1 to true on Android, my app does not get the data from the notification, and the icon shows a blue circle with thick borders. When I set it to false, everything works. On iOS, everything works fine.

I am using Expo SDK 49.

krazykriskomar commented 1 year ago

#24844 (comment)

For me, the plugin here is unnecessary because the AndroidManifest that's created seems fine. I have 2 notification_icon additions. <meta-data android:name="com.google.firebase.messaging.default_notification_icon" android:resource="@drawable/notification_icon"/> and <meta-data android:name="expo.modules.notifications.default_notification_icon" android:resource="@drawable/notification_icon"/> I also see mapping files declaring the png and naming it to the correct asset name. I am on expo 51.0.2 and expo-notifications 0.28.1 (both latest). On prebuild, the png file is processed and correctly placed into all the xdpi drawable asset out put folders. I've also triple checked with our UX designer that the requirements are met: it's a white-only transparent PNG at 72 dpi at 96x96 pixels exactly. Hopefully this info helps someone. I think there's been a mismatch of actual problems going on between expo 49, 50, and 51, because the manifest is now correct and I no longer need the withFirebaseMessagingNotificationIcon plugin.

NickTillinghastKrazy commented 1 year ago

Something else I noticed that might be worth noting is that notification images (not icons) don't seem to be working when the app is in the foreground.

Edit: Just to clarify, I tested this by sending the request to the FCM V1 api directly

I have the same issue on IOS.

douglowder commented 1 year ago

PR #28883 has been submitted to fix the Android notification response issues when app is in background or not running.

briazzi64 commented 1 year ago

PR #28883 has been submitted to fix the Android notification response issues when app is in background or not running.

Awesome. thank you so much for tracking this down for all of us. Any idea when it will be available?

krazykriskomar commented 1 year ago

PR #28883 has been submitted to fix the Android notification response issues when app is in background or not running.

Awesome. thank you so much for tracking this down for all of us. Any idea when it will be available?

Second that! Really need it :)

krazykriskomar commented 12 months ago

0.28.2 didn't fix anything :(

fdelu commented 12 months ago

In my case I can confirm that with the new version, useLastNotificationResponse now does get notifications received when the app is in the background (sent with FCM v1) However in that same scenario I'm not getting the full request payload, the data is missing. Trying to debug if it's something I can change on my end to fix it

douglowder commented 12 months ago

@fdelu see the fix that was just added in #28883 , specifically the new method in NotificationSerializer.java, toResponseBundleFromExtras():

https://github.com/expo/expo/pull/28883/files#diff-385373049872662646fd4faf3b47aea7c97480a25b6de9a96259bef1e856d790

That is the method that takes the bundle passed into onNewIntent() when the notification is opened while the app is in the background, and constructs the notification response object.

If you are missing part of the payload, you should have a look to see if there are parts of the bundle that are not being mapped by NotificationSerializer.java. The payload is being logged to logcat in ExpoNotificationsLifecycleListener.java:

https://github.com/expo/expo/pull/28883/files#diff-f558167185b0f06ac4e093b7f22c59f01ca8096f127a1605245962f0927ed7e1

douglowder commented 12 months ago

@fdelu in looking at this further, it may be that you rebuilt your Android app with the new expo-notifications package, but did not rebuild your JS bundle to pick up the required changes on the JS side: https://github.com/expo/expo/pull/28883/files#diff-37b24aeb2e0d5d7c4365c987c51f03db4b6d20a0927f3eefe0d9e0a35b2c4df7

The data is passed as a string from Android native to the JS code, and is unpacked there, so if you did not rebuild your bundle with the new library, the data would indeed be missing.

fdelu commented 12 months ago

@fdelu in looking at this further, it may be that you rebuilt your Android app with the new expo-notifications package, but did not rebuild your JS bundle to pick up the required changes on the JS side: https://github.com/expo/expo/pull/28883/files#diff-37b24aeb2e0d5d7c4365c987c51f03db4b6d20a0927f3eefe0d9e0a35b2c4df7

The data is passed as a string from Android native to the JS code, and is unpacked there, so if you did not rebuild your bundle with the new library, the data would indeed be missing.

Yes, that seems like it. I noticed later that I had "dataString" but not "data" so I'm probably missing that. Not sure how that happened though. Thanks for your help!

deivijt commented 12 months ago

After updating to expo-notifications@0.28.2 and building, this works πŸŽ‰. Thanks, @douglowder. Great work! πŸ‘

... the only minor issue is that the notification icon is not displaying correctly with FCM V1.

douglowder commented 12 months ago

Fix for #28883 has been published. Pick up latest expo package (51.0.7) and do npx expo install --fix to pick up expo-notifications 0.28.2. You will also get a new version of expo-dev-client that fixes an unnecessary reload issue on Android (see #28893).

fdelu commented 12 months ago

@fdelu in looking at this further, it may be that you rebuilt your Android app with the new expo-notifications package, but did not rebuild your JS bundle to pick up the required changes on the JS side: https://github.com/expo/expo/pull/28883/files#diff-37b24aeb2e0d5d7c4365c987c51f03db4b6d20a0927f3eefe0d9e0a35b2c4df7 The data is passed as a string from Android native to the JS code, and is unpacked there, so if you did not rebuild your bundle with the new library, the data would indeed be missing.

Yes, that seems like it. I noticed later that I had "dataString" but not "data" so I'm probably missing that. Not sure how that happened though. Thanks for your help!

An update on this @douglowder: I verified and it is using the latest version.

When my app is in the foreground, I get a notificationd and click on it, I see the "response received" log and everything works fine. However, when my app is closed (fully), I get a notification and click on it to open the app, then I don't see that "response received" log, the "data" field is missing and the request content contains the "dataString" field.

Looking at the code for useLastNotificationResponse, I believe this happens because in the first scenario, the notification comes from the response received listener, which does parse the "dataString" (that's the change from the link you sent me). In the second scenario though, the notification seems to be coming from getLastNotificationResponseAsync which does not parse the "dataString".

Another issue I've been having is that if I get a notification when the app is open, I fully close it and then I reopen the app by clicking the notification, useLastNotificationResponse returns an almost empty notification. My code:

  const notification = useLastNotificationResponse();

  console.log("LAST RESPONSE", notification);
  console.log(
    "LAST RESPONSE CONTENT",
    notification?.notification?.request?.content
  );
  console.log(
    "LAST RESPONSE TRIGGER",
    notification?.notification?.request?.trigger
  );

The logs:

 LOG  response received: {
  "notification": {
    "request": {
      "trigger": {
        "channelId": null,
        "type": "push"
      },
      "content": {
        "title": null,
        "dataString": null,
        "body": null
      },
      "identifier": null
    },
    "date": 0
  },
  "actionIdentifier": "expo.modules.notifications.actions.DEFAULT"
}
 LOG  LAST RESPONSE {"actionIdentifier": "expo.modules.notifications.actions.DEFAULT", "notification": {"date": 0, "request": {"content": [Object], "identifier": null, "trigger": [Object]}}}
 LOG  LAST RESPONSE CONTENT {"body": null, "dataString": null, "title": null}
 LOG  LAST RESPONSE TRIGGER {"channelId": null, "type": "push"}
douglowder commented 12 months ago

@fdelu yes indeed that is a bug! I think cutting and pasting the new code from NotificationEmitter should fix it... I really appreciate you taking the time to track that down.

christopherwalter commented 12 months ago

Hi folks, if you're in this thread due to icon-related issues (@deivijt @zandvakiliramin @haplionheart):

Can you try using a config plugin like in this comment to ensure the correct fields are in your manifest? And let me know if that changes anything?

@krazykriskomar re: your comment here, I wasn't sure if you have it working or not? Can you confirm if icon behavior is correct for you when sending notifs using FCM V1?

douglowder commented 12 months ago

@fdelu you can use patch-package to apply the fix in the PR I just posted (https://github.com/expo/expo/pull/28938) and see if that fixes the issue for you. You will need to rebuild the JS bundle after applying the patch.

krazykriskomar commented 12 months ago

W> Hi folks, if you're in this thread due to icon-related issues (@deivijt @zandvakiliramin @haplionheart):

Can you try using a config plugin like in this comment to ensure the correct fields are in your manifest? And let me know if that changes anything?

@krazykriskomar re: your comment here, I wasn't sure if you have it working or not? Can you confirm if icon behavior is correct for you when sending notifs using FCM V1?

Hi there @christopherwalter ! Thank you for the feedback. Been there, done that, unfortunately. My AndroidManifest has all the right things including a couple ideal items like: <meta-data android:name="com.google.firebase.messaging.default_notification_icon" android:resource="@drawable/notification_icon"/> and <meta-data android:name="expo.modules.notifications.default_notification_icon" android:resource="@drawable/notification_icon"/> Tell me if I'm crazy and that's not what is expected. I'm new to this app dev stuff.

I already tried the plugin that you suggested but nothing changed and furthermore I did not see any change in my prebuild output. I would love to know what is at the core of this stuff as we have multiple bugs centering around this issue.

  1. We cannot currently see our android group notification icon.
  2. We cannot currently see icon images in iOS pushes. This is NOT the notification icon, this is NOT the "notification image". It is a SEPARATE image that appears to the right of the notification itself.

Anything me or my staff can do to help is ideal. I want to get this stuff fixed and in an ideal state. Let's go!!!!!!

christopherwalter commented 12 months ago

@krazykriskomar appreciate the clarification. Right now my plan is as follows: get an icon working for my own test app (I haven't before bothered with icons for test apps, which is why we missed this bug in FCM V1) so I understand how this is supposed to work (I'm also new to mobile, having been in the infrastructure trenches the past 2 years, so we're in this together). Once I get an icon working with FCM Legacy then I can debug FCM V1.

If I'm unable to get a fix for this by early next week, the following will happen: I'll contact folks in this thread and organize zoom calls to debug this together. This is a last resort for obvious reasons, but given Google's deadline for FCM V1 on June 20th we need to nail this down via any means necessary.

So, I'll report back here on Monday (or sooner!) with an update. Thanks everyone for your continued patience!

fdelu commented 12 months ago

@fdelu you can use patch-package to apply the fix in the PR I just posted (#28938) and see if that fixes the issue for you. You will need to rebuild the JS bundle after applying the patch.

Sorry for the delayed response. I can confirm the data is parsed correctly now with that fix.

The only remaining issue I have now is what I mentioned last in my previous comment: whenever I get a notification and the app is open, I close the app completely and then reopen it with that notification, then everything in the notification response is null:

{
  "actionIdentifier": "expo.modules.notifications.actions.DEFAULT",
  "notification": {
    "date": 0,
    "request": {
      "identifier": null,
      "content": {
        "body": null,
        "dataString": null,
        "title": null
      },
      "trigger": {
        "type": "push",
        "channelId": null
      }
    }
  }
}

Note that I only managed to reproduce this by receiving the notification when the app is open, then closing the app and then touching the notification to open it again. Any other combination seems to work well for me. Does this happen to anyone else?

This one seems trickier to debug but I think something must be going wrong in the toBundle or toResponseBundleFromExtras methods in the NotificationSerializer?

krazykriskomar commented 12 months ago

@krazykriskomar appreciate the clarification. Right now my plan is as follows: get an icon working for my own test app (I haven't before bothered with icons for test apps, which is why we missed this bug in FCM V1) so I understand how this is supposed to work (I'm also new to mobile, having been in the infrastructure trenches the past 2 years, so we're in this together). Once I get an icon working with FCM Legacy then I can debug FCM V1.

If I'm unable to get a fix for this by early next week, the following will happen: I'll contact folks in this thread and organize zoom calls to debug this together. This is a last resort for obvious reasons, but given Google's deadline for FCM V1 on June 20th we need to nail this down via any means necessary.

So, I'll report back here on Monday (or sooner!) with an update. Thanks everyone for your continued patience!

Thank you so much for your work on this @christopherwalter ! To clarify, our situation (me and @NickTillinghastKrazy)'s situation is a bit unique in that our company only sends notifications through Braze, and it's entirely possible we also have a compatibility issue with Braze/Expo that I have no idea how to begin to troubleshoot. However, I'm just waiting for others on this thread to start seeing their expected output before I fully engage Braze on the issue. Just to reiterate which issues we at KCL are experiencing:

krazykriskomar commented 12 months ago

@christopherwalter @bradvatne Checking in: I've updated to expo 51.0.8 and expo-notifications 0.28.3 and I added back in the withFirebaseMessagingNotificationIcon plugin but no luck. The notification icon on Android is still blank. Is there any useful log data/code/etc that I can send you that you could use? Let me know, I'm happy to provide it.

Here's the AndroidManifest section that i think you're interested in. Everything seems right, but tell me if you see something out of place.

image

Xnip2024-05-18_10-14-32

image
firashelou commented 12 months ago

Hello, I have upgraded to expo package (51.0.7) and expo-notifications 0.28.2 as mentioned here but the issue still persist which is, when i receive a notification, i click on it, and it does not go to the intended screen but instead to the homepage of the app !

here is my code for the notification receiver: (note: i have this same code on an older app and it works great without the update)

responseListener.current = Notifications.addNotificationResponseReceivedListener( (response) => { console.log('clicked response: ', response); //navigation.navigate('AboutScreen'); // Extract the screen name and any additional data from the response const { screen } = response.notification.request.content.data; console.log("response.notification.request.content.data: ", response.notification.request.content.data); const { data } = response.notification.request.content; console.log('data: ', data); //console.log('screen: ', screen); // Use the navigationRef to navigate to the desired screen // navigate("NewsDetailsScreen", data); if (screen) { navigate(screen, data); } } );

krazykriskomar commented 12 months ago

FYI @christopherwalter @bradvatne I've got notifcations v0.28.2 out there in prod and while it's not super prevalent, Google Play Console is now showing that this error is happening to some people. Just so you know. Caused by java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String java.lang.Object.toString()' on a null object reference at expo.modules.notifications.service.delegates.ExpoNotificationLifecycleListener.logExtra (ExpoNotificationLifecycleListener.java:73) at expo.modules.notifications.service.delegates.ExpoNotificationLifecycleListener.onCreate (ExpoNotificationLifecycleListener.java:43) at expo.modules.ReactActivityDelegateWrapper.onCreate (ReactActivityDelegateWrapper.kt:181) at com.facebook.react.ReactActivity.onCreate (ReactActivity.java:47) at com.thekrazycouponlady.kcl.MainActivity.onCreate (MainActivity.kt:19) at android.app.Activity.performCreate (Activity.java:8767) at android.app.Activity.performCreate (Activity.java:8745) at android.app.Instrumentation.callActivityOnCreate (Instrumentation.java:1519) at android.app.ActivityThread.performLaunchActivity (ActivityThread.java:3928)

christopherwalter commented 11 months ago

Hi folks, for those of you here for icon-related issues: it seems this is not an FCM V1 versus FCM Legacy problem, but a problem with the way we're configuring the manifest when icon config is provided in app.json. Follow the config plugin solution here for a temporary fix, and look for updates on this issue for a permanent fix.

christopherwalter commented 11 months ago

FYI I've updated the issue description with statuses for the bugs we're tracking here. Here's the current state of the world:

krazykriskomar commented 11 months ago

FYI I've updated the issue description with statuses for the bugs we're tracking here. Here's the current state of the world:

Thank you so much @christopherwalter for your hard work on this! Just as a clarification, the temp fix for the icon doesn't work for me, unfortunately. If there's anyone else that's gotten that to work, are you aware of any gotchas or modifications I must make to that code to make it work for me?

viljark commented 11 months ago

Fix for #28883 has been published. Pick up latest expo package (51.0.7) and do npx expo install --fix to pick up expo-notifications 0.28.2. You will also get a new version of expo-dev-client that fixes an unnecessary reload issue on Android (see #28893).

@douglowder do you know if the fixes will be ported to SDK 50 also? It seems 0.28 has some breaking changes and is not compatible with SDK 50. Just trying to understand if I need to prioritize our SDK upgrade as the June 20th is around the corner.

kubmir commented 11 months ago

Hello @christopherwalter, unfortunately I am afraid that solution is not 100% correct yet.

We use expo 51.0.8 and the newest version of expo-notifications 0.28.3 with FCM V1 service account key but push notifications are still not working well for Android. Our issue is related to useLastNotificationResponse and specifically notification.request.content.data is missing here - body is null.

Works well with Expo 49 and deprecated token.

Codelica commented 11 months ago

I'm afraid I'm seeing issues that don't like up with the current status here. I'm testing with an EAS Android build based on:

Testing with a device running Android 13.

Push message sent with useFcmV1: false:

App State Sound Banner Icon Pressed notification data
Foreground βœ… βœ… βœ… βœ…
Background βœ… βœ… βœ… βœ…
Killed βœ… βœ… βœ… Press launches app but notification request.content = { "body": null, "dataString": null, "title": null }

Push message being sent with useFcmV1: true:

App State Sound Banner Icon Pressed notification data
Foreground βœ… βœ… βœ… βœ…
Background βœ… ❌ ❌ βœ…
Killed βœ… ❌ ❌ Press launches app. Notification request.content has body and title, does not have data. However it contains dataString with the JSON that should be parsed for data.

So something seems off. I definitely see different results depending on useFcmV1. Technically I could get the notification presses working with useFcmV1: true (for now) by parsing that dataString JSON, but it still doesn't explain the banner and icon not working which isn't supposed to be related?

fdelu commented 11 months ago

I'm afraid I'm seeing issues that don't like up with the current status here. I'm testing with an EAS Android build based on:

  • expo 51.0.8
  • expo-notifications 0.28.3
  • Using useLastNotificationResponse() hook

Testing with a device running Android 13.

Push message sent with useFcmV1: false:

App State Sound Banner Icon Pressed notification data Foreground βœ… βœ… βœ… βœ… Background βœ… βœ… βœ… βœ… Killed βœ… βœ… βœ… Press launches app but notification request.content = { "body": null, "dataString": null, "title": null }

Push message being sent with useFcmV1: true:

App State Sound Banner Icon Pressed notification data Foreground βœ… βœ… βœ… βœ… Background βœ… ❌ ❌ βœ… Killed βœ… ❌ ❌ Press launches app. Notification request.content has body and title, does not have data. However it contains dataString with the JSON that should be parsed for data. So something seems off. I definitely see different results depending on useFcmV1. Technically I could get the notification presses working with useFcmV1: true (for now) by parsing that dataString JSON, but it still doesn't explain the banner and icon not working which isn't supposed to be related?

@Codelica I had the dataString issue too. AFAIK it is already fixed but it is currently unreleased (see here). I also had an issue with notifications received in the foreground where you receive the notification, close the app and then press it. The notification data is empty in that case. Do you also happen to have that issue?

Codelica commented 11 months ago

I also had an issue with notifications received in the foreground where you receive the notification, close the app and then press it. The notification data is empty in that case. Do you also happen to have that issue?

@fdelu Thanks for the info on dataString.

I just gave the case you mentioned a try and everything worked OK for me with both useFcmV1 as true or false. It seems that if it comes in while foregrounded it acts like my foreground case regardless if it's pressed while backgrounded after or not -- for me at least. If that makes sense :)

christopherwalter commented 11 months ago

@kubmir I added this to our list of items on this tracking issue, I think @douglowder should be able to get a fix for this when he's back in the office next week.

jnoleau commented 11 months ago

@Codelica Thank you for this great summary.

Using in my case the addNotificationResponseReceivedListener , I have exactly the same behaviour of no data in the data field in FCM v1 & FCM v2 when the app is Killed (and no issue when the app is in background or foreground)

mgscreativa commented 11 months ago

I'm afraid I'm seeing issues that don't like up with the current status here. I'm testing with an EAS Android build based on:

* expo 51.0.8

* expo-notifications 0.28.3

* Using `useLastNotificationResponse()` hook

Testing with a device running Android 13.

Push message sent with useFcmV1: false:

App State Sound Banner Icon Pressed notification data Foreground βœ… βœ… βœ… βœ… Background βœ… βœ… βœ… βœ… Killed βœ… βœ… βœ… Press launches app but notification request.content = { "body": null, "dataString": null, "title": null }

Push message being sent with useFcmV1: true:

App State Sound Banner Icon Pressed notification data Foreground βœ… βœ… βœ… βœ… Background βœ… ❌ ❌ βœ… Killed βœ… ❌ ❌ Press launches app. Notification request.content has body and title, does not have data. However it contains dataString with the JSON that should be parsed for data.

So something seems off. I definitely see different results depending on useFcmV1. Technically I could get the notification presses working with useFcmV1: true (for now) by parsing that dataString JSON, but it still doesn't explain the banner and icon not working which isn't supposed to be related?

The exact same behavior here

PS: Updated my test project to SDK 51 here https://github.com/mgscreativa/expo-notifications-test

christopherwalter commented 11 months ago

Hi everyone, there are two issues we're prioritizing fixes for at this moment:

Codelica commented 11 months ago
  • Notification icon not working

    • Status: @douglowder is working on a fix that incorporates the changes in this config plugin. We expect a PR by EOW (5/31) but need to confirm working behavior, so aim for fix in production by 6/3.

Just to clarify a couple things...

Would this also address the banner not showing when using useFcmV1: true or is that a separate issue ?

Would this also handle the alert icon color setting ? (the config plugin linked seems to imply that could be needed also)

Thanks for the update!

mlukasik-dev commented 11 months ago

Has anyone else experienced crashes on Android when opening app links (deep links) after migrating to Expo 51? The stack trace starts with expo.modules.notifications.service.delegates.ExpoNotificationLifecycleListener, so I'm pretty sure the issue is related to this one.

The error message is: NullPointerException: Attempt to invoke virtual method 'java.lang.String java.lang.Object.toString()' on a null object reference

jludwiggreenaction commented 11 months ago

I see the issue was updated to mention cherry-picking changes into SDK 51. Is there a timeline when they'll be pushed to SDK50 as well?

douglowder commented 11 months ago

@jludwiggreenaction we are going to prioritize getting everything working on SDK 51, and after that we will start backporting these changes to SDK 50 and do testing there.

last-core commented 11 months ago

new release today? πŸ‘€

codercoder292 commented 11 months ago

I came across this issue while migrating our app's push notifications to FCM v1. Let me briefly summarize the problems I've encountered.

only android The only difference is the true or false value of useFcmV1

  1. When using the legacy API (?useFcmV1=false):

    • The deeplink works correctly, directing to the defined deeplink. [payload] "data" => ["url" => {url}],
    • The registered push icon (icon_notification.png) is displayed correctly.
    • The push alert is displayed correctly.
  2. When using the v1 API (?useFcmV1=true):

    • The deeplink does not work and redirects to the main screen.
    • The registered push icon is not displayed, and the app icon is shown instead.
    • The push alert is not displayed (no popup at the top).

sdk 51 expo-noti 0.28.3

krazykriskomar commented 11 months ago

Not sure if it was supposed to fix the issues, but just reporting here: expo 51.0.9 expo-notifications 0.28.4 did not fix:

mikerogerz commented 11 months ago

I can confirm that getLastNotificationResponseAsync() still doesn't have any data property in the payload. This is after upgrading to 0.28.4.

mlukasik-dev commented 11 months ago

Has anyone else experienced crashes on Android when opening app links (deep links) after migrating to Expo 51? The stack trace starts with expo.modules.notifications.service.delegates.ExpoNotificationLifecycleListener, so I'm pretty sure the issue is related to this one.

The error message is: NullPointerException: Attempt to invoke virtual method 'java.lang.String java.lang.Object.toString()' on a null object reference

Upgrading to expo-notifications 0.28.4 resolved the push notifications issue, but the problem with opening app links persists. This isn't a configuration issue, as everything functioned correctly before upgrading to Expo 51.

Here is the call stack that indicates the issue is related to the notifications module:

java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String java.lang.Object.toString()' on a null object reference
    at expo.modules.notifications.service.delegates.ExpoNotificationLifecycleListener.logExtra(ExpoNotificationLifecycleListener.java:73)
    at expo.modules.notifications.service.delegates.ExpoNotificationLifecycleListener.onNewIntent(ExpoNotificationLifecycleListener.java:63)
    at expo.modules.ReactActivityDelegateWrapper.onNewIntent(ReactActivityDelegateWrapper.kt:278)
    at com.facebook.react.ReactActivity.onNewIntent(ReactActivity.java:107)
    at android.app.Activity.performNewIntent(Activity.java:8607)
    at android.app.Instrumentation.callActivityOnNewIntent(Instrumentation.java:1484)
    at android.app.Instrumentation.callActivityOnNewIntent(Instrumentation.java:1497)
    at android.app.ActivityThread.deliverNewIntents(ActivityThread.java:4365)
    at android.app.ActivityThread.handleNewIntent(ActivityThread.java:4372)
    at android.app.servertransaction.NewIntentItem.execute(NewIntentItem.java:56)
    at android.app.servertransaction.ActivityTransactionItem.execute(ActivityTransactionItem.java:45)
    at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
    at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2574)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loopOnce(Looper.java:226)
    at android.os.Looper.loop(Looper.java:313)
    at android.app.ActivityThread.main(ActivityThread.java:8762)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:604)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)
douglowder commented 11 months ago

@mlukasik-dev that does indeed appear related to the recent changes to fix notification responses. Could you file a new issue (with a repro) and link it here?

douglowder commented 11 months ago

The fix for #28938 is now published in expo-notifications@0.28.4. You should upgrade to expo@51.0.9 and do npx expo install --fix to get all the latest fixes.