Skip to content

Commit

Permalink
Merge pull request #246 from daniel-sc/correct-handling-on-error-with…
Browse files Browse the repository at this point in the history 
…-streams

fix: handle http errors correctly when using return_raw_as_resource_only
  • Loading branch information
@daniel-sc
daniel-sc authored Feb 22, 2024
2 parents 64468c9 + a7974f0 commit 2b69aec
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 17 deletions.
6 changes: 5 additions & 1 deletion CHANGES.md
View file
Original file line number Diff line number Diff line change
@@ -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

Expand Down
34 changes: 18 additions & 16 deletions lib/PodioClient.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
72 changes: 72 additions & 0 deletions tests/PodioClientTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2b69aec

Please sign in to comment.