fix(FN-673): remove beta header and add retry logic for Claude usage

- Remove outdated beta header from dashboard Header component - Add retry logic with exponential backoff for Claude API rate limits - Differentiate between rate limit errors and other Claude usage errors - Add comprehensive tests for retry logic and error handling - Add changeset for the fix
2026-04-01 07:10:57 -07:00
parent 800146550e
commit 70d047fe27
3 changed files with 450 additions and 19 deletions
--- a/packages/dashboard/src/usage.test.ts
+++ b/packages/dashboard/src/usage.test.ts
@@ -4,6 +4,8 @@ import {
  clearUsageCache,
  ProviderUsage,
  calculatePace,
+  _setSleepFn,
+  _resetSleepFn,
 } from "./usage.js";

 // Mock the https module
@@ -364,6 +366,378 @@ describe("usage", () => {
      expect(claude.status).toBe("error");
      expect(claude.error).toContain("Auth expired");
    });
+
+    it("does not send anthropic-beta header in requests", async () => {
+      const mockResponse = {
+        five_hour: { utilization: 10.0 },
+      };
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "test-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      let capturedHeaders: Record<string, string> = {};
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        capturedHeaders = options.headers || {};
+        const mockRes = {
+          statusCode: 200,
+          headers: {},
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              handler(Buffer.from(JSON.stringify(mockResponse)));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      await fetchAllProviderUsage();
+
+      // Verify no anthropic-beta header is sent
+      expect(capturedHeaders).not.toHaveProperty("anthropic-beta");
+    });
+
+    it("retries on 429 and succeeds after transient rate limit", async () => {
+      const noopSleep = vi.fn().mockResolvedValue(undefined);
+      _setSleepFn(noopSleep);
+
+      const mockResponse = {
+        five_hour: {
+          utilization: 20.0,
+          resets_at: new Date(Date.now() + 3 * 60 * 60 * 1000).toISOString(),
+        },
+      };
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "test-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      let callCount = 0;
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        callCount++;
+        const is429 = callCount <= 2; // First 2 calls return 429, third succeeds
+        const mockRes = {
+          statusCode: is429 ? 429 : 200,
+          headers: is429 ? { "retry-after": "1" } : {},
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              const body = is429
+                ? '{"error":"rate_limited"}'
+                : JSON.stringify(mockResponse);
+              handler(Buffer.from(body));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      const providers = await fetchAllProviderUsage();
+      const claude = providers.find((p) => p.name === "Claude")!;
+
+      expect(claude.status).toBe("ok");
+      expect(claude.windows).toHaveLength(1);
+      expect(claude.windows[0].percentUsed).toBe(20);
+
+      // Verify sleep was called for retries (2 retry sleeps)
+      expect(noopSleep).toHaveBeenCalledTimes(2);
+
+      _resetSleepFn();
+    });
+
+    it("reports rate limited after all retries exhausted on 429", async () => {
+      const noopSleep = vi.fn().mockResolvedValue(undefined);
+      _setSleepFn(noopSleep);
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "test-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      // Always return 429
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        const mockRes = {
+          statusCode: 429,
+          headers: {},
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              handler(Buffer.from('{"error":"rate_limited"}'));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      const providers = await fetchAllProviderUsage();
+      const claude = providers.find((p) => p.name === "Claude")!;
+
+      expect(claude.status).toBe("error");
+      expect(claude.error).toBe("Rate limited — try again later");
+
+      // Verify retries happened (2 sleeps for 3 attempts)
+      expect(noopSleep).toHaveBeenCalledTimes(2);
+
+      _resetSleepFn();
+    });
+
+    it("uses exponential backoff delays when retry-after header is absent", async () => {
+      const noopSleep = vi.fn().mockResolvedValue(undefined);
+      _setSleepFn(noopSleep);
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "test-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      // Always return 429 without retry-after
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        const mockRes = {
+          statusCode: 429,
+          headers: {}, // No retry-after header
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              handler(Buffer.from('{"error":"rate_limited"}'));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      await fetchAllProviderUsage();
+
+      // Exponential backoff: 1000ms * 2^0 = 1000, 1000ms * 2^1 = 2000
+      expect(noopSleep).toHaveBeenCalledTimes(2);
+      expect(noopSleep).toHaveBeenNthCalledWith(1, 1000);
+      expect(noopSleep).toHaveBeenNthCalledWith(2, 2000);
+
+      _resetSleepFn();
+    });
+
+    it("respects retry-after header value for delay", async () => {
+      const noopSleep = vi.fn().mockResolvedValue(undefined);
+      _setSleepFn(noopSleep);
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "test-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      // 429 with retry-after: 5 seconds
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        const mockRes = {
+          statusCode: 429,
+          headers: { "retry-after": "5" },
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              handler(Buffer.from('{"error":"rate_limited"}'));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      await fetchAllProviderUsage();
+
+      // Should use retry-after value (5s = 5000ms) for both retries
+      expect(noopSleep).toHaveBeenCalledTimes(2);
+      expect(noopSleep).toHaveBeenNthCalledWith(1, 5000);
+      expect(noopSleep).toHaveBeenNthCalledWith(2, 5000);
+
+      _resetSleepFn();
+    });
+
+    it("does not retry on 401 auth errors", async () => {
+      const noopSleep = vi.fn().mockResolvedValue(undefined);
+      _setSleepFn(noopSleep);
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "expired-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        const mockRes = {
+          statusCode: 401,
+          headers: {},
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              handler(Buffer.from('{"error": "unauthorized"}'));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      const providers = await fetchAllProviderUsage();
+      const claude = providers.find((p) => p.name === "Claude")!;
+
+      expect(claude.status).toBe("error");
+      expect(claude.error).toContain("Auth expired");
+      // No retries should happen for auth errors
+      expect(noopSleep).not.toHaveBeenCalled();
+
+      _resetSleepFn();
+    });
+
+    it("does not retry on 403 auth errors", async () => {
+      const noopSleep = vi.fn().mockResolvedValue(undefined);
+      _setSleepFn(noopSleep);
+
+      mockReadFileSync.mockImplementation((path: string) => {
+        if (path.includes("claude")) {
+          return JSON.stringify({
+            accessToken: "forbidden-token",
+            scopes: ["user:profile"],
+          });
+        }
+        throw new Error("File not found");
+      });
+      mockExecFileSync.mockImplementation(() => {
+        throw new Error("Keychain item not found");
+      });
+
+      const mockReq = {
+        on: vi.fn(),
+        write: vi.fn(),
+        end: vi.fn(),
+      };
+
+      mockRequest.mockImplementation((options: any, callback: any) => {
+        const mockRes = {
+          statusCode: 403,
+          headers: {},
+          on: vi.fn((event: string, handler: any) => {
+            if (event === "data") {
+              handler(Buffer.from('{"error": "forbidden"}'));
+            }
+            if (event === "end") {
+              handler();
+            }
+          }),
+        };
+        callback(mockRes);
+        return mockReq;
+      });
+
+      const providers = await fetchAllProviderUsage();
+      const claude = providers.find((p) => p.name === "Claude")!;
+
+      expect(claude.status).toBe("error");
+      expect(claude.error).toContain("Auth expired");
+      // No retries should happen for auth errors
+      expect(noopSleep).not.toHaveBeenCalled();
+
+      _resetSleepFn();
+    });
  });

  describe("Codex provider", () => {
--- a/packages/dashboard/src/usage.ts
+++ b/packages/dashboard/src/usage.ts
@@ -228,6 +228,26 @@ function readClaudeKeychainCredentials(): any | null {
  }
 }

+/** Max number of retries for transient 429 responses */
+const CLAUDE_MAX_RETRIES = 3;
+/** Initial retry delay in ms (doubles each attempt) */
+const CLAUDE_INITIAL_RETRY_MS = 1000;
+
+/**
+ * Sleep for the given duration. Exported for test mocking.
+ */
+export const _sleep = (ms: number): Promise<void> =>
+  new Promise((resolve) => setTimeout(resolve, ms));
+
+// Allow tests to swap the sleep implementation
+let sleepFn = _sleep;
+export function _setSleepFn(fn: typeof _sleep): void {
+  sleepFn = fn;
+}
+export function _resetSleepFn(): void {
+  sleepFn = _sleep;
+}
+
 async function fetchClaudeUsage(): Promise<ProviderUsage> {
  const usage: ProviderUsage = {
    name: "Claude",
@@ -280,29 +300,61 @@ async function fetchClaudeUsage(): Promise<ProviderUsage> {
  }

  try {
-    const res = await httpsRequest("https://api.anthropic.com/api/oauth/usage", {
-      method: "GET",
-      headers: {
-        authorization: `Bearer ${oauthCreds.accessToken}`,
-        "anthropic-beta": "oauth-2025-04-20",
-      },
-    });
+    // Retry loop for transient 429 responses
+    let res: { status: number; headers: Record<string, string>; body: string } | undefined;
+    let lastStatus = 0;

-    if (res.status === 401 || res.status === 403) {
-      usage.status = "error";
-      usage.error = "Auth expired — run 'claude' to re-login";
-      return usage;
+    for (let attempt = 0; attempt < CLAUDE_MAX_RETRIES; attempt++) {
+      res = await httpsRequest("https://api.anthropic.com/api/oauth/usage", {
+        method: "GET",
+        headers: {
+          authorization: `Bearer ${oauthCreds.accessToken}`,
+        },
+      });
+
+      lastStatus = res.status;
+
+      // Auth errors are not transient — fail immediately
+      if (res.status === 401 || res.status === 403) {
+        usage.status = "error";
+        usage.error = "Auth expired — run 'claude' to re-login";
+        return usage;
+      }
+
+      // 429 is potentially transient — retry with exponential backoff
+      if (res.status === 429) {
+        if (attempt < CLAUDE_MAX_RETRIES - 1) {
+          // Use retry-after header if available, otherwise exponential backoff
+          const retryAfter = res.headers["retry-after"];
+          let delayMs: number;
+          if (retryAfter && !isNaN(Number(retryAfter))) {
+            delayMs = Number(retryAfter) * 1000;
+          } else {
+            delayMs = CLAUDE_INITIAL_RETRY_MS * Math.pow(2, attempt);
+          }
+          await sleepFn(delayMs);
+          continue;
+        }
+        // All retries exhausted
+        usage.status = "error";
+        usage.error = "Rate limited — try again later";
+        return usage;
+      }
+
+      // Any other non-200 status — fail immediately (not transient)
+      if (res.status !== 200) {
+        usage.status = "error";
+        usage.error = `HTTP ${res.status}`;
+        return usage;
+      }
+
+      // Success — break out of retry loop
+      break;
    }

-    if (res.status === 429) {
+    if (!res || lastStatus !== 200) {
      usage.status = "error";
-      usage.error = "Rate limited — try again later";
-      return usage;
-    }
-
-    if (res.status !== 200) {
-      usage.status = "error";
-      usage.error = `HTTP ${res.status}`;
+      usage.error = `HTTP ${lastStatus}`;
      return usage;
    }