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

Improve exception handling #710

Open
oprypkhantc opened this issue Dec 7, 2021 · 1 comment
Open

Improve exception handling #710

oprypkhantc opened this issue Dec 7, 2021 · 1 comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap

Comments

@oprypkhantc
Copy link

Issue Summary

The SDK has very poor error/exception handling. Debugging is awfully complex, catching specific errors in business code is a pain. Some examples:

  1. Twilio API returns a 4xx HTTP code, indicating an error. Being more specific, it returns a 20404 Twilio error code, which indicates that the requested entity was not found. If you try to catch it in your code, it would look something like this:
try {
	$twilioConversation = $this->conversationsServiceContext
		->conversations($this->uniqueName($thread))
		->fetch();

	// do something

	return $twilioConversation;
} catch (RestException $e) {
	// If not found by unique name, create a new one.
	if ($e->getCode() === 20404) {
		return $this->conversationsServiceContext
			->conversations
			->create($data->toArray());
	}

	throw $e;
}

It's fine, but gets pretty annoying if you have multiple such catch blocks. SDK could instead offer more specific exceptions (obviously, not for every error, but at least for the most common that are usually being catched):

try {
	$twilioConversation = $this->conversationsServiceContext
		->conversations($this->uniqueName($thread))
		->fetch();

	// do something

	return $twilioConversation;
} catch (EntityNotFoundException $e) {
	return $this->conversationsServiceContext
		->conversations
		->create($data->toArray());
}

This is a lot cleaner and doesn't rely on magic error code numbers.

  1. Twilio API returns a 4xx or 5xx error code, indicating an error. Being more specific, it returns a random validation/uniqueness error that is completely unexpected in your business logic. You'll get an exception that isn't informative and one that is quite hard to debug, especially on non-local envs:
Twilio\Exceptions\RestException : [HTTP 400] Unable to create record: Unique name too long
 /app/vendor/twilio/sdk/src/Twilio/Version.php:88
 /app/vendor/twilio/sdk/src/Twilio/Version.php:223
 /app/vendor/twilio/sdk/src/Twilio/Rest/Conversations/V1/Service/ConversationList.php:60

Instead, we could have gotten something like this if we relied on Guzzle exceptions under the hood (while still rethrowing RestException for BC if needed):

GuzzleHttp\Exception\RequestException : Client error: `POST https://conversations.twilio.com/v1/Services/IS511111111111e5e9b52c2b010f8ea3a/Conversations` resulted in a `400 Bad Request` response:
{"code": 50004, "message": "Unique name too long", "more_info": "https://www.twilio.com/docs/errors/50004", "status": 400}, request:
POST /v1/Services/IS511111111111e5e9b52c2b010f8ea3a/Conversations HTTP/1.1
Host: conversations.twilio.com
User-Agent: twilio-php/6.31.1 (PHP 7.4.24)
Accept-Charset: utf-8
Content-Type: application/x-www-form-urlencoded
Accept: application/json
Authorization: **************************************************************************************************

UniqueName=QySP1rXvtHZRDuGHm3Cp0nPXq3A2AOYSopvKAO1g5fV92qc4oHi74VKhOEP7fdURDG5uCSrLZ1imvQcz3wKQYQtyyubnTD9mlBArXk1lyPcM8kjzToayu95gYauZedMZKdSf4IPVBUOUEZPISdTdhp9mhsvym6RQRY9HzobfZBtqQbuk2tF7GAMplQlIfJeAdIm2I6EQAflbWqV4mAgGmxOwFBS5bGhNuO0cr4UY2m1pJVSEK8GB8uHu9z8TXxhqFdnBti1nB4pF0LqkMPxhXzBsVshvTOyXfVLg7vnz1P8ICLJWi629t4V24pXh2yqpcDxCiV6G38JIStLIKLszLbdzlJbnwqM33mPUWvVwK84VZny5mI8par17XAJZm1NzJSgKwM83xholZSuGSqq0kVgJ9Dkw7PrgiQmO98LW7zVPMZ2tN9ltUkqBGkcwhcfMBYK8uS8j6vEgA7vjY8H7Oj6XrQX4e3vyAzpKRL4ICSqu4t5iCcEePL4Q7TFlRLZpx1j0s60axjNKgf89ChSggtQNDY4Qg2CmeMkKWIxPCArthreads_2243&Attributes=%7B%22id%22%3A2243%7D

Currently, both of these are achievable with a custom Twilio\Http\Client implementation (one that only returns Twilio\Http\Response for successful requests and throws exceptions in all other cases), but I think it's worth implementing in the SDK itself.

Technical details:

  • twilio-php version: 6.31.1
@JenniferMah JenniferMah added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap labels Dec 8, 2021
@JenniferMah
Copy link
Contributor

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

2 participants