InvalidArgumentException thrown when the verification code is expired

[nesk]

Following thephpleague/oauth2-client#476

When the verification code is expired, the response from Github is:

{
    "error": "bad_verification_code",
    "error_description": "The code passed is incorrect or expired.",
    "error_uri": "https://developer.github.com/v3/oauth/#bad-verification-code"
}

Then a new instance of the AccessToken class is created by providing the above response, which fails with an InvalidArgumentException.

It should throw an exception dedicated to this case. Can we agree on how we could handle this? Throwing an instance of a League\OAuth2\Client\Provider\GithubResponseException class containing the error_description field as a message could be a solution.

I can totally create a PR, just waiting for your approval.

[stevenmaguire]

What is the status code that is returned with that error?

[nesk]

I do not really understand the question. Are you talking about the HTTP status code (which seems out of scope for me) or about the exception code (which doesn't really need to be set)?

[stevenmaguire]

It is the http status code I'm interested in.

[stevenmaguire]

There is a bit of code on line 87 of the provider that catches responses whose status code is greater than or equal to 400. If the error message you are trying to catch contains a proper HTTP status code, this exception should be thrown allowing you to inspect the error and handle it accordingly.

[nesk]

Ooooh! Seems like you made a point! The HTTP status code is 200, I will contact Github's support.

[nesk]

Here's their response:

Thanks for getting in touch and pointing this out. Yeah, I think a 200 status is expected there for now -- you should see an error description in the body if a problem happened:

https://developer.github.com/v3/oauth/#common-errors-for-the-access-token-request

Still, I agree that a 200 OK is a bit strange for when a problem happens. I've opened an internal issue so that the API team can track and investigate this, but can't promise an ETA for if/when the behavior might change.

Until they fix this (or not), maybe you should manually check if the response contains an error property and catch the response with your checkResponse method?

[stevenmaguire]

Ok. With this in mind, please put together a PR for this condition and we will plan to remove it if Github changes their API. Thanks for digging into that.

[stevenmaguire]

This is now resolved as of version 0.2.1

cc @nesk