From 2dd13a1ce480f89987e7e37dda84653d03e47779 Mon Sep 17 00:00:00 2001 From: Emile Pels Date: Wed, 4 Jul 2018 12:13:30 +0200 Subject: [PATCH 1/3] Add message when throwing exception based on error response --- src/MessageBird/Common/ResponseError.php | 83 +++++++++++++++++------- tests/integration/ResponseErrorTest.php | 41 ++++++++++++ 2 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 tests/integration/ResponseErrorTest.php diff --git a/src/MessageBird/Common/ResponseError.php b/src/MessageBird/Common/ResponseError.php index 687c78b0..27a76ff5 100644 --- a/src/MessageBird/Common/ResponseError.php +++ b/src/MessageBird/Common/ResponseError.php @@ -11,6 +11,7 @@ */ class ResponseError { + const EXCEPTION_MESSAGE = 'Got error response(s) from the server: %s'; const SUCCESS = 1; @@ -25,7 +26,7 @@ class ResponseError const CHAT_API_AUTH_ERROR = 1001; - public $errors = array (); + public $errors = array(); /** * Load the error data into an array. @@ -33,45 +34,81 @@ class ResponseError * * @param $body * - * @throws Exceptions\BalanceException * @throws Exceptions\AuthenticateException + * @throws Exceptions\BalanceException */ public function __construct($body) { if (!empty($body->errors)) { - foreach ($body->errors AS $error) { - - if ($error->code === self::NOT_ENOUGH_CREDIT) { - throw new Exceptions\BalanceException; - } elseif ($error->code === self::REQUEST_NOT_ALLOWED) { - throw new Exceptions\AuthenticateException; - } elseif ($error->code === self::CHAT_API_AUTH_ERROR) { - throw new Exceptions\AuthenticateException; - } - - // Rewrite error for Voice API. - if (!empty($error->message)) { - $error->description = $error->message; - unset($error->message); - } - - $this->errors[] = $error; + $this->errors = $body->errors; + + $this->rewriteVoiceApiErrors(); + $this->throwExceptionIfNeeded(); + } + } + + /** + * Voice API returns errors with a "message" field instead of "description". + * This ensures all errors have a description set. + */ + private function rewriteVoiceApiErrors() + { + $errors = []; + + foreach ($this->errors AS $error) { + if (!empty($error->message)) { + $error->description = $error->message; + unset($error->message); } + + $errors[] = $error; } + + $this->errors = $errors; } /** - * Get the error string to show in the Exception message. + * Throw an exception when important errors are found. + * + * @throws Exceptions\AuthenticateException + * @throws Exceptions\BalanceException + */ + private function throwExceptionIfNeeded() + { + foreach ($this->errors AS $error) { + if ($error->code === self::NOT_ENOUGH_CREDIT) { + throw new Exceptions\BalanceException($this->getExceptionMessage()); + } elseif ($error->code === self::REQUEST_NOT_ALLOWED) { + throw new Exceptions\AuthenticateException($this->getExceptionMessage()); + } elseif ($error->code === self::CHAT_API_AUTH_ERROR) { + throw new Exceptions\AuthenticateException($this->getExceptionMessage()); + } + } + } + + /** + * Get the exception message for this response's errors. + * + * @return string + */ + private function getExceptionMessage() + { + return sprintf(self::EXCEPTION_MESSAGE, $this->getErrorString()); + } + + /** + * Get a string of all of this response's concatenated error descriptions. * * @return string */ public function getErrorString() { - $errorString = array (); + $errorDescriptions = array(); + foreach ($this->errors AS $error) { - $errorString[] = $error->description; + $errorDescriptions[] = $error->description; } - return implode(', ', $errorString); + return implode(', ', $errorDescriptions); } } diff --git a/tests/integration/ResponseErrorTest.php b/tests/integration/ResponseErrorTest.php new file mode 100644 index 00000000..0681e361 --- /dev/null +++ b/tests/integration/ResponseErrorTest.php @@ -0,0 +1,41 @@ +assertEquals( + sprintf(self::EXCEPTION_MESSAGE, 'foo'), + $this->getExceptionMessageFromJson(self::SINGLE_ERROR_JSON) + ); + } + + public function testMultipleErrors() + { + $this->assertEquals( + sprintf(self::EXCEPTION_MESSAGE, 'foo, bar'), + $this->getExceptionMessageFromJson(self::MULTIPLE_ERRORS_JSON) + ); + } + + private function getExceptionMessageFromJson($json) + { + try { + new ResponseError(json_decode($json)); + } catch (MessageBirdException $e) { + // Expected: we want the error message. + return $e->getMessage(); + } + + $this->fail('No exception thrown'); + } +} From aeb1ca56ae7cd4890de4e086c5254e410425ecba Mon Sep 17 00:00:00 2001 From: Emile Pels Date: Wed, 4 Jul 2018 12:17:43 +0200 Subject: [PATCH 2/3] Use array() instead of bracket syntax - latter is not supported in PHP 5.3 --- src/MessageBird/Common/ResponseError.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MessageBird/Common/ResponseError.php b/src/MessageBird/Common/ResponseError.php index 27a76ff5..368a3bff 100644 --- a/src/MessageBird/Common/ResponseError.php +++ b/src/MessageBird/Common/ResponseError.php @@ -53,7 +53,7 @@ public function __construct($body) */ private function rewriteVoiceApiErrors() { - $errors = []; + $errors = array(); foreach ($this->errors AS $error) { if (!empty($error->message)) { From a21e7ca9dcefd6e252d7bcf89d6ec8b2ba58efd0 Mon Sep 17 00:00:00 2001 From: Emile Pels Date: Wed, 4 Jul 2018 16:33:33 +0200 Subject: [PATCH 3/3] Change exception message to only contain single, current error --- src/MessageBird/Common/ResponseError.php | 67 ++++++++---------------- tests/integration/ResponseErrorTest.php | 6 +-- 2 files changed, 25 insertions(+), 48 deletions(-) diff --git a/src/MessageBird/Common/ResponseError.php b/src/MessageBird/Common/ResponseError.php index 368a3bff..ee8d9e19 100644 --- a/src/MessageBird/Common/ResponseError.php +++ b/src/MessageBird/Common/ResponseError.php @@ -11,7 +11,7 @@ */ class ResponseError { - const EXCEPTION_MESSAGE = 'Got error response(s) from the server: %s'; + const EXCEPTION_MESSAGE = 'Got error response from the server: %s'; const SUCCESS = 1; @@ -40,60 +40,37 @@ class ResponseError public function __construct($body) { if (!empty($body->errors)) { - $this->errors = $body->errors; - - $this->rewriteVoiceApiErrors(); - $this->throwExceptionIfNeeded(); - } - } - - /** - * Voice API returns errors with a "message" field instead of "description". - * This ensures all errors have a description set. - */ - private function rewriteVoiceApiErrors() - { - $errors = array(); - - foreach ($this->errors AS $error) { - if (!empty($error->message)) { - $error->description = $error->message; - unset($error->message); + foreach ($body->errors AS $error) { + // Voice API returns errors with a "message" field instead of "description". + // This ensures all errors have a description set. + if (!empty($error->message)) { + $error->description = $error->message; + unset($error->message); + } + + if ($error->code === self::NOT_ENOUGH_CREDIT) { + throw new Exceptions\BalanceException($this->getExceptionMessage($error)); + } elseif ($error->code === self::REQUEST_NOT_ALLOWED) { + throw new Exceptions\AuthenticateException($this->getExceptionMessage($error)); + } elseif ($error->code === self::CHAT_API_AUTH_ERROR) { + throw new Exceptions\AuthenticateException($this->getExceptionMessage($error)); + } + + $this->errors[] = $error; } - - $errors[] = $error; } - - $this->errors = $errors; } /** - * Throw an exception when important errors are found. + * Get the exception message for the provided error. * - * @throws Exceptions\AuthenticateException - * @throws Exceptions\BalanceException - */ - private function throwExceptionIfNeeded() - { - foreach ($this->errors AS $error) { - if ($error->code === self::NOT_ENOUGH_CREDIT) { - throw new Exceptions\BalanceException($this->getExceptionMessage()); - } elseif ($error->code === self::REQUEST_NOT_ALLOWED) { - throw new Exceptions\AuthenticateException($this->getExceptionMessage()); - } elseif ($error->code === self::CHAT_API_AUTH_ERROR) { - throw new Exceptions\AuthenticateException($this->getExceptionMessage()); - } - } - } - - /** - * Get the exception message for this response's errors. + * @param $error * * @return string */ - private function getExceptionMessage() + private function getExceptionMessage($error) { - return sprintf(self::EXCEPTION_MESSAGE, $this->getErrorString()); + return sprintf(self::EXCEPTION_MESSAGE, $error->description); } /** diff --git a/tests/integration/ResponseErrorTest.php b/tests/integration/ResponseErrorTest.php index 0681e361..55720d10 100644 --- a/tests/integration/ResponseErrorTest.php +++ b/tests/integration/ResponseErrorTest.php @@ -6,10 +6,10 @@ class ResponseErrorTest extends BaseTest { - const EXCEPTION_MESSAGE = 'Got error response(s) from the server: %s'; + const EXCEPTION_MESSAGE = 'Got error response from the server: %s'; const SINGLE_ERROR_JSON = '{"errors":[{"code":25,"description":"foo"}]}'; - const MULTIPLE_ERRORS_JSON = '{"errors":[{"code":2,"description":"foo"},{"code":25,"description":"bar"}]}'; + const MULTIPLE_ERRORS_JSON = '{"errors":[{"code":9,"description":"foo"},{"code":25,"description":"bar"}]}'; public function testSingleError() { @@ -22,7 +22,7 @@ public function testSingleError() public function testMultipleErrors() { $this->assertEquals( - sprintf(self::EXCEPTION_MESSAGE, 'foo, bar'), + sprintf(self::EXCEPTION_MESSAGE, 'bar'), $this->getExceptionMessageFromJson(self::MULTIPLE_ERRORS_JSON) ); }