laravel-notification-channels / twilio

Twilio notifications channel for Laravel
https://laravel-notification-channels.com
227 stars 36 forks source link

Could we fire an event on \Twilio\Exceptions\RestException? #136

Open jdavidbakr opened 2 years ago

jdavidbakr commented 2 years ago

Thanks for this great package, I love using it.

One issue I'm having is that if a phone number has been unsubscribed, Twilio throws a \Twilio\Exceptions\RestException but in that exception there is no information about what the phone number was that caused the error. I'd like to unsubscribe that user when this happens because they have been unsubscribed (error code 21610).

The exception appears to be thrown at /twilio/src/Twilio.php:103 - would it be possible to discuss wrapping this line in a try/catch and firing custom exception that includes the data from the RestException but also includes the $to and $params values? I'm thinking something like this:

class TwilioNotificationRestException extends \Twilio\Exceuptions\RestException {
    protected $to;
    protected $params;

    public function __construct($to, $params, string $message, int $code, int $statusCode, string $moreInfo = '', array $details = []) {
      $this->to = $to;
      $this->params = $params;
      parent::__construct($message, $code, $statusCode, $moreInfo, $details);
    }
}

...

try {
    return $this->twilioService->messages->create($to, $params);
} catch (RestException $e) {
    throw new TwilioNotificationRestException(
        $to, $params, $e->getMessage(), $e->getCode(), $e->getStatusCode(), $e->getMoreInfo(), $e->getDetails()
    );
}

I'd be happy to put together a PR if this is something you'd be interested in including.

lrljoe commented 1 year ago

This is something I've done in my fork, but there is an alternative approach using existing events if you're interested.

muhzak commented 10 months ago

@lrljoe what is the alternative approach?