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

Serialization of 'Closure' is not allowed #176

Closed haleyngonadi closed 9 months ago

haleyngonadi commented 9 months ago

Hi, I recently updated to the 4.0.0, and I've updated my code as in the docs, but running the notify command now throws an error:

Serialization of 'Closure' is not allowed

  at vendor/laravel/framework/src/Illuminate/Queue/Queue.php:159
    155▕         ]);
    156▕ 
    157▕         $command = $this->jobShouldBeEncrypted($job) && $this->container->bound(Encrypter::class)
    158▕                     ? $this->container[Encrypter::class]->encrypt(serialize(clone $job))
  ➜ 159▕                     : serialize(clone $job);
    160▕ 
    161▕         return array_merge($payload, [
    162▕             'data' => array_merge($payload['data'], [
    163▕                 'commandName' => get_class($job),

In my notification file, the code looks like this:

class DailyCalendar extends Notification implements ShouldQueue
{
    use Queueable;

[...]
public function via($notifiable)
  {
return [ FcmChannel::class]

}

public function toFcm(): FcmMessage
  {
    $title = __('notifications.ratings.title', [
      'date' => $this->date,
    ]);

    $message = __('notifications.ratings.body', [
      'date' => $this->date,
    ]);
    return (new FcmMessage(notification: new FcmNotification(
      title: $title,
      body: $message,
    )))
      ->data(['type' => 'ratings', 'route' => 'ratings', 'url' => $this->url])
      ->custom([
        'webpush' => [
          'fcm_options' => [
            'analytics_label' => 'analytics_web',
            'link' => $this->url,
          ],
        ],
      ]);
  }

}
dwightwatson commented 9 months ago

Hey - I'm not sure this will necessarily be on this package. As I understand it's not that the channel gets queued, but Laravel will queue a job that calls out to the channel. I am using this in production on a queue so I'm not sure what's going on.

Is it possible that there were old jobs on the queue that broke after the upgrade? Or are you able to dig more into what that Closure is that is failing?

haleyngonadi commented 9 months ago

@dwightwatson Thank you for the quick response. There are no queued jobs at all as it is my local env. When I comment out the FCMChannel and just use the database channel, it works fine. So strange.

I'm running $user->notify through a command in app\Console\Commands 🤔

dwightwatson commented 9 months ago

Weird then. If you're able to provide a minimal reproduction on a fresh Laravel app it would be super helpful in working out what is going on.

haleyngonadi commented 9 months ago

@dwightwatson Oh, your comment just gave me an idea. I tried it on a completely fresh Laravel install, and it worked fine! Pfft, I wonder what it could on my existing project.

It's pointing to

  NotificationChannels\Fcm\FcmChannel::NotificationChannels\Fcm\{closure}(Object(Kreait\Firebase\Messaging\MulticastSendReport))
haleyngonadi commented 9 months ago

I think I got it - My database had some tokens that are no longer valid! But I thought listening to NotificationFailed should catch the error and move to the next user token? That isn't currently happening.

UPDATE: I commented out [ 'report' => $report] in the dispatchFailedNotification function in FcmChannel.php and the Serialization error has gone away.

You should be able to reproduce the error by changing one of your web tokens to an invalid token (that is when a user has more than one token). I think whatever $report is returning is causing the issue.

dwightwatson commented 9 months ago

It should dispatch the event for failed tokens to let you clean them up. But the notification itself should continue to run. Still not sure how this would lead to a serialization error.

You may want to try debugging the event listener to be sure it is being called and working as you expect.

haleyngonadi commented 9 months ago

🤦🏾‍♀️ Removing ShouldQueue from the event listener fixed the issue! Thank you for your patience with me :')