From 58eb37d1d7751a0d3cea9032c04803173d1a1689 Mon Sep 17 00:00:00 2001 From: Daniel Schreiber Date: Tue, 13 Feb 2024 21:36:00 +0100 Subject: [PATCH] fix: handle http errors correctly when using return_raw_as_resource_only --- CHANGES.md | 6 ++++- lib/PodioClient.php | 32 ++++++++++++----------- tests/PodioClientTest.php | 53 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f2af656..192d39a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,4 +1,8 @@ -[7.0.2](#v7.0.2) / [UNRELEASED] +[7.0.3](#v7.0.2) / [UNRELEASED] +================== +* Bugfix: handle http errors correctly when using `return_raw_as_resource_only` to stream responses. + +[7.0.2](#v7.0.2) / [2023-11-21] ================== * Bugfix: Correct handling of `null` values for date/datetime fields. #244 diff --git a/lib/PodioClient.php b/lib/PodioClient.php index 432363e..b638a6d 100644 --- a/lib/PodioClient.php +++ b/lib/PodioClient.php @@ -298,17 +298,19 @@ public function request($method, $url, $attributes = [], $options = []) return $response; case 400: // invalid_grant_error or bad_request_error - $body = $response->json_body(); + $body_str = $response->body ?? $http_response->getBody()->getContents(); + $body = json_decode($body_str, true); if (strstr($body['error'], 'invalid_grant')) { // Reset access token & refresh_token $this->clear_authentication(); - throw new PodioInvalidGrantError($response->body, $response->status, $url); + throw new PodioInvalidGrantError($body_str, $response->status, $url); } else { - throw new PodioBadRequestError($response->body, $response->status, $url); + throw new PodioBadRequestError($body_str, $response->status, $url); } // no break case 401: - $body = $response->json_body(); + $body_str = $response->body ?? $http_response->getBody()->getContents(); + $body = json_decode($body_str, true); if (strstr($body['error_description'], 'expired_token') || strstr($body['error'], 'invalid_token')) { if ($this->oauth->refresh_token) { // Access token is expired. Try to refresh it. @@ -317,37 +319,37 @@ public function request($method, $url, $attributes = [], $options = []) return $this->request($method, $original_url, $attributes); } else { $this->clear_authentication(); - throw new PodioAuthorizationError($response->body, $response->status, $url); + throw new PodioAuthorizationError($body_str, $response->status, $url); } } else { // We have tried in vain to get a new access token. Log the user out. $this->clear_authentication(); - throw new PodioAuthorizationError($response->body, $response->status, $url); + throw new PodioAuthorizationError($body_str, $response->status, $url); } } elseif (strstr($body['error'], 'invalid_request') || strstr($body['error'], 'unauthorized')) { // Access token is invalid. $this->clear_authentication(); - throw new PodioAuthorizationError($response->body, $response->status, $url); + throw new PodioAuthorizationError($body_str, $response->status, $url); } break; case 403: - throw new PodioForbiddenError($response->body, $response->status, $url); + throw new PodioForbiddenError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); case 404: - throw new PodioNotFoundError($response->body, $response->status, $url); + throw new PodioNotFoundError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); case 409: - throw new PodioConflictError($response->body, $response->status, $url); + throw new PodioConflictError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); case 410: - throw new PodioGoneError($response->body, $response->status, $url); + throw new PodioGoneError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); case 420: - throw new PodioRateLimitError($response->body, $response->status, $url); + throw new PodioRateLimitError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); case 500: - throw new PodioServerError($response->body, $response->status, $url); + throw new PodioServerError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); case 502: case 503: case 504: - throw new PodioUnavailableError($response->body, $response->status, $url); + throw new PodioUnavailableError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); default: - throw new PodioError($response->body, $response->status, $url); + throw new PodioError($response->body ?? $http_response->getBody()->getContents(), $response->status, $url); } return false; } diff --git a/tests/PodioClientTest.php b/tests/PodioClientTest.php index 7a1204a..90fdbea 100644 --- a/tests/PodioClientTest.php +++ b/tests/PodioClientTest.php @@ -4,6 +4,7 @@ use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; +use GuzzleHttp\Psr7\Utils; use PodioClient; use PHPUnit\Framework\TestCase; @@ -57,6 +58,58 @@ public function test_clear_authentication_sets_session_manger_auth() $client->clear_authentication(); } + + public function test_throw_exception_on_400() + { + $client = new PodioClient('test-client', 'test-secret'); + $httpClientMock = $this->createMock(Client::class); + $httpClientMock->method('send')->willReturn(new Response(400, [], Utils::streamFor('{"error": "some reason"}'))); + $client->http_client = $httpClientMock; + + $this->expectException(\PodioBadRequestError::class); + $client->get('/test'); + } + + public function test_throw_exception_on_400_when_return_raw_as_resource_only_is_true() + { + $client = new PodioClient('test-client-id', 'test-client-secret'); + $httpClientMock = $this->createMock(Client::class); + $httpClientMock->method('send')->willReturn(new Response(400, [], Utils::streamFor('{"error": "some reason"}'))); + $client->http_client = $httpClientMock; + + $this->expectException(\PodioBadRequestError::class); + $client->get('/test', [], ['return_raw_as_resource_only' => true]); + } + + public function test_throw_exception_with_body_on_500() + { + $client = new PodioClient('test-client-id', 'test-client-secret'); + $httpClientMock = $this->createMock(Client::class); + $httpClientMock->method('send')->willReturn(new Response(500, [], Utils::streamFor('{"error": "some reason"}'))); + $client->http_client = $httpClientMock; + + try { + $client->get('/test'); + $this->fail('Exception not thrown'); + } catch (\PodioServerError $e) { + $this->assertEquals(['error' => 'some reason'], $e->body); + } + } + + public function test_throw_exception_with_body_on_500_when_return_raw_as_resource_only_is_true() + { + $client = new PodioClient('test-client-id', 'test-client-secret'); + $httpClientMock = $this->createMock(Client::class); + $httpClientMock->method('send')->willReturn(new Response(500, [], Utils::streamFor('{"error": "some reason"}'))); + $client->http_client = $httpClientMock; + + try { + $client->get('/test', [], ['return_raw_as_resource_only' => true]); + $this->fail('Exception not thrown'); + } catch (\PodioServerError $e) { + $this->assertEquals(['error' => 'some reason'], $e->body); + } + } } class TestSessionManager