fix: handle http errors correctly when using return_raw_as_resource_only

podio-community · Feb 13, 2024 · 58eb37d · 58eb37d
1 parent 64468c9
commit 58eb37d
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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