🐛 fix(desktop): add token refresh retry mechanism (#10575)

* 🐛 fix(desktop): add token refresh retry mechanism - Add `async-retry` library for exponential backoff retry - Implement retry logic in RemoteServerConfigCtr.refreshAccessToken() - Retries up to 3 times with exponential backoff (1s, 2s, 4s) - Distinguishes between retryable (network) and non-retryable (invalid_grant) errors - Update AuthCtr to only clear tokens for non-retryable errors - Transient errors now preserve tokens for retry on next cycle - Add isNonRetryableError() helper method This fixes the issue where temporary network problems would cause users to be logged out and require re-authorization. Closes LOBE-1368 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * update * 🐛 fix: treat deterministic failures as non-retryable errors Add deterministic failures to non-retryable error list: - 'No refresh token available' - refresh token missing from storage - 'Remote server is not active or configured' - config invalid/disabled - 'Missing tokens in refresh response' - server returned incomplete response These permanent failures now trigger immediate token clearing and authorizationRequired broadcast instead of infinite retry loop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * 📝 docs: clarify issue status workflow - use "In Review" after PR creation - Change workflow to set status to "In Review" when PR is created - "Done" status should only be set after PR is merged - Add note about Linear-GitHub integration for auto status update 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * 🐛 fix: add grace period for consumed RefreshToken When rotateRefreshToken is enabled, the old refresh token is consumed when a new one is issued. If the client fails to receive/save the new token (network issues, crashes), the login state is lost. This adds a 3-minute grace period allowing consumed refresh tokens to be reused, giving clients a chance to retry the refresh operation. Changes: - Add REFRESH_TOKEN_GRACE_PERIOD_SECONDS constant (180s) - Modify find() to allow RefreshToken reuse within grace period - Add unit tests for grace period behavior Closes LOBE-1369 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * 📝 style: translate adapter test descriptions to English 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
2026-03-27 13:29:15 +07:00 · 2025-12-03 15:46:14 +08:00
parent 95bc5c2e6c
commit 83fc2e8bc6
7 changed files with 347 additions and 110 deletions
--- a/apps/desktop/package.json
+++ b/apps/desktop/package.json
@@ -45,11 +45,13 @@
    "@lobechat/electron-server-ipc": "workspace:*",
    "@lobechat/file-loaders": "workspace:*",
    "@lobehub/i18n-cli": "^1.25.1",
+    "@types/async-retry": "^1.4.9",
    "@types/lodash": "^4.17.21",
    "@types/resolve": "^1.20.6",
    "@types/semver": "^7.7.1",
    "@types/set-cookie-parser": "^2.4.10",
    "@typescript/native-preview": "7.0.0-dev.20250711.1",
+    "async-retry": "^1.3.3",
    "consola": "^3.4.2",
    "cookie": "^1.1.1",
    "diff": "^8.0.2",
--- a/apps/desktop/src/main/controllers/AuthCtr.ts
+++ b/apps/desktop/src/main/controllers/AuthCtr.ts
@@ -246,12 +246,23 @@ export default class AuthCtr extends ControllerModule {
            logger.info('Auto-refresh successful');
            this.broadcastTokenRefreshed();
          } else {
-            logger.error(`Auto-refresh failed: ${result.error}`);
-            // If auto-refresh fails, stop timer and clear token
-            this.stopAutoRefresh();
-            await this.remoteServerConfigCtr.clearTokens();
-            await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
-            this.broadcastAuthorizationRequired();
+            logger.error(`Auto-refresh failed after retries: ${result.error}`);
+
+            // Only clear tokens for non-retryable errors (e.g., invalid_grant)
+            // The retry mechanism in RemoteServerConfigCtr already handles transient errors
+            if (this.remoteServerConfigCtr.isNonRetryableError(result.error)) {
+              logger.warn(
+                'Non-retryable error detected, clearing tokens and requiring re-authorization',
+              );
+              this.stopAutoRefresh();
+              await this.remoteServerConfigCtr.clearTokens();
+              await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
+              this.broadcastAuthorizationRequired();
+            } else {
+              // For other errors (after retries exhausted), log but don't clear tokens immediately
+              // The next refresh cycle will retry
+              logger.warn('Refresh failed but error may be transient, will retry on next cycle');
+            }
          }
        }
      } catch (error) {
@@ -335,11 +346,12 @@ export default class AuthCtr extends ControllerModule {

  /**
   * Refresh access token
+   * This method includes retry mechanism via RemoteServerConfigCtr.refreshAccessToken()
   */
  async refreshAccessToken() {
    logger.info('Starting to refresh access token');
    try {
-      // Call the centralized refresh logic in RemoteServerConfigCtr
+      // Call the centralized refresh logic in RemoteServerConfigCtr (includes retry)
      const result = await this.remoteServerConfigCtr.refreshAccessToken();

      if (result.success) {
@@ -350,25 +362,38 @@ export default class AuthCtr extends ControllerModule {
        this.startAutoRefresh();
        return { success: true };
      } else {
-        // Throw an error to be caught by the catch block below
-        // This maintains the existing behavior of clearing tokens on failure
        logger.error(`Token refresh failed via AuthCtr call: ${result.error}`);
-        throw new Error(result.error || 'Token refresh failed');
+
+        // Only clear tokens for non-retryable errors (e.g., invalid_grant)
+        if (this.remoteServerConfigCtr.isNonRetryableError(result.error)) {
+          logger.warn(
+            'Non-retryable error detected, clearing tokens and requiring re-authorization',
+          );
+          this.stopAutoRefresh();
+          await this.remoteServerConfigCtr.clearTokens();
+          await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
+          this.broadcastAuthorizationRequired();
+        } else {
+          // For transient errors, don't clear tokens - allow manual retry
+          logger.warn('Refresh failed but error may be transient, tokens preserved for retry');
+        }
+
+        return { error: result.error, success: false };
      }
    } catch (error) {
-      // Keep the existing logic to clear tokens and require re-auth on failure
-      logger.error('Token refresh operation failed via AuthCtr, initiating cleanup:', error);
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      logger.error('Token refresh operation failed via AuthCtr:', errorMessage);

-      // Refresh failed, clear tokens and disable remote server
-      logger.warn('Refresh failed, clearing tokens and disabling remote server');
-      this.stopAutoRefresh();
-      await this.remoteServerConfigCtr.clearTokens();
-      await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
+      // Only clear tokens for non-retryable errors
+      if (this.remoteServerConfigCtr.isNonRetryableError(errorMessage)) {
+        logger.warn('Non-retryable error in catch block, clearing tokens');
+        this.stopAutoRefresh();
+        await this.remoteServerConfigCtr.clearTokens();
+        await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
+        this.broadcastAuthorizationRequired();
+      }

-      // Notify render process that re-authorization is required
-      this.broadcastAuthorizationRequired();
-
-      return { error: error.message, success: false };
+      return { error: errorMessage, success: false };
    }
  }

@@ -601,7 +626,7 @@ export default class AuthCtr extends ControllerModule {
      if (currentTime >= expiresAt) {
        logger.info('Token has expired, attempting to refresh it');

-        // Attempt to refresh token
+        // Attempt to refresh token (includes retry mechanism)
        const refreshResult = await this.remoteServerConfigCtr.refreshAccessToken();
        if (refreshResult.success) {
          logger.info('Token refresh successful during initialization');
@@ -611,10 +636,18 @@ export default class AuthCtr extends ControllerModule {
          return;
        } else {
          logger.error(`Token refresh failed during initialization: ${refreshResult.error}`);
-          // Clear token and require re-authorization only on refresh failure
-          await this.remoteServerConfigCtr.clearTokens();
-          await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
-          this.broadcastAuthorizationRequired();
+
+          // Only clear token for non-retryable errors
+          if (this.remoteServerConfigCtr.isNonRetryableError(refreshResult.error)) {
+            logger.warn('Non-retryable error during initialization, clearing tokens');
+            await this.remoteServerConfigCtr.clearTokens();
+            await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
+            this.broadcastAuthorizationRequired();
+          } else {
+            // For transient errors, still start auto-refresh timer to retry later
+            logger.warn('Transient error during initialization, will retry via auto-refresh');
+            this.startAutoRefresh();
+          }
          return;
        }
      }
--- a/apps/desktop/src/main/controllers/RemoteServerConfigCtr.ts
+++ b/apps/desktop/src/main/controllers/RemoteServerConfigCtr.ts
@@ -1,4 +1,5 @@
 import { DataSyncConfig } from '@lobechat/electron-client-ipc';
+import retry from 'async-retry';
 import { safeStorage } from 'electron';
 import querystring from 'node:querystring';
 import { URL } from 'node:url';
@@ -8,6 +9,28 @@ import { createLogger } from '@/utils/logger';

 import { ControllerModule, ipcClientEvent } from './index';

+/**
+ * Non-retryable OIDC error codes
+ * These errors indicate the refresh token is invalid and retry won't help
+ */
+const NON_RETRYABLE_OIDC_ERRORS = [
+  'invalid_grant', // refresh token is invalid, expired, or revoked
+  'invalid_client', // client configuration error
+  'unauthorized_client', // client not authorized
+  'access_denied', // user denied access
+  'invalid_scope', // requested scope is invalid
+];
+
+/**
+ * Deterministic failures that will never succeed on retry
+ * These are permanent state issues that require user intervention
+ */
+const DETERMINISTIC_FAILURES = [
+  'no refresh token available', // refresh token is missing from storage
+  'remote server is not active or configured', // config is invalid or disabled
+  'missing tokens in refresh response', // server returned incomplete response
+];
+
 // Create logger
 const logger = createLogger('controllers:RemoteServerConfigCtr');

@@ -246,9 +269,34 @@ export default class RemoteServerConfigCtr extends ControllerModule {
  }

  /**
-   * Refresh access token
+   * Check if an error is non-retryable
+   * Includes OIDC errors (e.g., invalid_grant) and deterministic failures
+   * (e.g., missing refresh token, invalid config)
+   * @param error Error message to check
+   * @returns true if the error should not be retried
+   */
+  isNonRetryableError(error?: string): boolean {
+    if (!error) return false;
+    const lowerError = error.toLowerCase();
+
+    // Check OIDC error codes
+    if (NON_RETRYABLE_OIDC_ERRORS.some((code) => lowerError.includes(code))) {
+      return true;
+    }
+
+    // Check deterministic failures that require user intervention
+    if (DETERMINISTIC_FAILURES.some((msg) => lowerError.includes(msg))) {
+      return true;
+    }
+
+    return false;
+  }
+
+  /**
+   * Refresh access token with retry mechanism
   * Use stored refresh token to obtain a new access token
   * Handles concurrent requests by returning the existing refresh promise if one is in progress.
+   * Retries up to 3 times with exponential backoff for transient errors.
   */
  async refreshAccessToken(): Promise<{ error?: string; success: boolean }> {
    // If a refresh is already in progress, return the existing promise
@@ -257,14 +305,62 @@ export default class RemoteServerConfigCtr extends ControllerModule {
      return this.refreshPromise;
    }

-    // Start a new refresh operation
-    logger.info('Initiating new token refresh operation.');
-    this.refreshPromise = this.performTokenRefresh();
+    // Start a new refresh operation with retry
+    logger.info('Initiating new token refresh operation with retry.');
+    this.refreshPromise = this.performTokenRefreshWithRetry();

    // Return the promise so callers can wait
    return this.refreshPromise;
  }

+  /**
+   * Performs token refresh with retry mechanism
+   * Uses exponential backoff: 1s, 2s, 4s
+   */
+  private async performTokenRefreshWithRetry(): Promise<{ error?: string; success: boolean }> {
+    try {
+      return await retry(
+        async (bail, attemptNumber) => {
+          logger.debug(`Token refresh attempt ${attemptNumber}/3`);
+
+          const result = await this.performTokenRefresh();
+
+          if (result.success) {
+            return result;
+          }
+
+          // Check if error is non-retryable
+          if (this.isNonRetryableError(result.error)) {
+            logger.warn(`Non-retryable error encountered: ${result.error}`);
+            // Use bail to stop retrying immediately
+            bail(new Error(result.error));
+            return result; // This won't be reached, but TypeScript needs it
+          }
+
+          // Throw error to trigger retry for transient errors
+          throw new Error(result.error);
+        },
+        {
+          factor: 2, // Exponential backoff factor
+          maxTimeout: 4000, // Max wait time between retries: 4s
+          minTimeout: 1000, // Min wait time between retries: 1s
+          onRetry: (err: Error, attempt: number) => {
+            logger.info(`Token refresh retry ${attempt}/3: ${err.message}`);
+          },
+          retries: 3, // Total retry attempts
+        },
+      );
+    } catch (error) {
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      logger.error('Token refresh failed after all retries:', errorMessage);
+      return { error: errorMessage, success: false };
+    } finally {
+      // Ensure the promise reference is cleared once the operation completes
+      logger.debug('Clearing the refresh promise reference.');
+      this.refreshPromise = null;
+    }
+  }
+
  /**
   * Performs the actual token refresh logic.
   * This method is called by refreshAccessToken and wrapped in a promise.
@@ -337,10 +433,6 @@ export default class RemoteServerConfigCtr extends ControllerModule {
      const errorMessage = error instanceof Error ? error.message : String(error);
      logger.error('Exception during token refresh operation:', errorMessage, error);
      return { error: `Exception occurred during token refresh: ${errorMessage}`, success: false };
-    } finally {
-      // Ensure the promise reference is cleared once the operation completes
-      logger.debug('Clearing the refresh promise reference.');
-      this.refreshPromise = null;
    }
  }

--- a/apps/desktop/src/main/controllers/tests/RemoteServerConfigCtr.test.ts
+++ b/apps/desktop/src/main/controllers/tests/RemoteServerConfigCtr.test.ts
@@ -355,6 +355,41 @@ describe('RemoteServerConfigCtr', () => {
    });
  });

+  describe('isNonRetryableError', () => {
+    it('should return false for null/undefined error', () => {
+      expect(controller.isNonRetryableError(undefined)).toBe(false);
+      expect(controller.isNonRetryableError('')).toBe(false);
+    });
+
+    it('should return true for OIDC error codes', () => {
+      expect(controller.isNonRetryableError('invalid_grant')).toBe(true);
+      expect(controller.isNonRetryableError('Token refresh failed: invalid_client')).toBe(true);
+      expect(controller.isNonRetryableError('unauthorized_client error')).toBe(true);
+      expect(controller.isNonRetryableError('access_denied by user')).toBe(true);
+      expect(controller.isNonRetryableError('invalid_scope requested')).toBe(true);
+    });
+
+    it('should return true for deterministic failures', () => {
+      expect(controller.isNonRetryableError('No refresh token available')).toBe(true);
+      expect(controller.isNonRetryableError('Remote server is not active or configured')).toBe(
+        true,
+      );
+      expect(controller.isNonRetryableError('Missing tokens in refresh response')).toBe(true);
+    });
+
+    it('should return false for transient/network errors', () => {
+      expect(controller.isNonRetryableError('Network error')).toBe(false);
+      expect(controller.isNonRetryableError('fetch failed')).toBe(false);
+      expect(controller.isNonRetryableError('ETIMEDOUT')).toBe(false);
+      expect(controller.isNonRetryableError('Connection refused')).toBe(false);
+    });
+
+    it('should be case insensitive', () => {
+      expect(controller.isNonRetryableError('INVALID_GRANT')).toBe(true);
+      expect(controller.isNonRetryableError('NO REFRESH TOKEN AVAILABLE')).toBe(true);
+    });
+  });
+
  describe('refreshAccessToken', () => {
    let mockFetch: ReturnType<typeof vi.fn>;

@@ -556,7 +591,7 @@ describe('RemoteServerConfigCtr', () => {
      expect(mockFetch).toHaveBeenCalledTimes(1);
    });

-    it('should handle network errors', async () => {
+    it('should handle network errors with retry', async () => {
      const { safeStorage } = await import('electron');
      vi.mocked(safeStorage.isEncryptionAvailable).mockReturnValue(true);
      vi.mocked(safeStorage.decryptString).mockImplementation((buffer: Buffer) =>
@@ -582,7 +617,9 @@ describe('RemoteServerConfigCtr', () => {

      expect(result.success).toBe(false);
      expect(result.error).toContain('Network error');
-    });
+      // With retry mechanism, fetch should be called 4 times (1 initial + 3 retries)
+      expect(mockFetch).toHaveBeenCalledTimes(4);
+    }, 15000);
  });

  describe('afterAppReady', () => {