fix(auth): keep a slow Auth exchange from freezing every other Lens request - #28
Merged
Merged
Conversation
…equest
TLDR
Lens's sign-in callback and its back-channel logout receiver talk to the Auth
server with a blocking HTTP client, but they were declared as async handlers,
so the whole web process waited on that one network call. A slow or
unreachable Auth could stall the dashboard, health checks and every other
user for up to ten seconds per sign-in attempt. The two handlers now run in
FastAPI's worker threads, like Explorer's already do.
Problem
auth_callback and auth_backchannel_logout were `async def`, yet
complete_login calls CentralAuthClient.exchange (urllib, 10-second timeout)
and the logout receiver calls AuthKeyResolver.keys_for_token (urllib JWKS
fetch, 5-second timeout). A coroutine that makes a blocking call holds the
single Uvicorn event loop for the duration, so nothing else is served: not
/health, not the job monitor, not other callbacks. Anyone can trigger the
callback path (start /auth/login, then hit /auth/callback with any code), and
the receiver is public by design, so an Auth outage or slow link turned into
a Lens-wide stall exactly when operators need /health to answer.
Fix
- Declare both handlers as plain `def`. FastAPI runs sync handlers in its
thread pool, so a blocked exchange or JWKS fetch stalls only that request.
No behaviour, status code or response body changes; request.session and
Form parsing work identically for sync handlers.
- A comment on the callback explains why these two are deliberately sync so
a future "make everything async" pass does not undo it.
- The other central-auth routes (/auth/login, /logout, /signed-out) only touch
SQLite for sub-millisecond reads and writes and stay as they are.
Tests
- test_slow_code_exchange_does_not_stall_the_event_loop drives /auth/login,
schedules the callback against a fake exchange that blocks on a threading
event, then requests /health and asserts it answers in under two seconds
before the event is released. On the previous code /health waited the full
5-second fallback ("/health waited 5.0s behind a blocked code exchange");
with the fix it answers immediately and the callback still completes 303.
- ruff check, ruff format --check, and the full suite (334 passed, 1 skipped)
are green.
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.
TLDR
Lens's sign-in callback and its back-channel logout receiver talk to the Auth server with a blocking HTTP client, but they were declared as async handlers, so the whole web process waited on that one network call. A slow or unreachable Auth could stall the dashboard, health checks and every other user for up to ten seconds per sign-in attempt. The two handlers now run in FastAPI's worker threads, like Explorer's already do.
Problem
auth_callbackandauth_backchannel_logoutwereasync def, yetcomplete_logincallsCentralAuthClient.exchange(urllib, 10-second timeout) and the logout receiver callsAuthKeyResolver.keys_for_token(urllib JWKS fetch, 5-second timeout). A coroutine that makes a blocking call holds the single Uvicorn event loop for the duration, so nothing else is served: not/health, not the job monitor, not other callbacks. Anyone can trigger the callback path (start/auth/login, then hit/auth/callbackwith any code), and the receiver is public by design, so an Auth outage or slow link turned into a Lens-wide stall exactly when operators need/healthto answer.Fix
def. FastAPI runs sync handlers in its thread pool, so a blocked exchange or JWKS fetch stalls only that request. No behaviour, status code or response body changes;request.sessionandFormparsing work identically for sync handlers./auth/login,/logout,/signed-out) only touch SQLite for sub-millisecond reads and writes and stay as they are.Tests
test_slow_code_exchange_does_not_stall_the_event_loopdrives/auth/login, schedules the callback against a fake exchange that blocks on a threading event, then requests/healthand asserts it answers in under two seconds before the event is released. On the previous code/healthwaited the full 5-second fallback ("/health waited 5.0s behind a blocked code exchange"); with the fix it answers immediately and the callback still completes 303.ruff check,ruff format --check, and the full suite (334 passed, 1 skipped) are green.