laravel-notification-channels / fcm

Firebase Cloud Messaging (FCM) notifications channel for Laravel
https://laravel-notification-channels.com/
MIT License
472 stars 121 forks source link

Handle failed tokens #167

Closed Stevemoretz closed 10 months ago

Stevemoretz commented 10 months ago

So as there was no up-to-date alternative for this, I had to reverse engineer and see what's going on and how to fix this.

It's not that hard, here's the most important part of this library at vendor/laravel-notification-channels/fcm/src/FcmChannel.php

    public function send($notifiable, Notification $notification)
    {
        $tokens = Arr::wrap($notifiable->routeNotificationFor('fcm', $notification));

        if (empty($tokens)) {
            return [];
        }

        // Get the message from the notification class
        $fcmMessage = $notification->toFcm($notifiable);

        if (! $fcmMessage instanceof Message) {
            throw CouldNotSendNotification::invalidMessage();
        }

        $this->fcmProject = null;
        if (method_exists($notification, 'fcmProject')) {
            $this->fcmProject = $notification->fcmProject($notifiable, $fcmMessage);
        }

        $responses = [];

        try {
            if (count($tokens) === 1) {
                $responses[] = $this->sendToFcm($fcmMessage, $tokens[0]);
            } else {
                $partialTokens = array_chunk($tokens, self::MAX_TOKEN_PER_REQUEST, false);

                foreach ($partialTokens as $tokens) {
                    $responses[] = $this->sendToFcmMulticast($fcmMessage, $tokens);
                }
            }
        } catch (MessagingException $exception) {
            $this->failedNotification($notifiable, $notification, $exception, $tokens);
            throw CouldNotSendNotification::serviceRespondedWithAnError($exception);
        }

        return $responses;
    }

First of all I found that MessagingException only occurs for single tokens and that means NotificationFailed won't even be called when you have multiple users, so relying on that is a mistake, also when there is a method for sending multiple tokens why use sendToFcm the logic here is divided into two different paths that don't even behave in the same way.

My advice to the creator of this library is to get rid of sendToFcm and using use sendToFcmMulticast and there is no need for that event, as you are returning responses and responses from sendToFcmMulticast contains all the information about failed tokens.

How to handle failed tokens

  1. So I'm going to bypass this line
            if (count($tokens) === 1) {

and get rid of the logic for single tokens by a dirty workaround, simply return an array from your routeNotificationForFcm method in your model class, and if you have only one token to return also append a null to it, so you'll return 2 tokens but null will be ignored.

    public function routeNotificationForFcm()
    {
        $tokens = $this->fcmTokens;

        if(count($tokens) === 1){
            // add null to bypass if (count($tokens) === 1) { at vendor/laravel-notification-channels/fcm/src/FcmChannel.php
            $tokens[] = null;
        }

        return $tokens;
    }
  1. Listen to the Laravel's official Illuminate\Notifications\Events\NotificationSent event, make a listener for it eg: NotificationSentListener and then register it in EventServiceProvider.
    /**
     * The event to listener mappings for the application.
     *
     * @var array<class-string, array<int, class-string>>
     */
    protected $listen = [
        NotificationSent::class => [
            NotificationSentListener::class,
        ]
    ];
  1. in your listener grab the list of tokens to remove:
    public function handle(NotificationSent $event): void
    {
        if(is_array($event->response)){
            if($event->channel === FcmChannel::class){
                $tokensToRemove = [];
                foreach($event->response as $response){
                    if($response instanceof MulticastSendReport){
                        foreach($response->failures()->getItems() as $failed){
                            if ($failed->target()->type() === MessageTarget::TOKEN) {
                                $tokensToRemove[] = $failed->target()->value();
                            }
                        }
                    }
                }
                // remove these tokens from your database
                dd($tokensToRemove);
            }
        }
    }

Done, no need for any custom event or anything.

This should fix : https://github.com/laravel-notification-channels/fcm/issues/163 for now, I advise changing the library to function in this native way and add it to the docs, I would add a PR but I'm short on time, so this is the best I could do, good luck :)

dwightwatson commented 10 months ago

Thanks for this - please consider using dev-master or looking at #168. The intention is now to dispatch the NotificationFailed event for every failed token.