Bug: FCM - Duplicate Topic Notifications

roybeitner commented 1 year ago

[READ] Step 1: Are you in the right place?

For issues related to the code in this repository file a Github issue.
If the issue pertains to Cloud Firestore, read the instructions in the "Firestore issue" template.
For general technical questions, post a question on StackOverflow with the firebase tag.
For general Firebase discussion, use the firebase-talk google group.
For help troubleshooting your application that does not fall under one of the above categories, reach out to the personalized Firebase support channel.

[REQUIRED] Step 2: Describe your environment

Operating System version: Any iOS or Android running Flutter 3.00 with firebase_messaging 13.0.0
Firebase SDK version: 11.0.1
Firebase Product: FCM (auth, database, storage, etc)
Node.js version: v16.x AWS Lambda
NPM version: AWS Lambda

[REQUIRED] Step 3: Describe the problem

Android and iOS users receive at least once a day a duplicate topic notification sent from the Lambda service running the Firebase-Admin SDK. No duplicate notifications we're generated by our service itself (used extensive logging for each notification sent). We use the sendAll function (of the SDK) and send the notifications in batches. The client app does not create a local notification in any part of the code.

Steps to reproduce:

What happened? How can we make the problem occur? This could be a description, log/console output, etc.

Log from the client receiving a duplicate notification:

D/FLTFireMsgReceiver(11547): broadcast received for message
I/flutter (11547): ----------------------------
I/flutter (11547): {data, blablabla}
I/flutter (11547): 0:1662825724904819%88b82b6688b82b66
I/flutter (11547): Subtitle bla bla bla
I/flutter (11547): Title bla bla bla
I/flutter (11547): 2022-09-10 19:02:04.313
I/flutter (11547): ----------------------------
W/FirebaseMessaging(11547): Unable to log event: analytics library is missing
W/FirebaseMessaging(11547): Missing Default Notification Channel metadata in AndroidManifest. Default value will be used.
I/FirebaseMessaging(11547): Starting download of: imageurl
D/SurfaceView@bee2085(11547): updateSurface: surface is not valid
D/FLTFireMsgReceiver(11547): broadcast received for message
I/flutter (11547): ----------------------------
I/flutter (11547):  {data, blablabla}
I/flutter (11547): 0:1662825734630562%88b82b6688b82b66
I/flutter (11547): Subtitle bla bla bla
I/flutter (11547): Title bla bla bla
I/flutter (11547): 2022-09-10 19:02:14.219
I/flutter (11547): ----------------------------
W/FirebaseMessaging(11547): Unable to log event: analytics library is missing
W/FirebaseMessaging(11547): Missing Default Notification Channel metadata in AndroidManifest. Default value will be used.
I/FirebaseMessaging(11547): Starting download of:imageurl

the "sentTime"s are 2022-09-10 19:02:04.313 2022-09-10 19:02:14.219

Relevant Code:

Sent payload:

 const payload: TopicMessage= {
    topic: topicSomething,
    notification: {
      title,
      body,
      imageUrl: image,
    },
    data: {
     field1,
     field2,
     field3,
     field4,
     field5,
    },
    apns: {
      payload: {
        aps: {
          badge: 1,
          sound: "default",
        },
      },
      headers: {
        "apns-collapse-id": id,
      },
    },
    android: {
      notification: {
        imageUrl,
      },
      collapseKey: id,
      ttl: ONE_DAY_IN_MS,
    },
    webpush: {
      headers: {
        image,
        TTL: ONE_DAY_IN_SECONDS.toString(),
      },
    },
  };

Send command:

const notificationsResponse = await firebase
      .messaging()
      .sendAll([message1, message2, message3, ....]);

google-oss-bot commented 1 year ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

MarkChrisLevy commented 1 year ago

I can confirm, that I encounter the same issue. What is very weird it doesn't happen every single time, but rather from time to time and the number of duplicated notifications device receives varies, minimum 2 but it was even 6 in a few cases. I tried to find the pattern and the only difference I see is that when a message with duplicated notifications is send, the time of execution of sendAll is much longer (at least two times longer) than in messages without duplicates.

Today I had a situation, when a message was send and notifications was duplicated 5 times, but sendAll thrown an error "Error while making request: timeout of 10000ms exceeded", but still the message was send.

In my case, I'm passing 100 messages to sendAll. I'm using 10.0.2 version of firebase-admin within firebase function to send a message.

roybeitner commented 1 year ago

I can confirm, that I encounter the same issue. What is very weird it doesn't happen every single time, but rather from time to time and the number of duplicated notifications device receives varies, minimum 2 but it was even 6 in a few cases. I tried to find the pattern and the only difference I see is that when a message with duplicated notifications is send, the time of execution of sendAll is much longer (at least two times longer) than in messages without duplicates.

Today I had a situation, when a message was send and notifications was duplicated 5 times, but sendAll thrown an error "Error while making request: timeout of 10000ms exceeded", but still the message was send.

In my case, sendAll takes about 100 messages to be send. I'm using 10.0.2 version of firebase-admin.

+1 I'm experiencing exactly the same behavior.

chong-shao commented 1 year ago

Hello, from the "sentTime"s:

2022-09-10 19:02:04.313 2022-09-10 19:02:14.219

Looks like the time difference between the first send and the retry is 10s. Was the retry automatically performed by the Admin SDK or it's your code that implements the retry logic?

roybeitner commented 1 year ago

Hello, from the "sentTime"s:

2022-09-10 19:02:04.313

2022-09-10 19:02:14.219

Looks like the time difference between the first send and the retry is 10s. Was the retry automatically performed by the Admin SDK or it's your code that implements the retry logic?

Performed by the Admin SDK, might be the source of the problem.

chong-shao commented 1 year ago

@lahirumaramba Hi Lahiru, I think the wait time should be extended to 15s. Could you help make the change?

thopedam commented 1 year ago

This issue is still not fixed for batch-requests. The sendAll function still uses 10s.

roybeitner commented 1 year ago

I can confirm @thopedam comment. I still have the same issue. @lahirumaramba

lahirumaramba commented 1 year ago

Thanks folks, timeout fix for batch send is included in v11.4.0 https://firebase.google.com/support/release-notes/admin/node#11.4.0

SakuGunarathna commented 1 year ago

We are using firebase cloud messaging - topic base push notifications to send notifications for two apps (iOS and android). We are using Firebase cloud function to generate push notifications. We have experienced duplicated push notifications for some topics during last month (May). It occurs sometimes but not often. We have checked function logs and identified that cloud functions working properly and triggered once for each notification. We are I have mentioned the versions which used in the application.

Mobile message SDK - "@react-native-firebase/messaging": "^16.4.0" Node.js Admin SDK - "@google-cloud/tasks": "^3.1.0", "firebase-admin": "^10.0.2", "firebase-functions": "^3.18.0",

Here is the code block which we used to invoke firebase cloud messaging. //Making condition dynamically

if (global.length > 0) {
  condition = `'${data?.IdString}' in topics`;
} else {
  condition = fireb.docs.reduce(
    (cnd, data, i) =>
      `${cnd}${i > 0 ? ' || ' : ''}'${details?.Id[0]}-${
        data.data().refId
      }' in topics`,
    '',
  );
  // Example topic 1: "xxxxxxx-xxxxx-<firebase doc id>' in topics || 'xxxxxxx-xxxxx-<firebase doc id>' in topics";
  // Example topic 1: "xxxxxxx-xxxxx-<firebase doc id>' in topics
}

const message = {
 notification: {
  title: `${name} - ${title}`,
  body: details?.heading?.body,
 },
 apns: {
  payload: {
    aps: {
      sound: 'default',
      badge: 1,
    },
  },
 },
 data: {refId: `${data?.Id}`},
 condition: condition, //Dynamically made condition
};

//send topic based push notification const response = await admin.messaging().send(message);

MightySepp666 commented 3 months ago

@lahirumaramba we are experiencing this exact issue again since the 5th of march, starting at around 11:00 am UTC: many push notifications get delivered up to 5 times, caused by the internal default retry behavior of the HttpClient.

The "fix" to increase the timeout did not really solve this issue, if the Firebase backend doesn't respond in a reasonable amount of time.

This is a high priority issue, as it is very annoying for our users and they have started to uninstall our app! I have currently applied a hotfix patch, that overwrites (and effectively disables) the retry config for the AuthorizedHttpClient in the FirebaseMessagingRequestHandler. My patch is similar to this Pull Request, which unfortunately seems to have become stale: https://github.com/firebase/firebase-admin-node/pull/1739.

Please be aware, that while this severe bug is open, Firebase Messaging isn't usable for any serious organization.

lahirumaramba commented 3 months ago

Hey @MightySepp666 I understand your frustration. Let me reopen this issue and we can use it to track the next steps.

cc: @jonathanedey

DrLex0 commented 2 months ago

I have also noticed that sending a message to a topic can cause duplicate messages to be received. This seems to become consistent if a client's token has changed without the previous token being first unsubscribed. For instance, if I disable notification permissions for the website in Chrome's preferences and then re-enable them, the next getToken() will return a new token, but even before subscribing this new token to the topic, the client will still receive messages sent to the topic. When subscribing the new token, the messages might arrive twice. The old token cannot be unsubscribed, because trying to do so (in Python firebase-admin) will return in an INVALID_ARGUMENT or NOT_FOUND. It's already gone, but somehow FCM still manages to send messages to it, I suspect because the IID has not changed.\ Needless to say, this would make FCM totally unsuitable for any serious application where users can subscribe to a multitude of topics, but FTTB I just want to send a single kind of notification to a single topic, so in my case it is not a deal-breaker.

Although it seems that at least in MacOS Chrome, the duplicates eventually result in only a single notification being displayed, I still tried to be cautious by adding logic in the worker to ignore messages with the same tag if they arrive within a few seconds of the last.\ But, then I found out about the iOS practice of revoking permissions if an incoming push does not result in a showNotification call, for instance as explained here: https://meta.discourse.org/t/ios-notifications-can-lose-permission-to-push-if-the-user-is-currently-active/290225/8 \ What is unclear about this, is whether it applies to only notification type pushes, or also data push messages. Given the motivations for implementing it, I would expect it to apply to any type of push, but I am unsure.

So, my question is: should I not care about the possible duplicates and make sure to always trigger a showNotification for every push event, to avoid the iOS punishment?

enchorb commented 1 month ago

Why has this not been merged in yet https://github.com/firebase/firebase-admin-node/pull/1739?

firebase / firebase-admin-node