Fail-fast on server errors.

[kousu]

Most OAuth providers give HTTP error codes if something goes wrong. This calls raise_for_status() instead of failing with MissingTokenError in that case, which is uninformative.

The call is after the blocks of debug statements so that you can still figure out what went wrong, if you know where to look.

[Lukasa]

We very deliberately removed raise_for_status() there because some providers send 4XX errors that contain needed data. If you change this code to raise on 5XX errors, that would be substantially better.

[kousu]

Thanks for the explanation! That answers my implicit question about why this line was missing, then.

I ran into this when I was slightly misusing redirect_uri with Facebook. By turning on logging I could see "Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request", but that message gets buried the way the code is now. I would like to find a way to wrap these errors into python exceptions.

Here's the full reply. You can see that Facebook gives the message in both WWW-Authenticate and the JSON response. This isn't standardized, right?

DEBUG:requests_oauthlib.oauth2_session:Request to fetch token completed with status 400.
DEBUG:requests_oauthlib.oauth2_session:Request headers were {'Accept-Encoding': 'gzip, deflate', 'Content-Length': '911', 'User-Agent': 'python-requests/2.9.1', 'Connection': 'keep-alive', 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8', 'Accept': 'application/json', 'Authorization': 'Basic Tm9uZTpOb25l'}
DEBUG:requests_oauthlib.oauth2_session:Request body was code=AQCah260nnLHbZC3MLzF5PRmu5A7xDkydoh5LCyNBEJrPA3TCKmSGyTfB0dIEmRWr0xctvUOXHqyBZQwFp_qwibzlyjDa-OSF_yxsa1bFAszMKOIdOyYBYoLDZoTrUBazvKjdqwyrIjyi5u9kuTtTjUhgbgaCXd1vEB-fVx7Vc--4Aripk3p1jCYRDDXaBYGazGP8nO07_g2TOTohFeFr_AA2-4mybpjQx64FqH1TYW_O69k-AWdDefbYwgoK3mMGEcblnwm9OzXe998l2JN2XM6voY2Czzm5ojMeNH34yxONEZBhDgSbBohY99MBuKjWImGafU65pmdhd0hVW5PB93h&client_secret=e4f78a37b600c986d425cf5c2fc7860e&client_id=939310079547587&grant_type=authorization_code&redirect_uri=https%3A%2F%2Flocalhost%3A5000%2Foauth2%2Ffacebook%3Fcode%3DAQCah260nnLHbZC3MLzF5PRmu5A7xDkydoh5LCyNBEJrPA3TCKmSGyTfB0dIEmRWr0xctvUOXHqyBZQwFp_qwibzlyjDa-OSF_yxsa1bFAszMKOIdOyYBYoLDZoTrUBazvKjdqwyrIjyi5u9kuTtTjUhgbgaCXd1vEB-fVx7Vc--4Aripk3p1jCYRDDXaBYGazGP8nO07_g2TOTohFeFr_AA2-4mybpjQx64FqH1TYW_O69k-AWdDefbYwgoK3mMGEcblnwm9OzXe998l2JN2XM6voY2Czzm5ojMeNH34yxONEZBhDgSbBohY99MBuKjWImGafU65pmdhd0hVW5PB93h%26state%3DdKwz70jGs8TOroRrZkiujH5IODZ3Tr
DEBUG:requests_oauthlib.oauth2_session:Response headers were {'Date': 'Tue, 05 Jan 2016 11:48:03 GMT', 'Content-Length': '217', 'Connection': 'keep-alive', 'WWW-Authenticate': 'OAuth "Facebook Platform" "invalid_code" "Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request"', 'Access-Control-Allow-Origin': '*', 'Pragma': 'no-cache', 'X-FB-Trace-ID': 'BRwuHRXlNB8', 'Facebook-API-Version': 'v2.0', 'X-FB-Rev': '2115307', 'Cache-Control': 'no-store', 'Expires': 'Sat, 01 Jan 2000 00:00:00 GMT', 'Content-Type': 'application/json', 'X-FB-Debug': 'fFpm8W9AqmhBBtiYld5TbW6bNv74RJKvgrL7skd4UXAbRukDGCKEIxna0CMAC9RCmuulxHaKPI1GQN0ZSO2Nbg=='} and content {"error":{"message":"Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request","type":"OAuthException","code":100,"fbtrace_id":"BRwuHRXlNB8"}}.
DEBUG:requests_oauthlib.oauth2_session:Invoking 1 token response hooks.
DEBUG:requests_oauthlib.oauth2_session:Invoking hook <function facebook_compliance_fix.<locals>._compliance_fix at 0x7f70f0d2b7b8>.
INFO:werkzeug:127.0.0.1 - - [05/Jan/2016 06:51:33] "GET /oauth2/facebook?code=AQCah260nnLHbZC3MLzF5PRmu5A7xDkydoh5LCyNBEJrPA3TCKmSGyTfB0dIEmRWr0xctvUOXHqyBZQwFp_qwibzlyjDa-OSF_yxsa1bFAszMKOIdOyYBYoLDZoTrUBazvKjdqwyrIjyi5u9kuTtTjUhgbgaCXd1vEB-fVx7Vc--4Aripk3p1jCYRDDXaBYGazGP8nO07_g2TOTohFeFr_AA2-4mybpjQx64FqH1TYW_O69k-AWdDefbYwgoK3mMGEcblnwm9OzXe998l2JN2XM6voY2Czzm5ojMeNH34yxONEZBhDgSbBohY99MBuKjWImGafU65pmdhd0hVW5PB93h&state=dKwz70jGs8TOroRrZkiujH5IODZ3Tr HTTP/1.1" 500 -

It would be a lot more useful I could get that OAuthException python-side!

Which providers are giving data on 4xxs? Could they be handled with a compliance_fix? Here's a sketch of my idea: you could write a hook for the gammy providers which rewrite the hopefully small list of 4xxs to 200s (you might even be able to get away with simply r.status_code=200). Further, you could write a hook which catches and translates provider-specific error messages into a more specific exception. If after all that there's still an unhandled error code, then raise it as an HTTPError:

+def falseerror_hook(r):
+    r.status_code = 200
+    return r
[...]
+def facebook_error_hook(r):
+    if r.status_code == 400:
+        exc = r.json()['error']
+        exc = ERRORS[exc['type']](exc['code'], exc['message'])
+        raise exc
+    return r
[...]

def fetch_token():
[...]
        for hook in self.compliance_hook['access_token_response']:
            log.debug('Invoking hook %s.', hook)
            r = hook(r)
+      r.raise_for_status()
[...]

[sigmavirus24]

@kousu I think that's a fundamentally bad idea (changing a status code).

The library could provide more information in the exception instance to help you figure out what's available (or the Facebook integration could have a compliance fix for having the exception message in WWW-Authenticate) but I don't think introducing compliance fixes that quietly change status codes without preserving information is the right way of "fixing" services that implement a protocol that allows for such wild variations. When you do that, you're punishing the user instead of giving them the correct information via exceptions, etc.

tl;dr Improving information discoverable on exceptions: 👍 ; Hiding information about responses by changing attributes and information on the response: very strong 👎

[singingwolfboy]

Since this pull request hasn't had any activity in over two years, I'm closing it.