diff --git a/app/lib/discord/api.test.ts b/app/lib/discord/api.test.ts index ff7b39ce..5b9ad5b8 100644 --- a/app/lib/discord/api.test.ts +++ b/app/lib/discord/api.test.ts @@ -5,7 +5,13 @@ import { log } from "@/lib/log"; import type { Username } from "@/lib/session"; import { server } from "@/mocks/node"; -import { getChannelMessages, getMessageChain, postChannelMessage } from "./api"; +import { + _resetRateLimitState, + _setRateLimitGate, + getChannelMessages, + getMessageChain, + postChannelMessage, +} from "./api"; vi.mock(import("server-only"), () => ({})); @@ -708,6 +714,7 @@ describe("rate limiting", () => { afterEach(() => { vi.useRealTimers(); vi.restoreAllMocks(); + _resetRateLimitState(); }); it("should retry after 429 and succeed", async () => { @@ -759,6 +766,7 @@ describe("rate limiting", () => { elapsedMs: 0, retryAfterMs: 35000, global: true, + retries: 1, }, "Discord rate limit exceeded max wait time", ); @@ -786,7 +794,12 @@ describe("rate limiting", () => { expect(attempts).toBe(2); expect(logWarnSpy).toHaveBeenCalledWith( - { endpoint: expect.any(String), retryAfterMs: 1000, global: false }, + { + endpoint: expect.any(String), + retryAfterMs: 1000, + global: false, + retries: 1, + }, "Discord rate limited, retrying", ); }); @@ -823,7 +836,12 @@ describe("rate limiting", () => { expect(attempts).toBe(2); expect(logWarnSpy).toHaveBeenCalledWith( - { endpoint: expect.any(String), retryAfterMs: 15000, global: false }, + { + endpoint: expect.any(String), + retryAfterMs: 15000, + global: false, + retries: 1, + }, "Discord rate limited, retrying", ); expect(logErrorSpy).toHaveBeenCalledWith( @@ -832,8 +850,132 @@ describe("rate limiting", () => { elapsedMs: 15000, retryAfterMs: 16000, global: false, + retries: 2, }, "Discord rate limit exceeded max wait time", ); }); + + it("should throw when max retries exceeded", async () => { + const logWarnSpy = vi.spyOn(log, "warn").mockImplementation(() => {}); + const logErrorSpy = vi.spyOn(log, "error").mockImplementation(() => {}); + let attempts = 0; + + server.use( + http.get(`${DISCORD_BASE_URL}/channels/:channelId/messages`, () => { + attempts++; + return HttpResponse.json( + { message: "Rate limited", retry_after: 1, global: false }, + { status: 429 }, + ); + }), + ); + + const promise = getChannelMessages(); + const assertion = expect(promise).rejects.toThrow( + "Discord rate limit exceeded", + ); + + await vi.advanceTimersByTimeAsync(10000); + await assertion; + + // 1 initial + 5 retries = 6 total attempts + expect(attempts).toBe(6); + expect(logWarnSpy).toHaveBeenCalledTimes(5); + expect(logErrorSpy).toHaveBeenCalledWith( + expect.objectContaining({ retries: 6 }), + "Discord rate limit exceeded max wait time", + ); + }); + + it("should share rate limit state across concurrent requests", async () => { + vi.spyOn(log, "warn").mockImplementation(() => {}); + let fetchCount = 0; + + server.use( + http.get(`${DISCORD_BASE_URL}/channels/:channelId/messages`, () => { + fetchCount++; + if (fetchCount === 1) { + return HttpResponse.json( + { message: "Rate limited", retry_after: 2, global: false }, + { status: 429 }, + ); + } + return HttpResponse.json([]); + }), + ); + + // First request gets 429 and sets the shared gate + const promise1 = getChannelMessages(); + await vi.advanceTimersByTimeAsync(0); + + // Second request arrives while rate limit gate is active + const promise2 = getChannelMessages(); + + // Advance past the 2-second rate limit window + await vi.advanceTimersByTimeAsync(2000); + + await Promise.all([promise1, promise2]); + + // Without shared state: 4 fetches (both hit 429 independently, both retry) + // With shared state: 3 fetches (first gets 429, second waits on gate, both succeed after delay) + expect(fetchCount).toBe(3); + }); + + it("should throw when shared gate wait exceeds timeout", async () => { + const logErrorSpy = vi.spyOn(log, "error").mockImplementation(() => {}); + + // Directly set a gate 31 seconds in the future — exceeds RATE_LIMIT_TIMEOUT_MS (30s) + const endpoint = "channels/test-discord-channel-id/messages"; + _setRateLimitGate(endpoint, Date.now() + 31_000); + + // A fresh request checks the gate: waitMs=31000, elapsedMs≈0, 0+31000 > 30000 → throws + await expect(getChannelMessages()).rejects.toThrow( + "Discord rate limit exceeded", + ); + + expect(logErrorSpy).toHaveBeenCalledWith( + { endpoint, elapsedMs: expect.any(Number), waitMs: 31_000, retries: 0 }, + "Discord rate limit exceeded max wait time", + ); + }); + + it("should include retry count in log messages", async () => { + const logWarnSpy = vi.spyOn(log, "warn").mockImplementation(() => {}); + let attempts = 0; + + server.use( + http.get(`${DISCORD_BASE_URL}/channels/:channelId/messages`, () => { + attempts++; + if (attempts <= 3) { + return HttpResponse.json( + { message: "Rate limited", retry_after: 1, global: false }, + { status: 429 }, + ); + } + return HttpResponse.json([]); + }), + ); + + const promise = getChannelMessages(); + await vi.advanceTimersByTimeAsync(3000); + await promise; + + expect(logWarnSpy).toHaveBeenCalledTimes(3); + expect(logWarnSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ retries: 1 }), + "Discord rate limited, retrying", + ); + expect(logWarnSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ retries: 2 }), + "Discord rate limited, retrying", + ); + expect(logWarnSpy).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ retries: 3 }), + "Discord rate limited, retrying", + ); + }); }); diff --git a/app/lib/discord/api.ts b/app/lib/discord/api.ts index d4f82b8f..7be578e8 100644 --- a/app/lib/discord/api.ts +++ b/app/lib/discord/api.ts @@ -15,6 +15,7 @@ import { type DiscordMessage, DiscordMessageSchema } from "./schemas"; const BASE_URL = "https://discord.com/api/v10"; const RATE_LIMIT_TIMEOUT_MS = 30_000; +const MAX_RETRIES = 5; const RateLimitResponseSchema = z.object({ message: z.string(), @@ -23,6 +24,19 @@ const RateLimitResponseSchema = z.object({ code: z.number().optional(), }); +// Per-endpoint rate limit tracking: endpoint → timestamp until which requests should wait +const rateLimitUntil = new Map(); + +/** @internal Exported for test cleanup only */ +export function _resetRateLimitState(): void { + rateLimitUntil.clear(); +} + +/** @internal Exported for testing the gate timeout path */ +export function _setRateLimitGate(endpoint: string, until: number): void { + rateLimitUntil.set(endpoint, until); +} + async function call( method: string, endpoint: string, @@ -38,8 +52,26 @@ async function call( } const startTime = performance.now(); + let retries = 0; while (true) { + // Wait for any active rate limit gate on this endpoint + const waitUntil = rateLimitUntil.get(endpoint); + if (waitUntil != null) { + const waitMs = waitUntil - Date.now(); + if (waitMs > 0) { + const elapsedMs = performance.now() - startTime; + if (elapsedMs + waitMs > RATE_LIMIT_TIMEOUT_MS) { + log.error( + { endpoint, elapsedMs, waitMs, retries }, + "Discord rate limit exceeded max wait time", + ); + throw new Error(`Discord rate limit exceeded`); + } + await new Promise((resolve) => setTimeout(resolve, waitMs)); + } + } + const response = await fetch(url, { method, body: method === "POST" ? JSON.stringify(params) : undefined, @@ -53,6 +85,8 @@ async function call( const json = await response.json(); if (response.status === 429) { + retries++; + const rateLimit = RateLimitResponseSchema.safeParse(json); const retryAfterMs = rateLimit.success ? rateLimit.data.retry_after * 1000 @@ -61,16 +95,24 @@ async function call( const elapsedMs = performance.now() - startTime; - if (elapsedMs + retryAfterMs > RATE_LIMIT_TIMEOUT_MS) { + if ( + retries > MAX_RETRIES || + elapsedMs + retryAfterMs > RATE_LIMIT_TIMEOUT_MS + ) { log.error( - { endpoint, elapsedMs, retryAfterMs, global }, + { endpoint, elapsedMs, retryAfterMs, global, retries }, "Discord rate limit exceeded max wait time", ); throw new Error(`Discord rate limit exceeded`); } + // Update shared rate limit gate (Math.max avoids overwriting a longer wait) + const newUntil = Date.now() + retryAfterMs; + const existing = rateLimitUntil.get(endpoint) ?? 0; + rateLimitUntil.set(endpoint, Math.max(existing, newUntil)); + log.warn( - { endpoint, retryAfterMs, global }, + { endpoint, retryAfterMs, global, retries }, "Discord rate limited, retrying", ); @@ -78,6 +120,9 @@ async function call( continue; } + // Clear rate limit gate on success + rateLimitUntil.delete(endpoint); + if (!response.ok) { log.error( { endpoint, status: response.status, body: json },