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..344984e 100644 --- a/lib/PodioClient.php +++ b/lib/PodioClient.php @@ -298,56 +298,58 @@ 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. if ($this->refresh_access_token()) { // Try the original request again. - return $this->request($method, $original_url, $attributes); + return $this->request($method, $original_url, $attributes, $options); } 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..ef6af67 100644 --- a/tests/PodioClientTest.php +++ b/tests/PodioClientTest.php @@ -4,8 +4,10 @@ use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; +use GuzzleHttp\Psr7\Utils; use PodioClient; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\StreamInterface; class PodioClientTest extends TestCase { @@ -57,6 +59,76 @@ public function test_clear_authentication_sets_session_manger_auth() $client->clear_authentication(); } + + public function test_should_retry_after_401_and_token_refresh_and_return_stream() + { + $httpClientMock = $this->createMock(Client::class); + $httpClientMock->expects($this->exactly(3))->method('send')->willReturnOnConsecutiveCalls( + new Response(401, [], Utils::streamFor('{"error_description": "expired_token"}')), + new Response(200, [], Utils::streamFor('{"access_token": "new-token", "refresh_token": "new-refresh-token", "expires_in": 42000, "ref": "test-ref", "scope": "test-scope"}')), + new Response(200, [], Utils::streamFor('{"items": []}')) + ); + $client = new PodioClient('test-client', 'test-secret'); + $client->oauth = new \PodioOAuth('test-token', 'test-refresh-token'); + $client->http_client = $httpClientMock; + + $result = $client->request('GET', '/test', [], ['return_raw_as_resource_only' => true]); + + $this->assertInstanceOf(StreamInterface::class, $result); + $this->assertEquals('{"items": []}', $result->getContents()); + } + + 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