fix: prevent auth token refresh failure from deadlocking the client - #603
Open
glasstiger wants to merge 2 commits into
Open
fix: prevent auth token refresh failure from deadlocking the client#603glasstiger wants to merge 2 commits into
glasstiger wants to merge 2 commits into
Conversation
When an OIDC token refresh failed at the transport level (token endpoint unreachable, or a non-JSON response body), refreshTokenMethod() rejected inside the setInterval callback of refreshAuthToken(), before the static refreshTokenPending flag was reset. The flag was left stuck true, the outer promise never settled, and every subsequent query deadlocked waiting on it — the console silently froze with no 401 and no logout. Wrap the refresh in try/catch/finally so refreshTokenPending is always reset. On failure the stale token is kept in place and the next request receives a 401, which drives the existing re-auth flow — matching the behaviour when there is no refresh token at all. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
When OIDC is used,
Client.refreshAuthToken()proactively refreshes the token ~30s beforeexpires_inelapses. The token fetch was performed inside anasyncsetIntervalcallback:If
refreshTokenMethod()rejected — a transport-level failure such as the token endpoint being unreachable, a CORS/DNS error, or a non-JSON response body (e.g. a reverse-proxy502HTML page) — theawaitthrew before the flag was reset:await this.refreshAuthToken()hung forever (it never even rejected).refreshTokenPendingflag was left stucktrue.while (Client.refreshTokenPending)wait loop.Net effect: the Web Console silently froze — no query ran, no
401was triggered, no logout, no error surfaced. Only a manual page reload recovered it.This only affected the transport-failure case. A well-formed OAuth error response (e.g.
invalid_grantfor an expired refresh token) already degraded gracefully to the login screen, as does the no-refresh-token case (401→ logout).Fix
Restructure
refreshAuthToken()so the failure can't escape without cleanup:setIntervalnow only waits for in-flight queries to drain, thenresolve()s — a plain synchronous callback, no swallowed errors.refreshTokenMethod()call moves into the mainasyncbody, wrapped intry/catch/finally.finallyalways resetsrefreshTokenPending— it can never get stuck again.catchlogs and swallows the error, keeping the stale token in place. The next request then receives a401, which drives the existingMSG_CONNECTION_UNAUTHORIZED→ logout flow — consistent with the no-refresh-token path.The concurrency invariant is preserved: while
refreshTokenPendingistrue, no new query passes the gate to incrementnumOfPendingQueries, so "0 pending queries when we refresh" still holds.Testing
client.test.ts: with an in-window SSO token and arefreshTokenMethodthat rejects, the query still resolves,console.errorfires, and a second query also goes through (proving the flag was reset). Against the old code this test hangs to timeout; against the fix it passes in ~100ms.yarn test:unit src/utils/questdb/client.test.ts— 3/3 passtsc --noEmit— no new errorseslinton both changed files — clean🤖 Generated with Claude Code