albaraam / php-gcm-apns

A PHP Library for sending messages to devices (Android & IOS) through GCM and Apns respectively .
MIT License
13 stars 0 forks source link

HTTP Authentication Error with no targets in the Android section #2

Open tl8roy opened 8 years ago

tl8roy commented 8 years ago

When there are no targets in the Android section, I get an albaraam\gcm\exceptions\AuthenticationException: HTTP Authentication Error .

The backtrace tells me it is at: albaraam\gcm\GCMClient->send(Object(albaraam\gcm\GCMMessage)) /vendor/albaraam/php-gcm-apns/src/Client.php (90)

I have managed to temporarily fix the issue by changing Client->send() to check if there is a target before trying to send the message.

if($this->getAndroidClient() && count($message->android->getTo()) > 0) $response['android'] = $this->sendAndroid($message);
if($this->getIOSClient() && count($message->ios->getTo()) > 0) $response['ios'] = $this->sendIOS($message);

Is there a reason such a check is not made that I am not aware of?

albaraam commented 8 years ago

Hey @tl8roy,

You're right, it's better to be checked before sending, but I think it should still throw an exception (ex: NoRecipientsException) and the check (if no targets) should be handled by the developer using the library, because in case the developer sent an empty array by mistake he/she should know that there is something wrong. So what do you think?

Actually I fixed it in the php-gcm library, but I don't have much time for it, so if you can update and test that would be appreciated.

tl8roy commented 8 years ago

That isn't a bad idea, but the implementation would need to allow independent execution of the iOS and Android send functions.

Fixing the php-gcm library helps, but would still mean that no Android devices will still block Apple sending.

For this library, it would be better to look at both lists and throw an exception if there are no targets at all.