-
Notifications
You must be signed in to change notification settings - Fork 54
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
Feature/exception factory #50
Changes from 4 commits
4fae1d7
5d7e1fb
70a3293
7dcabdf
7599e8f
47f7170
926fd64
84b7ba4
554dd4c
b4a4bb6
c47a4cc
c4acc09
b1f1528
96996a3
5fda0df
c58a94a
33fe8a5
ed61b14
7b91a04
4f25c4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,35 @@ | |
|
||
/** | ||
*/ | ||
class ApiException extends BunqException | ||
class ApiException extends \Exception | ||
{ | ||
/** | ||
* The first item index in an array. | ||
*/ | ||
const INDEX_FIRST = 0; | ||
|
||
/** | ||
* @var int | ||
*/ | ||
private $responseCode; | ||
|
||
/** | ||
* ApiException constructor. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment can go :) |
||
* | ||
* @param int $responseCode | ||
*/ | ||
public function __construct($message, $args = []) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add strict types for arguments (PHP 7.0 style) and doc block params. Also, why are we using |
||
{ | ||
$this->responseCode = $args[self::INDEX_FIRST]; | ||
|
||
parent::__construct(vsprintf($message, $args)); | ||
} | ||
|
||
/** | ||
* @return int | ||
*/ | ||
public function getResponseCode() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add PHP 7.0 strict response type ( |
||
{ | ||
return $this->responseCode; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class BadRequestException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class ExceptionFactory | ||
{ | ||
/** | ||
* HTTP error response codes constants. | ||
*/ | ||
const HTTP_RESPONSE_CODE_BAD_REQUEST = 400; | ||
const HTTP_RESPONSE_CODE_UNAUTHORIZED = 401; | ||
const HTTP_RESPONSE_CODE_FORBIDDEN = 403; | ||
const HTTP_RESPONSE_CODE_NOT_FOUND = 404; | ||
const HTTP_RESPONSE_CODE_METHOD_NOT_ALLOWED = 405; | ||
const HTTP_RESPONSE_CODE_TOO_MANY_REQUESTS = 429; | ||
const HTTP_RESPONSE_CODE_INTERNAL_SERVER_ERROR = 500; | ||
|
||
/** | ||
* The first item index in an array. | ||
*/ | ||
const INDEX_FIRST = 0; | ||
|
||
/** | ||
* @param string $message | ||
* @param array $args | ||
* | ||
* @return BadRequestException|ForbiddenException|MethodNotAllowedException|NotFoundException|PleaseContactBunqException|ToManyRequestsException|UnauthorizedException|UnknownApiErrorException | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just |
||
*/ | ||
public static function createExceptionForResponse($message, $args = []) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question: do we need args here? Also please add strict param types and the response type :) |
||
{ | ||
switch ($args[self::INDEX_FIRST]) | ||
{ | ||
case self::HTTP_RESPONSE_CODE_BAD_REQUEST: | ||
return new BadRequestException($message, $args); | ||
case self::HTTP_RESPONSE_CODE_UNAUTHORIZED: | ||
return new UnauthorizedException($message, $args); | ||
case self::HTTP_RESPONSE_CODE_FORBIDDEN: | ||
return new ForbiddenException($message, $args); | ||
case self::HTTP_RESPONSE_CODE_NOT_FOUND: | ||
return new NotFoundException($message, $args); | ||
case self::HTTP_RESPONSE_CODE_METHOD_NOT_ALLOWED: | ||
return new MethodNotAllowedException($message, $args); | ||
case self::HTTP_RESPONSE_CODE_TOO_MANY_REQUESTS: | ||
return new ToManyRequestsException($message, $args); | ||
case self::HTTP_RESPONSE_CODE_INTERNAL_SERVER_ERROR: | ||
return new PleaseContactBunqException($message, $args); | ||
default: | ||
return new UnknownApiErrorException($message, $args); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class ForbiddenException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class MethodNotAllowedException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class NotFoundException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class PleaseContactBunqException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class ToManyRequestsException extends ApiException | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OMG this is in all the SDK's 😁 🙈 |
||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class UnauthorizedException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
namespace bunq\Exception; | ||
|
||
/** | ||
*/ | ||
class UnknownApiErrorException extends ApiException | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
namespace bunq\Http\Handler; | ||
|
||
use bunq\Exception\ApiException; | ||
use bunq\Exception\ExceptionFactory; | ||
use Psr\Http\Message\ResponseInterface; | ||
|
||
/** | ||
|
@@ -60,15 +61,15 @@ public function execute(ResponseInterface $response) | |
$errorDescriptions[] = $error[self::FIELD_ERROR_DESCRIPTION]; | ||
} | ||
|
||
throw new ApiException( | ||
throw ExceptionFactory::createExceptionForResponse( | ||
self::ERROR_FROM_JSON, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know it is not yours, but please make the interface of |
||
[ | ||
$response->getStatusCode(), | ||
implode(self::SEPERATOR_ERROR, $errorDescriptions), | ||
] | ||
); | ||
} else { | ||
throw new ApiException( | ||
throw ExceptionFactory::createExceptionForResponse( | ||
self::ERROR_UNKNOWN, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, please make error handling here consistent with what we have in e.g. Python :) |
||
[ | ||
$response->getStatusCode(), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us please do
use Exception;
and thenException
with no preceding\
:)