diff --git a/src/lib/formatters.avatars.test.ts b/src/lib/formatters.avatars.test.ts index 6ee2678..ae10c68 100644 --- a/src/lib/formatters.avatars.test.ts +++ b/src/lib/formatters.avatars.test.ts @@ -124,6 +124,57 @@ describe('bot avatar resolution', () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); + it('does not re-request a bot the API had no avatar for', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: false } as Response); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('gone[bot]')).resolves.toBeNull(); + await expect(resolveBotAvatar('gone[bot]')).resolves.toBeNull(); + await expect(resolveBotAvatar('gone[bot]')).resolves.toBeNull(); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('does not re-request a bot whose lookup threw', async () => { + const fetchMock = vi.fn().mockRejectedValue(new Error('offline')); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('flaky[bot]')).resolves.toBeNull(); + await expect(resolveBotAvatar('flaky[bot]')).resolves.toBeNull(); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('skips known-unresolvable bots on later preload passes', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: false } as Response); + vi.stubGlobal('fetch', fetchMock); + const { preloadBotAvatars } = await freshFormatters(); + + await preloadBotAvatars(['a[bot]', 'b[bot]']); + expect(fetchMock).toHaveBeenCalledTimes(2); + + await preloadBotAvatars(['a[bot]', 'b[bot]']); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('keeps failures out of storage so a reload retries them', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false } as Response)); + const first = await freshFormatters(); + await first.resolveBotAvatar('later[bot]'); + expect(storage.getItem(AVATAR_STORAGE_KEY)).toBeNull(); + + const fetchMock = vi.fn().mockResolvedValue(okResponse('https://example.test/late.png')); + vi.stubGlobal('fetch', fetchMock); + const second = await freshFormatters(); + + await expect(second.resolveBotAvatar('later[bot]')).resolves.toBe( + 'https://example.test/late.png', + ); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + it('queues requests beyond the concurrency cap but still resolves them all', async () => { const fetchMock = vi.fn().mockResolvedValue(okResponse('https://example.test/c.png')); vi.stubGlobal('fetch', fetchMock); diff --git a/src/lib/formatters.ts b/src/lib/formatters.ts index 86b14aa..787abce 100644 --- a/src/lib/formatters.ts +++ b/src/lib/formatters.ts @@ -368,6 +368,12 @@ const HARDCODED_BOTS = new Set(botAvatarCache.keys()); /** In-flight fetch promises to avoid duplicate API calls */ const pendingFetches = new Map>(); +// Usernames the API had no avatar for. Kept in memory only, never persisted, so +// a reload retries a bot that has since been created or renamed. Without this +// every failed lookup repeats on each dataset change and exhausts the 60/hr +// unauthenticated rate limit, after which no avatars resolve at all. +const unresolvedAvatars = new Set(); + /** Simple throttle: max concurrent GitHub API requests */ const MAX_CONCURRENT_FETCHES = 3; let activeFetchCount = 0; @@ -407,6 +413,7 @@ function throttledFetch(url: string): Promise { */ export async function resolveBotAvatar(username: string): Promise { if (botAvatarCache.has(username)) return botAvatarCache.get(username)!; + if (unresolvedAvatars.has(username)) return null; if (!isBot(username)) return null; // Deduplicate in-flight requests @@ -423,10 +430,15 @@ export async function resolveBotAvatar(username: string): Promise if (url) { botAvatarCache.set(username, url); persistAvatarCache(); + } else { + unresolvedAvatars.add(username); } return url; }) - .catch(() => null) + .catch(() => { + unresolvedAvatars.add(username); + return null; + }) .finally(() => pendingFetches.delete(username)); pendingFetches.set(username, promise); @@ -439,7 +451,9 @@ export async function resolveBotAvatar(username: string): Promise * Caps at 10 API lookups per batch to avoid rate limits. */ export async function preloadBotAvatars(usernames: string[]): Promise { - const bots = usernames.filter((u) => isBot(u) && !botAvatarCache.has(u)); + const bots = usernames.filter( + (u) => isBot(u) && !botAvatarCache.has(u) && !unresolvedAvatars.has(u), + ); if (bots.length === 0) return false; // Cap lookups to avoid hammering the API with many unknown bots const batch = bots.slice(0, 10);