chore(ui): ensure token updated before making failed req#27140
chore(ui): ensure token updated before making failed req#27140chirag-madlani merged 1 commit intomainfrom
Conversation
github-actions
bot
added
safe to test
Code Review ✅ ApprovedEnsures the authentication token is refreshed before retrying failed requests in the UI, preventing unnecessary 401 errors. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
Pull request overview
This PR aims to harden the UI authentication flow by improving token refresh reliability and the retry of failed (401) requests, primarily by ensuring Axios interceptors are initialized before replaying queued requests and by adding logic intended to wait for refreshed token persistence.
Changes:
- Await
initializeAxiosInterceptors()before retrying queued Axios requests after a token refresh. - Rename the pre-refresh token variable to
oldTokenfor clarity inTokenService. - Add a
waitForTokenPersistencehelper (currently unused) intended to wait until the refreshed token is actually persisted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts | Adds a token-persistence wait helper and refines token naming in refresh logic. |
| openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx | Ensures interceptor initialization is awaited before retrying queued requests after refresh. |
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx:524
- On the success path the queue is retried and then cleared, but in the failure paths (token falsy or
refreshToken()rejects) the queued promises are not rejected andpendingRequestsis not cleared. That can leave the original Axios call hanging and can leak queued configs. Consider rejecting all queued requests and clearingpendingRequestswhen refresh fails / logout is triggered.
await initializeAxiosInterceptors();
pendingRequests.forEach(({ resolve, reject, config }) => {
axiosClient.request(config).then(resolve).catch(reject);
});
| private async waitForTokenPersistence(oldToken: string) { | ||
| const maxAttempts = 20; | ||
| const delayMs = 50; | ||
|
|
||
| for (let attempt = 0; attempt < maxAttempts; attempt++) { | ||
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | ||
|
|
||
| const currentToken = await getOidcToken(); | ||
|
|
||
| if (currentToken && currentToken !== oldToken) { | ||
| return; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
waitForTokenPersistence is introduced but never called. As a result, the refresh flow still relies on the existing fixed delay and does not actually wait for the refreshed token to be persisted (as described in the PR). Either wire this into refreshToken() (e.g., using the captured oldToken) or remove the unused method. Also consider returning a boolean / throwing on timeout so callers can handle the persistence not completing instead of silently continuing after max attempts.
| .then(async (token) => { | ||
| if (token) { | ||
| // Retry the pending requests | ||
| initializeAxiosInterceptors(); | ||
| await initializeAxiosInterceptors(); | ||
| pendingRequests.forEach(({ resolve, reject, config }) => { |
There was a problem hiding this comment.
The retry path treats a falsy refreshToken() result as a refresh failure (resetUserDetails(true)), but some authenticators intentionally return void for silent refresh (e.g., OidcAuthenticator’s renewIdToken resolves with no token and persists it via the silent-callback). In those cases this branch will force-logout and will never retry queued requests even though the token may have been refreshed. Consider basing the decision to retry on the persisted token (e.g., re-read getOidcToken() / compare against the previous token) or adjust TokenService.refreshToken() to resolve with the persisted token once it’s available.
Jest test CoverageUI tests summary
|
|
🟡 Playwright Results — all passed (24 flaky)✅ 3594 passed · ❌ 0 failed · 🟡 24 flaky · ⏭️ 207 skipped
🟡 24 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
(cherry picked from commit 5cdf7fa)
(cherry picked from commit 5cdf7fa)
This pull request improves the reliability of token refresh and request retry logic in the authentication flow. The main focus is to ensure that Axios interceptors are fully initialized before retrying requests and that token persistence is properly handled after a refresh.
Authentication and Token Refresh Improvements:
initializeAxiosInterceptorscall asynchronous and awaited its completion before retrying pending requests inAuthProvider.tsx, ensuring interceptors are ready before requests are retried.waitForTokenPersistenceinTokenServiceUtil.tsto wait until the refreshed token is actually persisted, improving reliability in token handling.oldTokeninstead oftoken) inTokenServiceUtil.ts.Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>