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

Exception not thrown when sending to multiple devices #138

Closed hsul4n closed 1 year ago

hsul4n commented 1 year ago

I notice that when sending notification and there's an error, for example Android message is too large.

When using only one device and return token as string, the exception appears in log file.

public function routeNotificationForFcm()
{
    return 'foo';
}

but we using multiple devices and return array, I didn't get any exception in that case

public function routeNotificationForFcm()
{
    return ['foo', 'bar'];
}

Is that because Firebase FCM service doesn't give exception details when sending to multiple devices!

dwightwatson commented 1 year ago

We use a different method under the hood when passed multiple tokens. It should still throw an exception I'd imagine - can you debug on your end and see what happens?

I'm tempted to ditch the exceptions entirely and just rely on dispatching the failed notification event.

aaronhuisinga commented 1 year ago

We are seeing this same behavior. We weren't aware that push notifications hadn't been sending properly due to the exceptions getting swallowed up. A single token will throw an exception, but sending to multiple does not. Sentry doesn't receive anything, and I don't see it on our logs anywhere.

hsul4n commented 1 year ago

Any update!

dwightwatson commented 1 year ago

No, nothing here. Happy to review any PRs.

It looks like when you send to multiple tokens you get a report back and I presume this doesn't throw any exceptions.

My personal feeling is we should always do a multicast send instead of differentiating the API call we make if there is 1/1+ tokens. We could then map the report back and dispatch the NotificationFailed/throw exceptions as required).

webocoders commented 1 year ago

My personal feeling is we should always do a multicast send instead of differentiating the API call we make if there is 1/1+ tokens. We could then map the report back and dispatch the NotificationFailed/throw exceptions as required).

This would be a great solution!