Repository navigation
Conversation
JWTRefreshMiddleware runs outside the FastAPI exception handlers and only caught HTTPException / AuthManagerRefreshTokenExpiredException. Any other exception raised while resolving or refreshing the user - notably an OperationalError when the metadata DB connection is recycled underneath the API server - propagated to the ASGI layer and surfaced as a bare HTTP 500, even for requests that did not need the refresh at all. Treat such errors as "could not refresh this time": log a warning, proceed without injecting the user (the route's own auth dependency revalidates the token), and leave the cookie untouched so a later request refreshes once the backend recovers. HTTPException / token-expiry semantics are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Failing open on an unexpected error keeps the request alive, but it must not make a genuine bug in the auth manager quieter than the bare 500 it replaces. WARNING is easy to miss and a monitor keyed on 5xx would go silent on a real regression, so switch the unexpected arm to log.exception() - ERROR level with the traceback - while transient blips stay non-fatal (review feedback from @SameerMesiah97). Signed-off-by: kjh0623 <8412070+kjh0623@users.noreply.github.com>
Signed-off-by: kjh0623 <8412070+kjh0623@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ashb
approved these changes
Oct 14, 2026
1 task done
1 task done
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.
Re-submission of #70057, which was auto-closed as stale (bot-closed, so I can't reopen it). Same branch, with current
mainmerged in. Earlier review discussion is in #70057.Problem
JWTRefreshMiddleware.dispatchruns outside the FastAPI exception handlers and only catchesHTTPException/AuthManagerRefreshTokenExpiredException. Any other error from_refresh_user()becomes a bare 500. We hit this in production when PgBouncer drops a connection and the user lookup raisesOperationalError. Related: #57859.Fix
Add a final
except Exceptionarm that:log.exception()(ERROR + traceback),The catch is broad on purpose: the middleware can't know which exceptions are transient for the configured auth manager.
Testing
New unit test: a non-HTTP error from
_refresh_user→ the request succeeds, noset-cookie, an ERROR record withexc_infois logged.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code following the guidelines