laravel / vonage-notification-channel

Vonage Notification Channel for Laravel.
https://laravel.com/docs/notifications#sms-notifications
MIT License
739 stars 50 forks source link

ErrorException: You are sending a message as `text` when contains unicode only error thrown on 3.2.0 #75

Closed sts-ryan-holton closed 1 year ago

sts-ryan-holton commented 1 year ago

Description:

I've just tried sending a text message,, the same exact message that I sent in the previous version of this package. I'm getting an ErrorException thrown by the Vonage library:

ErrorException: You are sending a message as text when contains unicode only characters. This could result in encoding problems with the target device or increased billing - See https://developer.vonage.com/messaging/sms for details, or email support@vonage.com if you have any questions. in /var/www/snapisms-api/vendor/vonage/client-core/src/SMS/Client.php:44

The message that I'm trying to send is:

Another message [Sender: Ryan]

This previously worked, now fails as a failed job.

dungnh commented 1 year ago

I also getting this issue also. Hope someone can help.

sts-ryan-holton commented 1 year ago

@dungnh For now, I've had to rollback my version of this package.

Hopefully a solution/fix soon! 👍

SecondeJK commented 1 year ago

Will be a short turnaround for this, on it now.

SecondeJK commented 1 year ago

Release 4.0.6 of the Vonage SDK fixes the regex for this @sts-ryan-holton @dungnh

SecondeJK commented 1 year ago

The other issue here - that the Vonage SDK is firing a warning which is being turned into an Exception is a different problem. If something is picking up the trigger_error and converting it to an Exception, I'm tempted to either remove it altogether or get something like Monolog to put it in a logfile. I don't want to -stop- users from sending what they want to send, but I do want to warn them if it's the wrong encoding without an uncaught Exception stopping runtime.

sts-ryan-holton commented 1 year ago

@SecondeJK Thanks for this. Does this mean an update of this package won't be required? Maybe you can do some kind of config. In my system specifically I'm using a library to detect unicode characters and auto charge customers more credits per message.

SecondeJK commented 1 year ago

You can run composer update to pull the latest in. The important thing here is that the underlying Vonage SDK can check the encoding for you. \Vonage\SMS\Message\SMS::isGsm7() will give you back a boolean in order for you to decide whether to use Laravel's unicode()

sts-ryan-holton commented 1 year ago

Cool. So is your suggestion to wrap the part where I send a message in a if statement for this part of my code:

try {
    $amount = $this->getNumberOfDeductableCredits($this->message->data['body']);
    $this->createCreditTransaction($message, $amount);

    return (new VonageMessage)
                ->clientReference($this->message->id)
                ->content($this->message->data['body']);

} catch (\Exception $e) { }

Something like...

try {
    $amount = $this->getNumberOfDeductableCredits($this->message->data['body']);
    $this->createCreditTransaction($message, $amount);

    if (\Vonage\SMS\Message\SMS::isGsm7()) {
        return (new VonageMessage)
                ->clientReference($this->message->id)
                ->content($this->message->data['body'])
                ->unicode();
    }

    return (new VonageMessage)
                ->clientReference($this->message->id)
                ->content($this->message->data['body']);

} catch (\Exception $e) { }
driesvints commented 1 year ago

I guess the root issue here is resolved. Feel free to discuss further for anything else. Thanks