laravel-notification-channels / fcm

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

Throw the first exception if error happened during multicast send #83

Closed pyr0hu closed 1 year ago

pyr0hu commented 3 years ago

If an error happens during a multicast send, the catch part does not handle any of the errors.

This PR solves this by checking the multicast results and throws the first exception it found.

Real life example: Our application sent Android notifications with ->setTtl(3600). This parameter was changed sometimes recently or I don't know when, and our users did not receive any notifications since the change as the application did not handled the error during the multicast sending.

nicolasbeauvais commented 3 years ago

Tested on my end, @pyr0hu modification work as expected, and is necessary to debug and monitor in production.

Any maintainer could merge it? :pray:

SachinAgarwal1337 commented 2 years ago

This will break the existing behaviour where we can listen to the NotificationSent event. I would suggest to consider that behaviour

atymic commented 2 years ago

@SachinAgarwal1337 do you have a better suggestion?

SachinAgarwal1337 commented 2 years ago

@atymic My suggestion is, library needs to fire its own events so we can listen to them instead of genereic from laravel. So you can listen to those events and can know the responses.

And if this exception is still needed then should be part of a major release with breaking change release notes

SachinAgarwal1337 commented 2 years ago

My Major use case is, I listen to the failed response and delete all the fcm tokens from database which come down as invalid or unknown. Because there is no other way of knowing if the fcm token is still valid or not