-
Notifications
You must be signed in to change notification settings - Fork 760
Introduce ResponseParsingException and throw this exception in … #825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 8 commits
4867539
c86cf24
6f08509
d4f856c
18131dc
6d3d013
2281364
1548683
5843122
b7841a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
use League\OAuth2\Client\OptionProvider\OptionProviderInterface; | ||
use League\OAuth2\Client\OptionProvider\PostAuthOptionProvider; | ||
use League\OAuth2\Client\Provider\Exception\IdentityProviderException; | ||
use League\OAuth2\Client\Provider\Exception\ResponseParsingException; | ||
use League\OAuth2\Client\Token\AccessToken; | ||
use League\OAuth2\Client\Token\AccessTokenInterface; | ||
use League\OAuth2\Client\Tool\ArrayAccessorTrait; | ||
|
@@ -98,6 +99,15 @@ abstract class AbstractProvider | |
*/ | ||
protected $optionProvider; | ||
|
||
/** | ||
* If a response body cannot be parsed, a value true of this flag will allow | ||
* the response parser to throw a ResponseParsingException containing | ||
* the response and the body. | ||
* | ||
* @var bool | ||
*/ | ||
protected $mayThrowResponseParsingException = false; | ||
|
||
/** | ||
* Constructs an OAuth 2.0 service provider. | ||
* | ||
|
@@ -520,6 +530,8 @@ protected function getAccessTokenRequest(array $params) | |
* @param mixed $grant | ||
* @param array $options | ||
* @throws IdentityProviderException | ||
* @throws ResponseParsingException if the flag $mayThrowResponseParsingException is true and | ||
* response body cannot be parsed. | ||
* @return AccessTokenInterface | ||
*/ | ||
public function getAccessToken($grant, array $options = []) | ||
|
@@ -613,6 +625,8 @@ public function getResponse(RequestInterface $request) | |
* | ||
* @param RequestInterface $request | ||
* @throws IdentityProviderException | ||
* @throws ResponseParsingException if the flag $mayThrowResponseParsingException is true and | ||
* response body cannot be parsed. | ||
* @return mixed | ||
*/ | ||
public function getParsedResponse(RequestInterface $request) | ||
|
@@ -666,8 +680,10 @@ protected function getContentType(ResponseInterface $response) | |
* Parses the response according to its content-type header. | ||
* | ||
* @throws UnexpectedValueException | ||
* @throws ResponseParsingException if the flag $mayThrowResponseParsingException is true and | ||
* response body cannot be parsed. | ||
* @param ResponseInterface $response | ||
* @return array | ||
* @return array|string | ||
*/ | ||
protected function parseResponse(ResponseInterface $response) | ||
{ | ||
|
@@ -686,18 +702,26 @@ protected function parseResponse(ResponseInterface $response) | |
return $this->parseJson($content); | ||
} catch (UnexpectedValueException $e) { | ||
if (strpos($type, 'json') !== false) { | ||
throw $e; | ||
throw $this->mayThrowResponseParsingException | ||
? new ResponseParsingException($response, $content, $e->getMessage(), $e->getCode()) | ||
tandhika marked this conversation as resolved.
Show resolved
Hide resolved
|
||
: $e; | ||
} | ||
|
||
if ($response->getStatusCode() == 500) { | ||
throw new UnexpectedValueException( | ||
'An OAuth server error was encountered that did not contain a JSON body', | ||
0, | ||
$e | ||
); | ||
// for any other content types | ||
if ($this->mayThrowResponseParsingException) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we always throw the exception, no matter what? I know this suggestion is somewhat of a BC break, but if you have @shadowhand, @stevenmaguire, thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like your suggestion. I can implement the changes if no objection coming from @shadowhand and @stevenmaguire |
||
// let the calling function decide what to do with the response and its body | ||
throw new ResponseParsingException($response, $content, '', 0); | ||
} else { | ||
if ($response->getStatusCode() == 500) { | ||
throw new UnexpectedValueException( | ||
'An OAuth server error was encountered that did not contain a JSON body', | ||
0, | ||
$e | ||
); | ||
} | ||
|
||
return $content; | ||
} | ||
|
||
return $content; | ||
} | ||
} | ||
|
||
|
@@ -774,6 +798,8 @@ public function getResourceOwner(AccessToken $token) | |
* | ||
* @param AccessToken $token | ||
* @return mixed | ||
* @throws ResponseParsingException if the flag $mayThrowResponseParsingException is true and | ||
* response body cannot be parsed. | ||
*/ | ||
protected function fetchResourceOwnerDetails(AccessToken $token) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
<?php | ||
/** | ||
* This file is part of the league/oauth2-client library | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
* | ||
* @copyright Copyright (c) Alex Bilbie <hello@alexbilbie.com> | ||
* @license http://opensource.org/licenses/MIT MIT | ||
* @link http://thephpleague.com/oauth2-client/ Documentation | ||
* @link https://packagist.org/packages/league/oauth2-client Packagist | ||
* @link https://github.com/thephpleague/oauth2-client GitHub | ||
*/ | ||
|
||
namespace League\OAuth2\Client\Provider\Exception; | ||
|
||
use Exception; | ||
use Psr\Http\Message\ResponseInterface; | ||
|
||
/** | ||
* Exception thrown if the parser cannot parse the provider response. | ||
*/ | ||
class ResponseParsingException extends Exception | ||
tandhika marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
/** | ||
* @var ResponseInterface | ||
*/ | ||
protected $response; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
protected $responseBody; | ||
|
||
/** | ||
* @param ResponseInterface $response The response | ||
* @param string $responseBody The response body | ||
* @param null $message | ||
* @param int $code | ||
*/ | ||
public function __construct( | ||
ResponseInterface $response, | ||
$responseBody, | ||
$message = null, | ||
$code = 0 | ||
tandhika marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) { | ||
$this->response = $response; | ||
$this->responseBody = $responseBody; | ||
|
||
if (null === $message) { | ||
$message = sprintf('Cannot parse response body: "%s"', $responseBody); | ||
} | ||
|
||
parent::__construct($message, $code); | ||
tandhika marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Returns the exception's response. | ||
* | ||
* @return ResponseInterface | ||
*/ | ||
public function getResponse() | ||
{ | ||
return $this->response; | ||
} | ||
|
||
/** | ||
* Returns the exception's response body. | ||
* | ||
* @return string | ||
*/ | ||
public function getResponseBody() | ||
{ | ||
return $this->responseBody; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<?php | ||
|
||
namespace League\OAuth2\Client\Test\Provider\Exception; | ||
|
||
use GuzzleHttp\Psr7\Response; | ||
use League\OAuth2\Client\Provider\Exception\ResponseParsingException; | ||
use PHPUnit\Framework\TestCase; | ||
use Psr\Http\Message\ResponseInterface; | ||
|
||
class ResponseParsingExceptionTest extends TestCase | ||
{ | ||
private $body = 'foo'; | ||
|
||
protected function generateResponseParsingException() | ||
{ | ||
return new ResponseParsingException(new Response('401'), $this->body); | ||
} | ||
|
||
public function testGetResponse() | ||
{ | ||
$this->assertInstanceOf( | ||
ResponseInterface::class, | ||
$this->generateResponseParsingException()->getResponse() | ||
); | ||
} | ||
|
||
public function testGetResponseBody() | ||
{ | ||
$this->assertSame( | ||
$this->body, | ||
$this->generateResponseParsingException()->getResponseBody() | ||
); | ||
} | ||
|
||
public function testMissingMessage() | ||
{ | ||
$this->assertNotEmpty($this->generateResponseParsingException()->getMessage()); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.