Repository navigation
Conversation
Mariatta
force-pushed
the
handle-oauth-exchange-errors
branch
3 times, most recently
from
October 25, 2026 00:43
3990100 to
dbdb3b0
Compare
The OAuth callback leg of /add_blurb posted the code to GitHub's token endpoint and assumed the exchange worked. When GitHub answered with an outage or interstitial HTML page instead, its Content-Security-Policy header exceeded aiohttp's 8190 byte header limit and the request crashed with LineTooLong, which Sentry reported as a 500. Move the exchange into exchange_code_for_token(), which asks for JSON and sorts failures into three cases: - GitHubUnavailable: transport errors such as the oversized header, timeouts, 429, 5xx, or a non-JSON body. The handler renders a 503 that tells the user GitHub is not responding, and the error template adds a link to GitHub's status page on 503 pages. Outage errors from the follow-up /user lookup and the App installation check get the same page, and so does the already-signed-in branch of the same route, which checks the App installation on every visit and previously had no guard at all. - SignInFailed: GitHub is up but rejects the code because it expired or was reused, or refuses the token it just issued. The handler renders a 400 quoting GitHub's reason and asks the user to sign in again. - OAuthMisconfigured: any other error code from the token endpoint, such as incorrect_client_credentials. Only a deploy can fix those, so they are left to surface as a 500 that Sentry reports. Both branches share one require_app_installed() helper, and their sessions use a 30 second ClientTimeout so a GitHub that accepts the connection but never answers becomes a fast 503 rather than a five minute hang followed by a 500. The token endpoint URL is a keyword argument so tests can point the helper at a local aiohttp server. One test sends a header larger than 8190 bytes and reproduces the exact error from Sentry.
Mariatta
force-pushed
the
handle-oauth-exchange-errors
branch
from
October 25, 2026 01:04
dbdb3b0 to
b2a0ba5
Compare
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
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.
Problem
Sentry reported a 500 on
GET /add_blurb?code=..., the OAuth callback leg of sign-in:The handler posted the code to GitHub's token endpoint and assumed the exchange worked. GitHub instead answered with an HTML page (an outage or interstitial page: its
child-srcCSP is the one GitHub puts on full web pages, while the token endpoint's own CSP is about 170 bytes). That page'sContent-Security-Policyheader is larger than aiohttp's default 8190 byte header limit, so aiohttp raised before the body was read, and the error middleware surfaced it as a 500.The same path also mishandled a rejected code, for example when a refresh reuses one: the body carried
error=bad_verification_code, the token parsed asNone, and the follow-up/usercall failed.Fix
The exchange now lives in
exchange_code_for_token(), which asks GitHub for JSON and sorts failures into two cases:/userlookup and the App installation check get the same page, and so does the already-signed-in branch of the same route, which checks the App installation on every visit and previously had no guard or timeout at all.errorcode from the token endpoint, such asincorrect_client_credentialsorredirect_uri_mismatch. Only a deploy can fix those, so the handler leaves them to surface as a 500 that Sentry reports instead of telling users to retry.Both branches share one
require_app_installed()helper. Their sessions now use a 30 secondClientTimeout, so a GitHub that accepts the connection but never answers becomes a fast 503 instead of a five minute hang that ends in a 500.The messages render through the existing error template, which already shows
error_message.The old
get_access_token()form-string parser is gone since the exchange uses JSON now.Tests
The token endpoint URL is a keyword argument, so the helper tests run against a local aiohttp server instead of patching. One of them sends a CSP header over 8190 bytes and reproduces the exact error from Sentry before it is mapped to the 503. Other cases: a plain HTML page, 429, 503, invalid JSON, a timeout, connection refused, three rejected-code bodies, and the handler's 503, 400, success, and app-not-installed paths, plus outage and rejected-token failures from the
/userand installation lookups, and the signed-in branch hitting a 5xx, a connection error, or a timeout.