From 5836dfd23725964e4be9d0381beac0b5ec445ec2 Mon Sep 17 00:00:00 2001 From: Andrew McIntosh Date: Wed, 7 Jun 2023 07:15:03 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20=20Support=20new=20API=20version=20?= =?UTF-8?q?error=20format?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While FreshBooks API versions are not yet documented, the recent versions (2022-10-31 and forward) feature a slightly different response format when some /accounting endpoints fail. Update the accounting handlers to handle both formats. --- CHANGELOG.md | 1 + src/Exception/FreshBooksException.php | 10 ++- src/Resource/AccountingResource.php | 70 +++++++++++++++---- src/Resource/ProjectResource.php | 30 ++++++++- tests/Resource/AccountingResourceTest.php | 82 ++++++++++++++++++++--- tests/Resource/ProjectResourceTest.php | 31 +++++++++ 6 files changed, 198 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfd92ce..4efb4c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Remove warnings in PHP 8.2 +- Handle new API version accounting errors ## 0.6.0 diff --git a/src/Exception/FreshBooksException.php b/src/Exception/FreshBooksException.php index ee557ee..81c636d 100644 --- a/src/Exception/FreshBooksException.php +++ b/src/Exception/FreshBooksException.php @@ -8,18 +8,21 @@ final class FreshBooksException extends \Exception { public ?string $rawResponse; public ?int $errorCode; + public ?array $errorDetails; public function __construct( string $message, int $statusCode, Throwable $previous = null, string $rawResponse = null, - int $errorCode = null + int $errorCode = null, + array $errorDetails = null ) { parent::__construct($message, $statusCode, $previous); $this->rawResponse = $rawResponse; $this->errorCode = $errorCode; + $this->errorDetails = $errorDetails; } public function getRawResponse(): ?string @@ -31,4 +34,9 @@ public function getErrorCode(): ?int { return $this->errorCode; } + + public function getErrorDetails(): ?array + { + return $this->errorDetails; + } } diff --git a/src/Resource/AccountingResource.php b/src/Resource/AccountingResource.php index 77b8f6b..500826b 100644 --- a/src/Resource/AccountingResource.php +++ b/src/Resource/AccountingResource.php @@ -78,16 +78,16 @@ private function makeRequest(string $method, string $url, array $data = null): a throw new FreshBooksException('Failed to parse response', $statusCode, $e, $contents); } + if ($statusCode >= 400) { + $this->handleError($statusCode, $responseData, $contents); + } + if (is_null($responseData) || !array_key_exists('response', $responseData)) { throw new FreshBooksException('Returned an unexpected response', $statusCode, null, $contents); } $responseData = $responseData['response']; - if ($statusCode >= 400) { - $this->createResponseError($statusCode, $responseData, $contents); - } - if (array_key_exists('result', $responseData)) { return $responseData['result']; } @@ -95,27 +95,71 @@ private function makeRequest(string $method, string $url, array $data = null): a } /** - * Parse the json response from the accounting endpoint and create a FreshBooksException from it. + * Parse the json response for old-style accounting endpoint errors and create a FreshBooksException from it. * * @param int $statusCode HTTP status code * @param array $responseData The json-parsed response * @param string $rawRespone The raw response body * @return void */ - private function createResponseError(int $statusCode, array $responseData, string $rawRespone): void + private function createOldResponseError(int $statusCode, array $responseData, string $rawRespone): void { - if (!array_key_exists('errors', $responseData)) { - throw new FreshBooksException('Unknown error', $statusCode, null, $rawRespone); - } - $errors = $responseData['errors']; + $errors = $responseData['response']['errors']; if (array_key_exists(0, $errors)) { - $message = $errors[0]['message'] ?? 'Unknown error2'; + $message = $errors[0]['message'] ?? 'Unknown error'; $errorCode = $errors[0]['errno'] ?? null; - throw new FreshBooksException($message, $statusCode, null, $rawRespone, $errorCode); + throw new FreshBooksException($message, $statusCode, null, $rawRespone, $errorCode, $errors); } + $message = $errors['message'] ?? 'Unknown error'; $errorCode = $errors['errno'] ?? null; - throw new FreshBooksException($message, $statusCode, null, $rawRespone, $errorCode); + throw new FreshBooksException($message, $statusCode, null, $rawRespone, $errorCode, $errors); + } + + /** + * Parse the json response for new-style accounting endpoint errors and create a FreshBooksException from it. + * + * @param int $statusCode HTTP status code + * @param array $responseData The json-parsed response + * @param string $rawRespone The raw response body + * @return void + */ + private function createNewResponseError(int $statusCode, array $responseData, string $rawRespone): void + { + $message = $responseData['message']; + $details = []; + + foreach ($responseData['details'] as $detail) { + if (in_array('type.googleapis.com/google.rpc.ErrorInfo', $detail)) { + $errorCode = intval($detail['reason']) ?? null; + if (array_key_exists('metadata', $detail)) { + $details[] = $detail['metadata']; + if (array_key_exists('message', $detail['metadata'])) { + $message = $detail['metadata']['message']; + } + } + } + } + throw new FreshBooksException($message, $statusCode, null, $rawRespone, $errorCode, $details); + } + + /** + * Create a FreshBooksException from the json response from the accounting endpoint. + * + * @param int $statusCode HTTP status code + * @param array $responseData The json-parsed response + * @param string $rawRespone The raw response body + * @return void + */ + private function handleError(int $statusCode, array $responseData, string $rawRespone): void + { + if (array_key_exists('response', $responseData) && array_key_exists('errors', $responseData['response'])) { + $this->createOldResponseError($statusCode, $responseData, $rawRespone); + } elseif (array_key_exists('message', $responseData) && array_key_exists('code', $responseData)) { + $this->createNewResponseError($statusCode, $responseData, $rawRespone); + } else { + throw new FreshBooksException('Unknown error', $statusCode, null, $rawRespone); + } } private function rejectMissing(string $name): void diff --git a/src/Resource/ProjectResource.php b/src/Resource/ProjectResource.php index 20947b2..b033247 100644 --- a/src/Resource/ProjectResource.php +++ b/src/Resource/ProjectResource.php @@ -53,6 +53,32 @@ private function getUrl(int $businessId, int $resourceId = null, bool $isList = return "/projects/business/{$businessId}/{$this->singleResourcePath}"; } + /** + * Parse the json response for project endpoint errors and create a FreshBooksException from it. + * + * @param int $statusCode HTTP status code + * @param array $responseData The json-parsed response + * @param string $rawRespone The raw response body + * @return void + */ + private function createResponseError(int $statusCode, array $responseData, string $rawRespone): void + { + $message = $responseData['message'] ?? "Unknown error"; + $errorCode = $responseData['code'] ?? null; + $errorDetails = null; + + if (array_key_exists('error', $responseData) && is_array($responseData['error'])) { + $errorDetails = []; + foreach ($responseData['error'] as $errorKey => $errorDetail) { + $errorDetails[] = [$errorKey => $errorDetail]; + $message = 'Error: ' . $errorKey . ' ' . $errorDetail; + } + } elseif (array_key_exists('error', $responseData)) { + $message = $responseData['error']; + } + throw new FreshBooksException($message, $statusCode, null, $rawRespone, $errorCode, $errorDetails); + } + /** * Make a request against the accounting resource and return an array of the json response. * Throws a FreshBooksException if the response is not a 200 or if the response cannot be parsed. @@ -81,9 +107,7 @@ private function makeRequest(string $method, string $url, array $data = null): a } if ($statusCode >= 400) { - $errorCode = $responseData['code'] ?? null; - $message = $responseData['message'] ?? $responseData['error'] ?? "Unknown error"; - throw new FreshBooksException($message, $statusCode, null, $contents, $errorCode); + $this->createResponseError($statusCode, $responseData, $contents); } if ( !array_key_exists($this->singleModel::RESPONSE_FIELD, $responseData) && diff --git a/tests/Resource/AccountingResourceTest.php b/tests/Resource/AccountingResourceTest.php index b2c15c3..42545cb 100644 --- a/tests/Resource/AccountingResourceTest.php +++ b/tests/Resource/AccountingResourceTest.php @@ -88,30 +88,94 @@ public function testGetWrongErrorContent(): void $resource = new AccountingResource($mockHttpClient, 'users/clients', Client::class, ClientList::class); $this->expectException(FreshBooksException::class); - $this->expectExceptionMessage('Returned an unexpected response'); + $this->expectExceptionMessage('Unknown error'); $resource->get($this->accountId, $clientId); } - public function testGetNoPermission(): void + public function testGetNotFoundOldError(): void { $clientId = 12345; $mockHttpClient = $this->getMockHttpClient( - 401, + 404, ['response' => ['errors' => [[ - 'message' => 'The server could not verify that you are authorized to access the URL requested.', - 'errno' => 1003 + 'errno' => 1012, + 'field' => 'userid', + 'message' => 'Client not found.', + 'object' => 'client', + 'value' => '12345' ]]]] ); $resource = new AccountingResource($mockHttpClient, 'users/clients', Client::class, ClientList::class); - $this->expectException(FreshBooksException::class); - $this->expectExceptionMessage( - 'The server could not verify that you are authorized to access the URL requested.' + try { + $resource->get($this->accountId, $clientId); + $this->fail('FreshBooksException was not thrown'); + } catch (FreshBooksException $e) { + $this->assertSame('Client not found.', $e->getMessage()); + $this->assertSame(404, $e->getCode()); + $this->assertSame(1012, $e->getErrorCode()); + $this->assertSame( + [ + [ + 'errno' => 1012, + 'field' => 'userid', + 'message' => 'Client not found.', + 'object' => 'client', + 'value' => '12345' + ] + ], + $e->getErrorDetails() + ); + } + } + + public function testGetNotFoundNewError(): void + { + $clientId = 12345; + $mockHttpClient = $this->getMockHttpClient( + 404, + [ + 'code' => 5, + 'message' => 'Request failed with status_code: 404', + 'details' => [ + [ + '@type' => 'type.googleapis.com/google.rpc.ErrorInfo', + 'reason' => '1012', + 'domain' => 'accounting.api.freshbooks.com', + 'metadata' => [ + 'object' => 'client', + 'message' => 'Client not found.', + 'value' => '12345', + 'field' => 'userid' + ] + ] + ] + ] ); - $resource->get($this->accountId, $clientId); + $resource = new AccountingResource($mockHttpClient, 'users/clients', Client::class, ClientList::class); + + try { + $resource->get($this->accountId, $clientId); + $this->fail('FreshBooksException was not thrown'); + } catch (FreshBooksException $e) { + $this->assertSame('Client not found.', $e->getMessage()); + $this->assertSame(404, $e->getCode()); + $this->assertSame(1012, $e->getErrorCode()); + $this->assertSame( + [ + [ + 'object' => 'client', + 'message' => 'Client not found.', + 'value' => '12345', + 'field' => 'userid' + ] + ], + $e->getErrorDetails() + ); + } } public function testList(): void diff --git a/tests/Resource/ProjectResourceTest.php b/tests/Resource/ProjectResourceTest.php index d589e58..1e09506 100644 --- a/tests/Resource/ProjectResourceTest.php +++ b/tests/Resource/ProjectResourceTest.php @@ -310,6 +310,37 @@ public function testCreateWithIncludes(): void ); } + public function testCreateValidationErrors(): void + { + $mockHttpClient = $this->getMockHttpClient( + 422, + [ + 'errno' => 2001, + 'error' => [ + 'title' => 'field required', + 'description' => 'field required' + ] + ] + ); + + $resource = new ProjectResource($mockHttpClient, 'projects', 'projects', Project::class, ProjectList::class); + + try { + $resource->create($this->businessId, data: []); + $this->fail('FreshBooksException was not thrown'); + } catch (FreshBooksException $e) { + $this->assertSame('Error: description field required', $e->getMessage()); + $this->assertSame(422, $e->getCode()); + $this->assertSame( + [ + ['title' => 'field required'], + ['description' => 'field required'] + ], + $e->getErrorDetails() + ); + } + } + public function testUpdateByModel(): void { $projectId = 12345;