From 57998f566f496152d177d3bbbd2c03049694b79c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 16:17:20 +0000 Subject: [PATCH 1/6] feat: add Spotify 429 rate-limit retry logic with backoff Implement retry-with-backoff helper for handling Spotify API 429 responses: - New fetchWithRetry utility honors Retry-After header for rate limits - Spotify search and token refresh now retry twice with exponential backoff - Rate-limit errors are distinguishable from other failures with wait time - UI displays "Rate limited. Try again in Ns" message when applicable - Includes comprehensive unit tests for retry scenarios Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01E2ci2Q1QgHtkMY93c2zf9C --- src/api/artistSearch/types.ts | 1 + .../LinkWizard/useProviderCandidates.ts | 34 ++- .../functions/_shared/retry-utils.test.ts | 228 ++++++++++++++++++ supabase/functions/_shared/retry-utils.ts | 140 +++++++++++ .../functions/_shared/spotify-api/auth.ts | 91 ++++--- supabase/functions/_shared/types.ts | 1 + .../functions/search-artist-links/index.ts | 3 + .../search-artist-links/spotify-adapter.ts | 56 +++-- .../functions/search-artist-links/types.ts | 1 + 9 files changed, 479 insertions(+), 76 deletions(-) create mode 100644 supabase/functions/_shared/retry-utils.test.ts create mode 100644 supabase/functions/_shared/retry-utils.ts diff --git a/src/api/artistSearch/types.ts b/src/api/artistSearch/types.ts index cfc57ab5..81120fb8 100644 --- a/src/api/artistSearch/types.ts +++ b/src/api/artistSearch/types.ts @@ -18,6 +18,7 @@ export const searchResultSchema = z.object({ provider: providerSchema, candidates: z.array(candidateSchema), error: z.string().optional(), + rateLimitRetryAfter: z.number().optional(), }); export type SearchResult = z.infer; diff --git a/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts b/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts index 8da7e606..109fc64a 100644 --- a/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts +++ b/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts @@ -26,7 +26,7 @@ export function useProviderCandidates( const isLoading = batchQueryResult.isLoading || customResult.isLoading; - const { candidates, error } = resolveProviderResult({ + const { candidates, error, rateLimitRetryAfter } = resolveProviderResult({ provider, artistName, customSearch, @@ -42,7 +42,9 @@ export function useProviderCandidates( setCustomSearch(query); } - return { candidates, error, isLoading, search }; + const displayError = buildErrorMessage(error, rateLimitRetryAfter); + + return { candidates, error: displayError, isLoading, search }; } interface ResolveProviderResultArgs { @@ -62,6 +64,7 @@ function resolveProviderResult({ }: ResolveProviderResultArgs): { candidates: Candidate[]; error?: string | undefined; + rateLimitRetryAfter?: number | undefined; } { const providerLabel = PROVIDER_LABELS[provider]; @@ -75,7 +78,11 @@ function resolveProviderResult({ const result = customResult.data?.results.find( (r) => r.provider === provider, ); - return { candidates: result?.candidates ?? [], error: result?.error }; + return { + candidates: result?.candidates ?? [], + error: result?.error, + rateLimitRetryAfter: result?.rateLimitRetryAfter, + }; } if (batchQueryResult.isError) { @@ -88,5 +95,24 @@ function resolveProviderResult({ const result = batchQueryResult.data?.results.find( (r) => r.artistName === artistName && r.provider === provider, ); - return { candidates: result?.candidates ?? [], error: result?.error }; + return { + candidates: result?.candidates ?? [], + error: result?.error, + rateLimitRetryAfter: result?.rateLimitRetryAfter, + }; +} + +function buildErrorMessage( + error?: string, + retryAfterSeconds?: number, +): string | undefined { + if (!error) { + return undefined; + } + + if (error.includes("rate limited") && retryAfterSeconds) { + return `Rate limited. Try again in ${retryAfterSeconds} second${retryAfterSeconds > 1 ? "s" : ""}.`; + } + + return error; } diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts new file mode 100644 index 00000000..80f8c273 --- /dev/null +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -0,0 +1,228 @@ +import { assertEquals, assertExists } from "jsr:@std/assert@1"; +import { fetchWithRetry } from "./retry-utils.ts"; + +Deno.test( + "fetchWithRetry succeeds on first try", + async function fetchWithRetryFirstTry() { + function mockFetch() { + return Promise.resolve( + new Response(JSON.stringify({ data: "success" }), { status: 200 }), + ); + } + + async function mockParse(response: Response) { + return response.json() as Promise<{ data: string }>; + } + + const result = await fetchWithRetry(mockFetch, mockParse); + + assertEquals(result.success, true); + if (result.success) { + assertEquals(result.data.data, "success"); + } + }, +); + +Deno.test( + "fetchWithRetry returns rate-limit error on 429 after retries", + async function fetchWithRetry429Exhausted() { + let attemptCount = 0; + + function mockFetch() { + attemptCount++; + return Promise.resolve( + new Response(null, { + status: 429, + headers: { "Retry-After": "30" }, + }), + ); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 2, + initialDelayMs: 10, + maxDelayMs: 100, + }); + + assertEquals(result.success, false); + if (!result.success && result.type === "rate-limit") { + assertEquals(result.retryAfterSeconds, 30); + assertEquals(attemptCount, 3); + } + }, +); + +Deno.test( + "fetchWithRetry retries and succeeds on 429 then 200", + async function fetchWithRetryRecovery() { + let attemptCount = 0; + + async function mockFetch() { + attemptCount++; + if (attemptCount === 1) { + return new Response(null, { + status: 429, + headers: { "Retry-After": "1" }, + }); + } + return new Response(JSON.stringify({ data: "success" }), { + status: 200, + }); + } + + async function mockParse(response: Response) { + return response.json() as Promise<{ data: string }>; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 2, + initialDelayMs: 10, + maxDelayMs: 100, + }); + + assertEquals(result.success, true); + if (result.success) { + assertEquals(result.data.data, "success"); + assertEquals(attemptCount, 2); + } + }, +); + +Deno.test( + "fetchWithRetry does not retry on non-429 errors", + async function fetchWithRetryNon429() { + let attemptCount = 0; + + function mockFetch() { + attemptCount++; + return Promise.resolve( + new Response(null, { + status: 500, + statusText: "Internal Server Error", + }), + ); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 2, + }); + + assertEquals(result.success, false); + if (!result.success && result.type === "other") { + assertExists(result.error); + assertEquals(attemptCount, 1); + } + }, +); + +Deno.test( + "fetchWithRetry parses Retry-After as numeric seconds", + async function fetchWithRetryAfterNumeric() { + function mockFetch() { + return Promise.resolve( + new Response(null, { + status: 429, + headers: { "Retry-After": "45" }, + }), + ); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 0, + }); + + assertEquals(result.success, false); + if (!result.success && result.type === "rate-limit") { + assertEquals(result.retryAfterSeconds, 45); + } + }, +); + +Deno.test( + "fetchWithRetry defaults to 60 seconds when Retry-After is missing", + async function fetchWithRetryAfterDefault() { + function mockFetch() { + return Promise.resolve( + new Response(null, { + status: 429, + }), + ); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 0, + }); + + assertEquals(result.success, false); + if (!result.success && result.type === "rate-limit") { + assertEquals(result.retryAfterSeconds, 60); + } + }, +); + +Deno.test( + "fetchWithRetry respects maxRetries option", + async function fetchWithRetryMaxRetries() { + let attemptCount = 0; + + function mockFetch() { + attemptCount++; + return Promise.resolve( + new Response(null, { + status: 429, + headers: { "Retry-After": "1" }, + }), + ); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 1, + initialDelayMs: 10, + }); + + assertEquals(result.success, false); + assertEquals(attemptCount, 2); + }, +); + +Deno.test( + "fetchWithRetry returns other error when fetch throws", + async function fetchWithRetryFetchThrows() { + function mockFetch() { + return Promise.reject(new Error("Network error")); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 0, + }); + + assertEquals(result.success, false); + if (!result.success && result.type === "other") { + assertExists(result.error); + } + }, +); diff --git a/supabase/functions/_shared/retry-utils.ts b/supabase/functions/_shared/retry-utils.ts new file mode 100644 index 00000000..e17c5670 --- /dev/null +++ b/supabase/functions/_shared/retry-utils.ts @@ -0,0 +1,140 @@ +export interface RetryResult { + success: true; + data: T; +} + +export interface RateLimitError { + success: false; + type: "rate-limit"; + retryAfterSeconds: number; +} + +export interface OtherError { + success: false; + type: "other"; + error: unknown; +} + +export type RequestResult = RetryResult | RateLimitError | OtherError; + +interface FetchOptions { + maxRetries?: number; + initialDelayMs?: number; + maxDelayMs?: number; +} + +export async function fetchWithRetry( + fn: () => Promise, + parseResponse: (response: Response) => Promise, + options: FetchOptions = {}, +): Promise> { + const maxRetries = options.maxRetries ?? 2; + const initialDelayMs = options.initialDelayMs ?? 100; + const maxDelayMs = options.maxDelayMs ?? 32000; + + let lastError: unknown; + + for (let attempt = 0; attempt <= maxRetries; attempt++) { + try { + const response = await fn(); + + if (!response.ok) { + if (response.status === 429) { + const retryAfter = response.headers.get("Retry-After"); + const retryAfterSeconds = parseRetryAfter(retryAfter); + + if (attempt < maxRetries) { + const delay = Math.min( + initialDelayMs * Math.pow(2, attempt), + maxDelayMs, + ); + console.log( + `[fetchWithRetry] Received 429, retrying after ${delay}ms (attempt ${attempt + 1}/${maxRetries})`, + ); + await sleep(delay); + continue; + } + + console.error( + `[fetchWithRetry] Rate limited after ${maxRetries} retries, returning rate-limit error`, + ); + return { + success: false, + type: "rate-limit", + retryAfterSeconds, + }; + } + + const errorText = await response + .text() + .catch(() => "Unable to read error response"); + console.error( + `[fetchWithRetry] Request failed with status ${response.status}:`, + { + status: response.status, + statusText: response.statusText, + body: errorText, + }, + ); + + return { + success: false, + type: "other", + error: new Error(`HTTP ${response.status}: ${response.statusText}`), + }; + } + + const data = await parseResponse(response); + return { success: true, data }; + } catch (error) { + lastError = error; + console.error( + `[fetchWithRetry] Fetch attempt ${attempt + 1} failed:`, + error, + ); + + if (attempt < maxRetries) { + const delay = Math.min( + initialDelayMs * Math.pow(2, attempt), + maxDelayMs, + ); + console.log(`[fetchWithRetry] Retrying after ${delay}ms`); + await sleep(delay); + } + } + } + + return { + success: false, + type: "other", + error: lastError ?? new Error("Unknown error"), + }; +} + +function parseRetryAfter(retryAfterHeader: string | null): number { + if (!retryAfterHeader) { + return 60; + } + + const seconds = parseInt(retryAfterHeader, 10); + if (!isNaN(seconds) && seconds > 0) { + return seconds; + } + + try { + const retryDate = new Date(retryAfterHeader); + const now = new Date(); + const diffMs = retryDate.getTime() - now.getTime(); + if (diffMs > 0) { + return Math.ceil(diffMs / 1000); + } + } catch { + // Invalid date format, use default + } + + return 60; +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index da0ef5ed..6da42083 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -1,4 +1,5 @@ import { z } from "https://deno.land/x/zod@v3.22.4/mod.ts"; +import { fetchWithRetry } from "../retry-utils.ts"; const SpotifyTokenResponseSchema = z.object({ access_token: z.string(), @@ -29,62 +30,54 @@ export async function getSpotifyAccessToken(): Promise { console.log("[getSpotifyAccessToken] Requesting access token..."); const tokenUrl = "https://accounts.spotify.com/api/token"; - try { - const response = await fetch(tokenUrl, { - method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - Authorization: `Basic ${btoa(`${clientId}:${clientSecret}`)}`, - }, - body: "grant_type=client_credentials", - }); + const result = await fetchWithRetry( + () => + fetch(tokenUrl, { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + Authorization: `Basic ${btoa(`${clientId}:${clientSecret}`)}`, + }, + body: "grant_type=client_credentials", + }), + async (response) => response.json(), + { maxRetries: 2 }, + ); - console.log( - `[getSpotifyAccessToken] Token response status: ${response.status} ${response.statusText}`, - ); - - if (!response.ok) { - const errorBody = await response - .text() - .catch(() => "Unable to read error response"); - console.error("[getSpotifyAccessToken] Failed to get access token:", { - status: response.status, - statusText: response.statusText, - body: errorBody, - }); + if (!result.success) { + console.error("[getSpotifyAccessToken] Error obtaining access token:", { + error: result.error, + }); + if (result.type === "rate-limit") { throw new Error( - `Failed to get Spotify access token: ${response.statusText}`, + `Failed to get Spotify access token: rate limited (retry after ${result.retryAfterSeconds}s)`, ); } + throw new Error("Failed to get Spotify access token"); + } - const rawData = await response.json(); + const rawData = result.data; - try { - const tokenData = SpotifyTokenResponseSchema.parse(rawData); - console.log( - "[getSpotifyAccessToken] Successfully obtained and validated access token", - ); - const token = tokenData.access_token; - const expiresIn = tokenData.expires_in ?? 3600; // Default to 1 hour if not provided - const expiresAt = Date.now() + expiresIn * 1000; + try { + const tokenData = SpotifyTokenResponseSchema.parse(rawData); + console.log( + "[getSpotifyAccessToken] Successfully obtained and validated access token", + ); + const token = tokenData.access_token; + const expiresIn = tokenData.expires_in ?? 3600; // Default to 1 hour if not provided + const expiresAt = Date.now() + expiresIn * 1000; - cachedToken = { token, expiresAt }; - return token; - } catch (validationError) { - console.error("[getSpotifyAccessToken] Invalid token response format:", { - error: validationError, - rawData: - JSON.stringify({ ...rawData, access_token: "[REDACTED]" }).slice( - 0, - 200, - ) + "...", - }); - throw new Error("Invalid access token response from Spotify"); - } - } catch (error) { - console.error("[getSpotifyAccessToken] Error obtaining access token:", { - error, + cachedToken = { token, expiresAt }; + return token; + } catch (validationError) { + console.error("[getSpotifyAccessToken] Invalid token response format:", { + error: validationError, + rawData: + JSON.stringify({ ...rawData, access_token: "[REDACTED]" }).slice( + 0, + 200, + ) + "...", }); - throw error; + throw new Error("Invalid access token response from Spotify"); } } diff --git a/supabase/functions/_shared/types.ts b/supabase/functions/_shared/types.ts index 5de06875..7e0c59ed 100644 --- a/supabase/functions/_shared/types.ts +++ b/supabase/functions/_shared/types.ts @@ -10,6 +10,7 @@ export interface Candidate { export interface ProviderSearchOutcome { candidates: Candidate[]; error?: string; + rateLimitRetryAfter?: number; } export interface ProviderFetchOutcome { diff --git a/supabase/functions/search-artist-links/index.ts b/supabase/functions/search-artist-links/index.ts index a9afd7fb..2d3f316f 100644 --- a/supabase/functions/search-artist-links/index.ts +++ b/supabase/functions/search-artist-links/index.ts @@ -86,6 +86,9 @@ serve(async (req) => { provider, candidates: outcome.candidates, ...(outcome.error && { error: outcome.error }), + ...(outcome.rateLimitRetryAfter && { + rateLimitRetryAfter: outcome.rateLimitRetryAfter, + }), }); } } catch (providerError) { diff --git a/supabase/functions/search-artist-links/spotify-adapter.ts b/supabase/functions/search-artist-links/spotify-adapter.ts index 3082f6d0..bd4be0eb 100644 --- a/supabase/functions/search-artist-links/spotify-adapter.ts +++ b/supabase/functions/search-artist-links/spotify-adapter.ts @@ -1,6 +1,7 @@ import { getSpotifyAccessToken } from "../_shared/spotify-api/auth.ts"; import { SpotifySearchResponseSchema } from "../_shared/spotify-api/schemas.ts"; import { normalizeSpotifySearchResult } from "../_shared/normalize.ts"; +import { fetchWithRetry } from "../_shared/retry-utils.ts"; import type { ProviderSearchOutcome } from "./types.ts"; export async function searchSpotify( @@ -17,33 +18,42 @@ export async function searchSpotify( const query = encodeURIComponent(artistName); const endpoint = `https://api.spotify.com/v1/search?type=artist&q=${query}&limit=10`; - const response = await fetch(endpoint, { - headers: { - Authorization: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); + const result = await fetchWithRetry( + () => + fetch(endpoint, { + headers: { + Authorization: `Bearer ${accessToken}`, + "Content-Type": "application/json", + }, + }), + async (response) => response.json(), + { maxRetries: 2 }, + ); - if (!response.ok) { - const errorText = await response - .text() - .catch(() => "Unable to read error response"); - console.error( - `[searchSpotify] Error searching for artist ${artistName}:`, - { - status: response.status, - statusText: response.statusText, - body: errorText, - }, - ); - results.set(artistName, { - candidates: [], - error: `Spotify search failed (${response.status})`, - }); + if (!result.success) { + if (result.type === "rate-limit") { + console.error( + `[searchSpotify] Rate limited for artist ${artistName}, retry after ${result.retryAfterSeconds}s`, + ); + results.set(artistName, { + candidates: [], + error: `Spotify rate limited`, + rateLimitRetryAfter: result.retryAfterSeconds, + }); + } else { + console.error( + `[searchSpotify] Error searching for artist ${artistName}:`, + result.error, + ); + results.set(artistName, { + candidates: [], + error: "Spotify search failed", + }); + } continue; } - const rawData = await response.json(); + const rawData = result.data; const parseResponse = SpotifySearchResponseSchema.safeParse(rawData); if (!parseResponse.success) { diff --git a/supabase/functions/search-artist-links/types.ts b/supabase/functions/search-artist-links/types.ts index 2e65895a..fdb3371f 100644 --- a/supabase/functions/search-artist-links/types.ts +++ b/supabase/functions/search-artist-links/types.ts @@ -14,6 +14,7 @@ export interface SearchResult { provider: Provider; candidates: Candidate[]; error?: string; + rateLimitRetryAfter?: number; } export interface SearchResponse { From 2c9c706a81bd38b31c6e4a23151450c3b147727d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 16:20:51 +0000 Subject: [PATCH 2/6] test(api): add integration test for Spotify rate-limit 429 retry Adds integration test covering repeated 429 responses surfacing rate-limit error through full request path. Tests three scenarios: - 429 exhausting retries: returns distinguishable rate-limit error with Retry-After seconds - 429 then success: recovers after one retry with backoff - Non-429 error: fails immediately without retry Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01E2ci2Q1QgHtkMY93c2zf9C --- .../search-artist-links.integration.test.ts | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 supabase/functions/search-artist-links/search-artist-links.integration.test.ts diff --git a/supabase/functions/search-artist-links/search-artist-links.integration.test.ts b/supabase/functions/search-artist-links/search-artist-links.integration.test.ts new file mode 100644 index 00000000..2b7e464e --- /dev/null +++ b/supabase/functions/search-artist-links/search-artist-links.integration.test.ts @@ -0,0 +1,192 @@ +// Integration tests for search-artist-links edge function. +// Tests the full request path with mocked Spotify API responses. +// Run with: deno test --allow-env search-artist-links.integration.test.ts + +import { assertEquals, assertExists } from "jsr:@std/assert@1"; + +let mockFetchCallCount = 0; +let mockFetchResponses: Array<{ + status: number; + headers?: Record; +}> = []; + +async function setupMockFetch( + responses: Array<{ status: number; headers?: Record }>, +) { + mockFetchCallCount = 0; + mockFetchResponses = responses; + + const originalFetch = globalThis.fetch; + + globalThis.fetch = (async (url: string, _options?: RequestInit) => { + if (mockFetchCallCount >= mockFetchResponses.length) { + throw new Error( + `Mock fetch called ${mockFetchCallCount + 1} times, but only ${mockFetchResponses.length} responses configured`, + ); + } + + const response = mockFetchResponses[mockFetchCallCount]; + mockFetchCallCount++; + + const headers = new Headers(response.headers ?? {}); + + if (response.status === 429) { + return new Response(null, { + status: 429, + headers, + }); + } + + if (response.status === 200) { + if (url.includes("/search")) { + const mockSearchResponse = { + artists: { + items: [ + { + id: "test-artist-id", + name: "Test Artist", + genres: ["rock", "pop"], + followers: { total: 1000 }, + images: [{ url: "https://example.com/image.jpg" }], + external_urls: { + spotify: "https://open.spotify.com/artist/test-id", + }, + }, + ], + }, + }; + return new Response(JSON.stringify(mockSearchResponse), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + if (url.includes("/token")) { + const mockTokenResponse = { + access_token: "mock-token-12345", + token_type: "Bearer", + expires_in: 3600, + }; + return new Response(JSON.stringify(mockTokenResponse), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + } + + return new Response(null, { status: response.status, headers }); + }) as typeof fetch; + + return () => { + globalThis.fetch = originalFetch; + }; +} + +Deno.test( + "search-artist-links: repeated 429s exhaust retries and surface rate-limit error", + async function searchArtistLinksRateLimitExhausted() { + const cleanup = await setupMockFetch([ + { status: 200 }, + { status: 429, headers: { "Retry-After": "30" } }, + { status: 429, headers: { "Retry-After": "30" } }, + { status: 429, headers: { "Retry-After": "30" } }, + ]); + + try { + const { searchSpotify } = await import("./spotify-adapter.ts"); + const result = await searchSpotify(["Test Artist"]); + + assertEquals(result.has("Test Artist"), true, "Result has artist key"); + const artistResult = result.get("Test Artist"); + assertExists(artistResult, "Artist result exists"); + assertEquals( + artistResult.candidates.length, + 0, + "No candidates due to rate limit", + ); + assertEquals( + artistResult.error, + "Spotify rate limited", + "Error indicates rate limit", + ); + assertEquals( + artistResult.rateLimitRetryAfter, + 30, + "Rate limit wait time is 30 seconds", + ); + } finally { + cleanup(); + } + }, +); + +Deno.test( + "search-artist-links: 429 followed by success recovers with backoff", + async function searchArtistLinksRateLimitRecovery() { + const cleanup = await setupMockFetch([ + { status: 200 }, + { status: 429, headers: { "Retry-After": "1" } }, + { status: 200 }, + ]); + + try { + const { searchSpotify } = await import("./spotify-adapter.ts"); + const result = await searchSpotify(["Test Artist"]); + + assertEquals(result.has("Test Artist"), true, "Result has artist key"); + const artistResult = result.get("Test Artist"); + assertExists(artistResult, "Artist result exists"); + assertEquals( + artistResult.candidates.length, + 1, + "Candidates returned after recovery", + ); + assertEquals( + artistResult.candidates[0].name, + "Test Artist", + "Artist name in candidate", + ); + assertEquals(artistResult.error, undefined, "No error after recovery"); + assertEquals( + artistResult.rateLimitRetryAfter, + undefined, + "No rate limit wait time", + ); + } finally { + cleanup(); + } + }, +); + +Deno.test( + "search-artist-links: non-429 errors fail immediately without retry", + async function searchArtistLinksNon429Failure() { + const cleanup = await setupMockFetch([{ status: 200 }, { status: 500 }]); + + try { + const { searchSpotify } = await import("./spotify-adapter.ts"); + const result = await searchSpotify(["Test Artist"]); + + assertEquals(result.has("Test Artist"), true, "Result has artist key"); + const artistResult = result.get("Test Artist"); + assertExists(artistResult, "Artist result exists"); + assertEquals( + artistResult.error, + "Spotify search failed", + "Error indicates search failure", + ); + assertEquals( + artistResult.rateLimitRetryAfter, + undefined, + "No rate limit wait time for non-429", + ); + assertEquals( + mockFetchCallCount, + 2, + "Only two fetch attempts: token + one search", + ); + } finally { + cleanup(); + } + }, +); From 4337c47e119bb5210a8677146ff3eecd10a6c6f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 16:23:18 +0000 Subject: [PATCH 3/6] fix: respect Retry-After header in rate-limit retry delay and decouple UI from error wording Two fixes: 1. fetchWithRetry now incorporates Retry-After header into sleep duration using max(exponentialBackoff, retryAfterMs), capped at maxDelayMs, so we honor the server's requested wait time 2. buildErrorMessage detects rate-limit via rateLimitRetryAfter field directly instead of string matching on error text, decoupling UI from backend error wording Adds unit tests verifying: - Retry-After value is respected and delay is at least as long as requested - Large Retry-After values are capped at maxDelayMs to prevent hanging Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01E2ci2Q1QgHtkMY93c2zf9C --- .../LinkWizard/useProviderCandidates.ts | 6 +- .../functions/_shared/retry-utils.test.ts | 86 +++++++++++++++++++ supabase/functions/_shared/retry-utils.ts | 6 +- 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts b/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts index 109fc64a..032853a7 100644 --- a/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts +++ b/src/pages/admin/festivals/LinkWizard/useProviderCandidates.ts @@ -106,11 +106,7 @@ function buildErrorMessage( error?: string, retryAfterSeconds?: number, ): string | undefined { - if (!error) { - return undefined; - } - - if (error.includes("rate limited") && retryAfterSeconds) { + if (retryAfterSeconds) { return `Rate limited. Try again in ${retryAfterSeconds} second${retryAfterSeconds > 1 ? "s" : ""}.`; } diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts index 80f8c273..b1b452e6 100644 --- a/supabase/functions/_shared/retry-utils.test.ts +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -226,3 +226,89 @@ Deno.test( } }, ); + +Deno.test( + "fetchWithRetry respects Retry-After header in delay", + async function fetchWithRetryRespectRetryAfter() { + let attemptCount = 0; + const attemptTimes: number[] = []; + + async function mockFetch() { + attemptCount++; + attemptTimes.push(Date.now()); + if (attemptCount === 1) { + return new Response(null, { + status: 429, + headers: { "Retry-After": "2" }, + }); + } + return new Response(JSON.stringify({ data: "success" }), { + status: 200, + }); + } + + async function mockParse(response: Response) { + return response.json() as Promise<{ data: string }>; + } + + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 2, + initialDelayMs: 100, + maxDelayMs: 5000, + }); + + assertEquals(result.success, true); + assertEquals(attemptCount, 2); + assertEquals(attemptTimes.length, 2); + + const delayMs = attemptTimes[1] - attemptTimes[0]; + const retryAfterMs = 2000; + + assertEquals( + delayMs >= retryAfterMs - 50, + true, + `Delay ${delayMs}ms should be at least Retry-After ${retryAfterMs}ms (with 50ms tolerance)`, + ); + }, +); + +Deno.test( + "fetchWithRetry caps delay at maxDelayMs even with large Retry-After", + async function fetchWithRetryCapAtMax() { + let attemptCount = 0; + + function mockFetch() { + attemptCount++; + return Promise.resolve( + new Response(null, { + status: 429, + headers: { "Retry-After": "3600" }, + }), + ); + } + + async function mockParse(_response: Response) { + return { data: "should not reach here" }; + } + + const startTime = Date.now(); + const result = await fetchWithRetry(mockFetch, mockParse, { + maxRetries: 1, + initialDelayMs: 100, + maxDelayMs: 200, + }); + + const elapsedMs = Date.now() - startTime; + + assertEquals(result.success, false); + if (!result.success && result.type === "rate-limit") { + assertEquals(result.retryAfterSeconds, 3600); + } + + assertEquals( + elapsedMs <= 400, + true, + `Total time ${elapsedMs}ms should be capped around maxDelayMs 200ms (with buffer)`, + ); + }, +); diff --git a/supabase/functions/_shared/retry-utils.ts b/supabase/functions/_shared/retry-utils.ts index e17c5670..1dd2c9f5 100644 --- a/supabase/functions/_shared/retry-utils.ts +++ b/supabase/functions/_shared/retry-utils.ts @@ -44,12 +44,14 @@ export async function fetchWithRetry( const retryAfterSeconds = parseRetryAfter(retryAfter); if (attempt < maxRetries) { + const exponentialDelay = initialDelayMs * Math.pow(2, attempt); + const retryAfterMs = retryAfterSeconds * 1000; const delay = Math.min( - initialDelayMs * Math.pow(2, attempt), + Math.max(exponentialDelay, retryAfterMs), maxDelayMs, ); console.log( - `[fetchWithRetry] Received 429, retrying after ${delay}ms (attempt ${attempt + 1}/${maxRetries})`, + `[fetchWithRetry] Received 429, retrying after ${delay}ms (Retry-After: ${retryAfterSeconds}s, attempt ${attempt + 1}/${maxRetries})`, ); await sleep(delay); continue; From ba8df66cab3c17f1ba4b94673133910ee180bff0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 06:13:17 +0000 Subject: [PATCH 4/6] fix: rename unused attemptCount in retry-utils test --- supabase/functions/_shared/retry-utils.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts index b1b452e6..43c5b245 100644 --- a/supabase/functions/_shared/retry-utils.test.ts +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -275,10 +275,10 @@ Deno.test( Deno.test( "fetchWithRetry caps delay at maxDelayMs even with large Retry-After", async function fetchWithRetryCapAtMax() { - let attemptCount = 0; + let _attemptCount = 0; function mockFetch() { - attemptCount++; + _attemptCount++; return Promise.resolve( new Response(null, { status: 429, From 89c9ad9466a68542810376a9e81e28ee3b334b3f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 17:58:45 +0000 Subject: [PATCH 5/6] fix: type-narrow rate-limit error, stop searching after Spotify 429, use fake timers in retry tests - getSpotifyAccessToken no longer reads .error off a RateLimitError result (TS2339) - searchSpotify breaks out of the artist loop on a 429 instead of hammering remaining candidates with requests that will just be rate-limited again - retry-utils.test.ts uses Deno's FakeTime instead of waiting on real delays, cutting ~3s of real time off the suite --- supabase/functions/_shared/retry-utils.test.ts | 11 +++++++++-- supabase/functions/_shared/spotify-api/auth.ts | 12 +++++++++--- .../functions/search-artist-links/spotify-adapter.ts | 9 +++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts index 43c5b245..57169958 100644 --- a/supabase/functions/_shared/retry-utils.test.ts +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -1,4 +1,5 @@ import { assertEquals, assertExists } from "jsr:@std/assert@1"; +import { FakeTime } from "jsr:@std/testing@1/time"; import { fetchWithRetry } from "./retry-utils.ts"; Deno.test( @@ -179,6 +180,7 @@ Deno.test( Deno.test( "fetchWithRetry respects maxRetries option", async function fetchWithRetryMaxRetries() { + using time = new FakeTime(); let attemptCount = 0; function mockFetch() { @@ -195,10 +197,12 @@ Deno.test( return { data: "should not reach here" }; } - const result = await fetchWithRetry(mockFetch, mockParse, { + const resultPromise = fetchWithRetry(mockFetch, mockParse, { maxRetries: 1, initialDelayMs: 10, }); + await time.tickAsync(1000); + const result = await resultPromise; assertEquals(result.success, false); assertEquals(attemptCount, 2); @@ -230,6 +234,7 @@ Deno.test( Deno.test( "fetchWithRetry respects Retry-After header in delay", async function fetchWithRetryRespectRetryAfter() { + using time = new FakeTime(); let attemptCount = 0; const attemptTimes: number[] = []; @@ -251,11 +256,13 @@ Deno.test( return response.json() as Promise<{ data: string }>; } - const result = await fetchWithRetry(mockFetch, mockParse, { + const resultPromise = fetchWithRetry(mockFetch, mockParse, { maxRetries: 2, initialDelayMs: 100, maxDelayMs: 5000, }); + await time.tickAsync(2000); + const result = await resultPromise; assertEquals(result.success, true); assertEquals(attemptCount, 2); diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index 6da42083..43569d01 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -45,14 +45,20 @@ export async function getSpotifyAccessToken(): Promise { ); if (!result.success) { - console.error("[getSpotifyAccessToken] Error obtaining access token:", { - error: result.error, - }); if (result.type === "rate-limit") { + console.error( + "[getSpotifyAccessToken] Rate limited obtaining access token", + { + retryAfterSeconds: result.retryAfterSeconds, + }, + ); throw new Error( `Failed to get Spotify access token: rate limited (retry after ${result.retryAfterSeconds}s)`, ); } + console.error("[getSpotifyAccessToken] Error obtaining access token:", { + error: result.error, + }); throw new Error("Failed to get Spotify access token"); } diff --git a/supabase/functions/search-artist-links/spotify-adapter.ts b/supabase/functions/search-artist-links/spotify-adapter.ts index bd4be0eb..1cf8cd97 100644 --- a/supabase/functions/search-artist-links/spotify-adapter.ts +++ b/supabase/functions/search-artist-links/spotify-adapter.ts @@ -40,6 +40,15 @@ export async function searchSpotify( error: `Spotify rate limited`, rateLimitRetryAfter: result.retryAfterSeconds, }); + for (const remaining of artistNames) { + if (remaining === artistName || results.has(remaining)) continue; + results.set(remaining, { + candidates: [], + error: `Spotify rate limited`, + rateLimitRetryAfter: result.retryAfterSeconds, + }); + } + break; } else { console.error( `[searchSpotify] Error searching for artist ${artistName}:`, From 6ac01f5d92ced433ef48acb675eef66775c655de Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 18:04:50 +0000 Subject: [PATCH 6/6] fix: seed Spotify credentials and reset token cache in edge-function integration tests getSpotifyAccessToken threw "Spotify credentials are not configured" in CI since SPOTIFY_CLIENT_ID/SECRET aren't set for deno test. Also the module-level token cache persisted across the file's three Deno.test cases (they share the same imported auth.ts instance), so tests after the first skipped the mocked token fetch and desynced their response queues. --- supabase/functions/_shared/spotify-api/auth.ts | 4 ++++ .../search-artist-links.integration.test.ts | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index 43569d01..3832efac 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -12,6 +12,10 @@ let cachedToken: { expiresAt: number; } | null = null; +export function resetSpotifyTokenCacheForTests(): void { + cachedToken = null; +} + export async function getSpotifyAccessToken(): Promise { const clientId = Deno.env.get("SPOTIFY_CLIENT_ID"); const clientSecret = Deno.env.get("SPOTIFY_CLIENT_SECRET"); diff --git a/supabase/functions/search-artist-links/search-artist-links.integration.test.ts b/supabase/functions/search-artist-links/search-artist-links.integration.test.ts index 2b7e464e..14958bdd 100644 --- a/supabase/functions/search-artist-links/search-artist-links.integration.test.ts +++ b/supabase/functions/search-artist-links/search-artist-links.integration.test.ts @@ -3,6 +3,10 @@ // Run with: deno test --allow-env search-artist-links.integration.test.ts import { assertEquals, assertExists } from "jsr:@std/assert@1"; +import { resetSpotifyTokenCacheForTests } from "../_shared/spotify-api/auth.ts"; + +Deno.env.set("SPOTIFY_CLIENT_ID", "test-client-id"); +Deno.env.set("SPOTIFY_CLIENT_SECRET", "test-client-secret"); let mockFetchCallCount = 0; let mockFetchResponses: Array<{ @@ -15,6 +19,7 @@ async function setupMockFetch( ) { mockFetchCallCount = 0; mockFetchResponses = responses; + resetSpotifyTokenCacheForTests(); const originalFetch = globalThis.fetch;