Wrong "Recipient of the message is not set." error

[tlacaelelrl]

Describe the bug
When sending a message the wrong error is thrown "Recipient of the message is not set." when in fact the issue is the data passed

To Reproduce

private function deliverMessage($to, $withData)
{
        $file = 'firebaseinfo.json';
        $firebase = (new Factory)->withServiceAccount($file);
        $messaging = $firebase->createMessaging();
        $message = CloudMessage
            ::withTarget('token', $to)
            ->withData($withData);
        try {
            $messaging->send($message);
        } catch (\Exception $e) {
            //Catched some wrong exception
        }
}
public function triggerInvalidException()
{
        $to = 'aVerifiedValidToken';
        $data = [
              'string'=>'data',
              'binary'=>binaryData, /* Caused of the issue, forgot to convert the binary data into string  */
        ];
        $this->deliverMessage($to, $withData);
        /*
         * Issue is triggered if binary data is passed
         * Not sure if the same happens with other data types
         * However the exception sends you looking in the wrong places for a problem
         * */
}
public function aNonTriggerInvalidException()
{
        $to = 'aVerifiedValidToken';
        $data = [
              'string'=>'data',
              'binary'=>bin2hex(binaryData),
        ];
        $this->deliverMessage($to, $withData);
        /*
         * No error is triggered
         * */
}

Expected behavior
A proper error message or at least some generic message other than a completely unrelated error.

Environment
OS: Ubuntu 18.04.4 LTS (GNU/Linux 4.15.0-106-generic x86_64)
PHP version: PHP 7.2.24-0ubuntu0.18.04.6 (cli) (built: May 26 2020 13:09:11) ( NTS )
Firebase SDK Version: 5.5.0

https://www.php.net/manual/en/function.bin2hex.php

[jeromegamez]

       try {
            $messaging->send($message);
        } catch (\Exception $e) {
            //Catched some wrong exception
        }

You're catching the exception but not doing anything with it. 😅

[jeromegamez]

Just a general remark: Please catch Throwable instead of Exception

Where is the Recipient of the message is not set. error thrown and why is it unrelated? If the problem is that you passed binary data instead of a string, what behavior would you expect?

The data array must be an array of string keys and string values, this is checked in https://github.com/kreait/firebase-php/blob/main/src/Firebase/Messaging/MessageData.php#L21 - so I would normally assume that the error is thrown outside of the try/catch - but in that case, the error message indeed doesn't match.

You can configure Debug Logging for the HTTP requests/responses the SDK is sending/receiving (https://firebase-php.readthedocs.io/en/5.5.0/setup.html#logging) - this way you could check if the error message is coming from the Firebase API and what actual data has been sent to the APIs.

[tlacaelelrl]

       try {
            $messaging->send($message);
        } catch (\Exception $e) {
            //Catched some wrong exception
        }

You're catching the exception but not doing anything with it. 😅

Not really, that is a sample code to replicate the issue and that is why I named it wrong exception since that is the problem I faced.

[tlacaelelrl]

Just a general remark: Please catch Throwable instead of Exception

Ok.

Where is the Recipient of the message is not set. error thrown and why is it unrelated?

If you run this sample code you can see the error thrown

$file = 'firebaseinfo.json';
$firebase = (new Factory)->withServiceAccount($file);
$messaging = $firebase->createMessaging();
$message = CloudMessage
     ::withTarget('token', 'useAValidTokenHere')
    ->withData([
        'binaryData'=>openssl_random_pseudo_bytes(5, true)
    ]);
$messaging->send($message);

But if you run this sample code you can send the message successfully

$file = 'firebaseinfo.json';
$firebase = (new Factory)->withServiceAccount($file);
$messaging = $firebase->createMessaging();
$message = CloudMessage
     ::withTarget('token', 'useAValidTokenHere')
    ->withData([
        'binaryData'=>bin2hex(openssl_random_pseudo_bytes(5, true))
    ]);
$messaging->send($message);

and why is it unrelated?

Is unrelated because the "Recipient of the message" was actually set so there is no reason for that error to be thrown

The data array must be an array of string keys and string values, this is checked in https://github.com/kreait/firebase-php/blob/main/src/Firebase/Messaging/MessageData.php#L21 - so I would normally assume that the error is thrown outside of the try/catch - but in that case, the error message indeed doesn't match.

My intention was never to pass binary data, I forgot to convert the binary data to string before passing it. The error received sent me looking for the issue in the wrong place. Not really critical but having a correct exception can help trace the issue.

[jeromegamez]

Thank you for the further explanations 🤗, I will try to reproduce it and find a way to catch this earlier 🤞

[jeromegamez]

d21925a adds a simple check for binary data that doesn't catch all cases, but probably the problematic ones.

Thanks again! 🙏

[tlacaelelrl]

d21925a adds a simple check for binary data that doesn't catch all cases, but probably the problematic ones.

Thanks again! 🙏

Thank you!!