OAuth2 token fetch crashes with JSONDecodeError instead of a catchable error on rate-limit responses - #929
Open
juneja-varun wants to merge 2 commits into
Open
OAuth2 token fetch crashes with JSONDecodeError instead of a catchable error on rate-limit responses#929juneja-varun wants to merge 2 commits into
juneja-varun wants to merge 2 commits into
Conversation
Author
|
@lepture the failing tests were a real gap on my end — this repo enforces 100% diff coverage and my fix's re-raise branch (only reachable on a 2xx response with a malformed body) wasn't covered. Added a test for that case and confirmed locally: full suite passes (954 passed, 4 skipped), diff coverage is 100%, and ruff is clean. Could you approve the workflow run again when you get a chance? |
Author
|
Just following up — let me know if there's anything else needed on my end before the workflow can be re-approved. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #928.
I'm calling a token endpoint behind an API gateway. When the gateway rate-limits me, it sends back a 429 with a plain-text body - completely normal gateway behavior - but
fetch_token()crashed with an unhandledjson.decoder.JSONDecodeErrorinstead of something catchable.OAuth2Client.parse_response_token()only callsresp.raise_for_status()for status codes >= 500, so any 4xx with a non-JSON body (rate-limit pages, WAF blocks, plain text) blows up trying to parse it as JSON. Fixed by wrapping theresp.json()call in a try/except and falling back toraise_for_status()on a decode failure, so the caller gets a properHTTPStatusErrorinstead. Kept the normal OAuth2 flow intact - a 400 with a well-formed JSON error body still raises the expectedOAuthError, verified explicitly.Added a regression test confirming it fails with the exact reported error on unpatched code and passes with the fix.