ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 208 forks source link

Receiving duplicate notifications on Android Apps #302

Closed clareluna closed 6 years ago

clareluna commented 6 years ago

Hi!

I am facing an issue where our android app is receiving duplicate notifications. I had mentioned this on an earlier issue (https://github.com/ToothlessGear/node-gcm/issues/267).

Upon further investigation into our notification system, we are fairly convinced we are only sending a single notification but that our android app is receiving duplicate notifications when the app is running in the background. When the app is live, however, we only receive a single notification.

I noticed on a separate issue that when using addData we can consume the notification received in the app, while addNotification causes the OS to take control over the notification. Could you clarify that this understanding is correct? And if so, could you think of any reason that addNotification could be interacting with our android SDK to cause duplicate notifications to show?

Thanks, Clare

hypesystem commented 6 years ago

Hi Clare!

The way the android SDK interacts with the data sent is slightly out of scope of this library. StackOverflow would probably be the best bet for a good answer to these questions.

I can give a brief explanation on an abstract level, though.

When using addData, the data-field is set in the payload. This is always handled by the app in question.

When using addNotification, the notification-field is set in the payload. This may be handled by the OS if the app is in the background (but, as far as I recall may also be handled by the app if the app is active).

I don't think this should result in several notifications being received, but it is hard to debug without seeing the code that receives notifications. Even then, it would be outside the scope of this particular project.

We looked at the sending code in the other issue, and it did not seem to contain any errors. If you are confident that you are not sending multiple messages, the error is on the client side. And, unfortunately, outside the scope of this project and outside of my expertise :smile:

I hope this helps! Good luck tracking down the bug!

Best, Niels

eladnava commented 6 years ago

Just adding my two cents -- a lot of people recommend completely avoiding the notification fields in FCM notifications and always sending your notification data in the payload (using addData).

This is because FCM will not generate a notification if your app is in the foreground, leading to inconsistent behavior and bad UX.

Simply create and show a notification in your push BroadcastReceiver using the extracted data payload as follows:

package {YOUR_PACKAGE_NAME};

import android.content.Intent;
import android.graphics.Color;
import android.content.Context;
import android.media.RingtoneManager;
import android.app.NotificationManager;
import android.content.BroadcastReceiver;
import android.support.v4.app.NotificationCompat;

public class PushReceiver extends BroadcastReceiver {
    @Override
    public void onReceive(Context context, Intent intent) {
        String notificationTitle = "MyApp";
        String notificationText = "Test notification";

        // Attempt to extract the "message" property from the payload: {"message":"Hello World!"}
        if (intent.getStringExtra("message") != null) {
            notificationText = intent.getStringExtra("message");
        }

        // Prepare a notification with vibration, sound and lights
        NotificationCompat.Builder builder = new NotificationCompat.Builder(context)
                .setSmallIcon(android.R.drawable.ic_dialog_info)
                .setContentTitle(notificationTitle)
                .setContentText(notificationText)
                .setLights(Color.RED, 1000, 1000)
                .setVibrate(new long[]{0, 400, 250, 400})
                .setSound(RingtoneManager.getDefaultUri(RingtoneManager.TYPE_NOTIFICATION))
                .setContentIntent(PendingIntent.getActivity(context, 0, new Intent(context, MainActivity.class), PendingIntent.FLAG_UPDATE_CURRENT));

        // Get an instance of the NotificationManager service
        NotificationManager notificationManager = (NotificationManager) context.getSystemService(context.NOTIFICATION_SERVICE);

        // Build the notification and display it
        notificationManager.notify(1, builder.build());
    }
}
clareluna commented 6 years ago

Hi Neils,

Just wanted to give you guys an update. We decided follow Elad's advice and forgo using notification all together. We did not investigate the root cause that notification was producing duplicate notifications when the app was in the background, but we found that adding some handling code for the data object and switching to the addData formatting solved our duplicate notifications issue when the app was active or in the background.

Thanks for your advice and help last week! Clare

eladnava commented 6 years ago

Great to hear.

It sounds like, most likely, duplicate notifications were caused by using the addNotification fields and also writing custom handler logic in your push BroadcastReceiver to display your own system notification using NotificationManager, when FCM already does this for you if your app is in the background.

hypesystem commented 6 years ago

Great that you solved the issue! It really is odd that the device handling of messages with the notification field works so weirdly.

verybluebot commented 5 years ago

@eladnava can you explain a bit more about your solution? are you creating new PushReceiver class? where do call it and how?

sorry for the noob questions, Don't have a lot of native android milage on me yet (working on a React Native app)

EDIT: also when creating this class PendingIntent is not recognized..

ps: having the exact same issue of duplicated notifications when app is in foreground or closed

eladnava commented 5 years ago

Hi @verybluebot, Since you are using React Native this question should most likely be directed towards this plugin which you are probably using to send/receive notifications on RN: https://github.com/zo0r/react-native-push-notification

But what it usually comes down to is having a code-based notification handler which emits a system notification, and using the FCM notification parameter which makes the FCM service on the phone also emit a system notification.

verybluebot commented 5 years ago

@eladnava Im already using this plugin problem is on the server side, if I emit the notification object nothing is being sent to the client. Or at lest nothing react-native-push-notification can catch even though it seems it a server issue

PS: dash mi Israel :)

eladnava commented 5 years ago

@verybluebot Dash gam leha. :)

This is definitely on the plugin's side of things, as it seems to ignore scenarios where the notification parameter is removed, but when it's provided it invokes your app which creates a notification in addition to the built-in notification creation by FCM which uses the notification parameter, causing duplicate notifications or no notification in either case.