Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jdavidbakr opened this issue Nov 5, 2022 · 2 comments
Open

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

jdavidbakr opened this issue Nov 5, 2022 · 2 comments

Comments

@jdavidbakr
Copy link

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
Copy link

lrljoe commented May 30, 2023

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

@muhzak
Copy link

muhzak commented Dec 17, 2023

@lrljoe what is the alternative approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants