From 57998f566f496152d177d3bbbd2c03049694b79c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 16:17:20 +0000 Subject: [PATCH 01/14] 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 02/14] 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 03/14] 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 04/14] 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 05/14] 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 06/14] 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; From 1c19dde6bfa87ad002b4554a140be549ca6c87b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:00:23 +0000 Subject: [PATCH 07/14] refactor: split getSpotifyAccessToken into cache-check and request helpers Extracts getCachedToken() and requestSpotifyToken() to make the credential-check/cache/fetch flow easier to follow. --- .../functions/_shared/spotify-api/auth.ts | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index 3832efac..868dbd16 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -16,21 +16,18 @@ 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"); - if (!clientId || !clientSecret) { - throw new Error("Spotify credentials are not configured"); - } - - if ( - cachedToken && - cachedToken.expiresAt > Date.now() + 60 * 1000 // 1 minute buffer - ) { +function getCachedToken(): string | null { + if (cachedToken && cachedToken.expiresAt > Date.now() + 60 * 1000) { console.log("[getSpotifyAccessToken] Using cached access token"); return cachedToken.token; } + return null; +} +async function requestSpotifyToken( + clientId: string, + clientSecret: string, +): Promise<{ token: string; expiresAt: number }> { console.log("[getSpotifyAccessToken] Requesting access token..."); const tokenUrl = "https://accounts.spotify.com/api/token"; @@ -73,12 +70,11 @@ export async function getSpotifyAccessToken(): Promise { 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; + return { + token: tokenData.access_token, + expiresAt: Date.now() + expiresIn * 1000, + }; } catch (validationError) { console.error("[getSpotifyAccessToken] Invalid token response format:", { error: validationError, @@ -91,3 +87,19 @@ export async function getSpotifyAccessToken(): Promise { throw new Error("Invalid access token response from Spotify"); } } + +export async function getSpotifyAccessToken(): Promise { + const clientId = Deno.env.get("SPOTIFY_CLIENT_ID"); + const clientSecret = Deno.env.get("SPOTIFY_CLIENT_SECRET"); + if (!clientId || !clientSecret) { + throw new Error("Spotify credentials are not configured"); + } + + const cached = getCachedToken(); + if (cached) { + return cached; + } + + cachedToken = await requestSpotifyToken(clientId, clientSecret); + return cachedToken.token; +} From 7a69f3ddce7b024b665db72049806667cfc9ccf9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:02:22 +0000 Subject: [PATCH 08/14] refactor: split requestSpotifyToken into fetch and parse steps Separates the HTTP/retry/error-handling concern (fetchTokenResponse) from response validation (parseTokenResponse) for clarity. --- .../functions/_shared/spotify-api/auth.ts | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index 868dbd16..e0258e55 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -24,10 +24,10 @@ function getCachedToken(): string | null { return null; } -async function requestSpotifyToken( +async function fetchTokenResponse( clientId: string, clientSecret: string, -): Promise<{ token: string; expiresAt: number }> { +): Promise { console.log("[getSpotifyAccessToken] Requesting access token..."); const tokenUrl = "https://accounts.spotify.com/api/token"; @@ -63,8 +63,13 @@ async function requestSpotifyToken( throw new Error("Failed to get Spotify access token"); } - const rawData = result.data; + return result.data; +} +function parseTokenResponse(rawData: unknown): { + token: string; + expiresAt: number; +} { try { const tokenData = SpotifyTokenResponseSchema.parse(rawData); console.log( @@ -79,15 +84,23 @@ async function requestSpotifyToken( console.error("[getSpotifyAccessToken] Invalid token response format:", { error: validationError, rawData: - JSON.stringify({ ...rawData, access_token: "[REDACTED]" }).slice( - 0, - 200, - ) + "...", + JSON.stringify({ + ...(rawData as object), + access_token: "[REDACTED]", + }).slice(0, 200) + "...", }); throw new Error("Invalid access token response from Spotify"); } } +async function requestSpotifyToken( + clientId: string, + clientSecret: string, +): Promise<{ token: string; expiresAt: number }> { + const rawData = await fetchTokenResponse(clientId, clientSecret); + return parseTokenResponse(rawData); +} + export async function getSpotifyAccessToken(): Promise { const clientId = Deno.env.get("SPOTIFY_CLIENT_ID"); const clientSecret = Deno.env.get("SPOTIFY_CLIENT_SECRET"); From 643c7eaa9ee95f9073f32b6577aa6238aecfa3cc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:04:38 +0000 Subject: [PATCH 09/14] refactor: move exported functions to top of auth.ts --- .../functions/_shared/spotify-api/auth.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index e0258e55..06735c7f 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -16,6 +16,22 @@ 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"); + if (!clientId || !clientSecret) { + throw new Error("Spotify credentials are not configured"); + } + + const cached = getCachedToken(); + if (cached) { + return cached; + } + + cachedToken = await requestSpotifyToken(clientId, clientSecret); + return cachedToken.token; +} + function getCachedToken(): string | null { if (cachedToken && cachedToken.expiresAt > Date.now() + 60 * 1000) { console.log("[getSpotifyAccessToken] Using cached access token"); @@ -24,6 +40,14 @@ function getCachedToken(): string | null { return null; } +async function requestSpotifyToken( + clientId: string, + clientSecret: string, +): Promise<{ token: string; expiresAt: number }> { + const rawData = await fetchTokenResponse(clientId, clientSecret); + return parseTokenResponse(rawData); +} + async function fetchTokenResponse( clientId: string, clientSecret: string, @@ -92,27 +116,3 @@ function parseTokenResponse(rawData: unknown): { throw new Error("Invalid access token response from Spotify"); } } - -async function requestSpotifyToken( - clientId: string, - clientSecret: string, -): Promise<{ token: string; expiresAt: number }> { - const rawData = await fetchTokenResponse(clientId, clientSecret); - return parseTokenResponse(rawData); -} - -export async function getSpotifyAccessToken(): Promise { - const clientId = Deno.env.get("SPOTIFY_CLIENT_ID"); - const clientSecret = Deno.env.get("SPOTIFY_CLIENT_SECRET"); - if (!clientId || !clientSecret) { - throw new Error("Spotify credentials are not configured"); - } - - const cached = getCachedToken(); - if (cached) { - return cached; - } - - cachedToken = await requestSpotifyToken(clientId, clientSecret); - return cachedToken.token; -} From 009c4d15e2bcfdf0c16b4b24550b315798b06d51 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:06:33 +0000 Subject: [PATCH 10/14] refactor: extract handleFailedResult from fetchTokenResponse Pulls the rate-limit/other-error branching out of fetchTokenResponse into its own function. --- .../functions/_shared/spotify-api/auth.ts | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/supabase/functions/_shared/spotify-api/auth.ts b/supabase/functions/_shared/spotify-api/auth.ts index 06735c7f..959c5fe7 100644 --- a/supabase/functions/_shared/spotify-api/auth.ts +++ b/supabase/functions/_shared/spotify-api/auth.ts @@ -1,5 +1,5 @@ import { z } from "https://deno.land/x/zod@v3.22.4/mod.ts"; -import { fetchWithRetry } from "../retry-utils.ts"; +import { fetchWithRetry, type RequestResult } from "../retry-utils.ts"; const SpotifyTokenResponseSchema = z.object({ access_token: z.string(), @@ -70,26 +70,32 @@ async function fetchTokenResponse( ); if (!result.success) { - 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"); + handleFailedResult(result); } return result.data; } +function handleFailedResult( + result: Extract, { success: false }>, +): never { + 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"); +} + function parseTokenResponse(rawData: unknown): { token: string; expiresAt: number; From 3cb6ee98639ceeb30abc783efbcb202521262327 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:10:48 +0000 Subject: [PATCH 11/14] test: use FakeTime instead of real delays in retry-utils tests Converts the 429-exhausted and maxDelayMs-cap tests to advance a FakeTime clock instead of waiting on real setTimeout delays, matching the pattern already used by the other retry-utils tests. Removes reliance on wall-clock elapsed-time assertions, which were flaky under load. --- .../functions/_shared/retry-utils.test.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts index 57169958..2f60650d 100644 --- a/supabase/functions/_shared/retry-utils.test.ts +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -27,6 +27,7 @@ Deno.test( Deno.test( "fetchWithRetry returns rate-limit error on 429 after retries", async function fetchWithRetry429Exhausted() { + using time = new FakeTime(); let attemptCount = 0; function mockFetch() { @@ -43,11 +44,13 @@ Deno.test( return { data: "should not reach here" }; } - const result = await fetchWithRetry(mockFetch, mockParse, { + const resultPromise = fetchWithRetry(mockFetch, mockParse, { maxRetries: 2, initialDelayMs: 10, maxDelayMs: 100, }); + await time.tickAsync(1000); + const result = await resultPromise; assertEquals(result.success, false); if (!result.success && result.type === "rate-limit") { @@ -282,10 +285,13 @@ Deno.test( Deno.test( "fetchWithRetry caps delay at maxDelayMs even with large Retry-After", async function fetchWithRetryCapAtMax() { - let _attemptCount = 0; + using time = new FakeTime(); + let attemptCount = 0; + const attemptTimes: number[] = []; function mockFetch() { - _attemptCount++; + attemptCount++; + attemptTimes.push(Date.now()); return Promise.resolve( new Response(null, { status: 429, @@ -298,24 +304,24 @@ Deno.test( return { data: "should not reach here" }; } - const startTime = Date.now(); - const result = await fetchWithRetry(mockFetch, mockParse, { + const resultPromise = fetchWithRetry(mockFetch, mockParse, { maxRetries: 1, initialDelayMs: 100, maxDelayMs: 200, }); - - const elapsedMs = Date.now() - startTime; + await time.tickAsync(200); + const result = await resultPromise; assertEquals(result.success, false); if (!result.success && result.type === "rate-limit") { assertEquals(result.retryAfterSeconds, 3600); } + assertEquals(attemptCount, 2); assertEquals( - elapsedMs <= 400, - true, - `Total time ${elapsedMs}ms should be capped around maxDelayMs 200ms (with buffer)`, + attemptTimes[1] - attemptTimes[0], + 200, + "Delay between attempts should be capped at maxDelayMs (200ms)", ); }, ); From 64f555fe30ef97e5fd22bb263c0ddc6ec8b99ff2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:14:05 +0000 Subject: [PATCH 12/14] refactor: extract shared fetchSpotifyAPI helper, decompose spotify-adapter Adds fetchSpotifyAPI to _shared/spotify-api/api.ts, mirroring the fetchSoundCloudAPI pattern: retry + schema validation in one place. getSpotifyArtistById now goes through it too, gaining 429 retry handling it previously lacked. spotify-adapter.ts's searchSpotify is split into searchSpotifyArtist, handleSearchFailure, and fillRemainingWithRateLimit for readability. Also adds an optional status field to retry-utils' OtherError so callers can branch on HTTP status (e.g. 404) without inspecting error message text. --- supabase/functions/_shared/retry-utils.ts | 2 + supabase/functions/_shared/spotify-api/api.ts | 130 ++++++++------ .../search-artist-links/spotify-adapter.ts | 163 +++++++++--------- 3 files changed, 162 insertions(+), 133 deletions(-) diff --git a/supabase/functions/_shared/retry-utils.ts b/supabase/functions/_shared/retry-utils.ts index 1dd2c9f5..b7aabef2 100644 --- a/supabase/functions/_shared/retry-utils.ts +++ b/supabase/functions/_shared/retry-utils.ts @@ -13,6 +13,7 @@ export interface OtherError { success: false; type: "other"; error: unknown; + status?: number; } export type RequestResult = RetryResult | RateLimitError | OtherError; @@ -83,6 +84,7 @@ export async function fetchWithRetry( success: false, type: "other", error: new Error(`HTTP ${response.status}: ${response.statusText}`), + status: response.status, }; } diff --git a/supabase/functions/_shared/spotify-api/api.ts b/supabase/functions/_shared/spotify-api/api.ts index a6958075..1b395421 100644 --- a/supabase/functions/_shared/spotify-api/api.ts +++ b/supabase/functions/_shared/spotify-api/api.ts @@ -1,3 +1,5 @@ +import { z } from "https://deno.land/x/zod@v3.22.4/mod.ts"; +import { fetchWithRetry, type RequestResult } from "../retry-utils.ts"; import { getSpotifyAccessToken } from "./auth.ts"; import { SpotifyArtistSchema } from "./schemas.ts"; import { normalizeSpotifySearchResult } from "../normalize.ts"; @@ -19,70 +21,68 @@ export function extractSpotifyArtistId(url: string): string | null { } } -export async function getSpotifyArtistById( - artistId: string, -): Promise { - try { - const accessToken = await getSpotifyAccessToken(); +export async function fetchSpotifyAPI( + endpoint: string, + accessToken: string, + schema: z.ZodSchema, +): Promise> { + const fullUrl = `https://api.spotify.com/v1${endpoint}`; + console.log(`[fetchSpotifyAPI] Making request to: ${fullUrl}`); - console.log(`[getSpotifyArtistById] Fetching artist: ${artistId}`); + const result = await fetchWithRetry( + () => + fetch(fullUrl, { + headers: { + Authorization: `Bearer ${accessToken}`, + "Content-Type": "application/json", + }, + }), + async (response) => response.json(), + { maxRetries: 2 }, + ); - const endpoint = `https://api.spotify.com/v1/artists/${encodeURIComponent(artistId)}`; + if (!result.success) { + return result; + } - const response = await fetch(endpoint, { - headers: { - Authorization: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, + const parseResponse = schema.safeParse(result.data); + if (!parseResponse.success) { + console.error(`[fetchSpotifyAPI] Validation error for ${endpoint}:`, { + error: parseResponse.error, + rawData: JSON.stringify(result.data).slice(0, 200) + "...", }); + return { + success: false, + type: "other", + error: new Error("Spotify returned data in an unexpected format"), + }; + } - if (!response.ok) { - const errorText = await response - .text() - .catch(() => "Unable to read error response"); - console.error( - `[getSpotifyArtistById] Error fetching artist ${artistId}:`, - { - status: response.status, - statusText: response.statusText, - body: errorText, - }, - ); + return { success: true, data: parseResponse.data }; +} - if (response.status === 404) { - return { - candidate: null, - error: "Artist not found on Spotify", - }; - } +export async function getSpotifyArtistById( + artistId: string, +): Promise { + try { + const accessToken = await getSpotifyAccessToken(); - return { - candidate: null, - error: `Spotify lookup failed (${response.status})`, - }; - } + console.log(`[getSpotifyArtistById] Fetching artist: ${artistId}`); - const rawData = await response.json(); - const parseResponse = SpotifyArtistSchema.safeParse(rawData); + const endpoint = `/artists/${encodeURIComponent(artistId)}`; + const result = await fetchSpotifyAPI( + endpoint, + accessToken, + SpotifyArtistSchema, + ); - if (!parseResponse.success) { - console.error( - `[getSpotifyArtistById] Validation error for artist ${artistId}:`, - { - error: parseResponse.error, - }, - ); - return { - candidate: null, - error: "Spotify returned data in an unexpected format", - }; + if (!result.success) { + return handleFetchFailure(artistId, result); } - const artist = parseResponse.data; - const candidate = normalizeSpotifySearchResult(artist); - + const artist = result.data; console.log(`[getSpotifyArtistById] Found artist: ${artist.name}`); - return { candidate }; + return { candidate: normalizeSpotifySearchResult(artist) }; } catch (error) { console.error( `[getSpotifyArtistById] Error fetching artist ${artistId}:`, @@ -94,3 +94,29 @@ export async function getSpotifyArtistById( }; } } + +function handleFetchFailure( + artistId: string, + result: Extract, { success: false }>, +): ProviderFetchOutcome { + if (result.type === "rate-limit") { + console.error( + `[getSpotifyArtistById] Rate limited fetching artist ${artistId}, retry after ${result.retryAfterSeconds}s`, + ); + return { candidate: null, error: "Spotify rate limited" }; + } + + console.error( + `[getSpotifyArtistById] Error fetching artist ${artistId}:`, + result.error, + ); + + if (result.status === 404) { + return { candidate: null, error: "Artist not found on Spotify" }; + } + + return { + candidate: null, + error: `Spotify lookup failed${result.status ? ` (${result.status})` : ""}`, + }; +} diff --git a/supabase/functions/search-artist-links/spotify-adapter.ts b/supabase/functions/search-artist-links/spotify-adapter.ts index 1cf8cd97..e408e96e 100644 --- a/supabase/functions/search-artist-links/spotify-adapter.ts +++ b/supabase/functions/search-artist-links/spotify-adapter.ts @@ -1,104 +1,105 @@ import { getSpotifyAccessToken } from "../_shared/spotify-api/auth.ts"; +import { fetchSpotifyAPI } from "../_shared/spotify-api/api.ts"; import { SpotifySearchResponseSchema } from "../_shared/spotify-api/schemas.ts"; import { normalizeSpotifySearchResult } from "../_shared/normalize.ts"; -import { fetchWithRetry } from "../_shared/retry-utils.ts"; +import type { RequestResult } from "../_shared/retry-utils.ts"; import type { ProviderSearchOutcome } from "./types.ts"; export async function searchSpotify( artistNames: string[], ): Promise> { const results = new Map(); - const accessToken = await getSpotifyAccessToken(); for (const artistName of artistNames) { - try { - console.log(`[searchSpotify] Searching for artist: ${artistName}`); + if (results.has(artistName)) continue; - const query = encodeURIComponent(artistName); - const endpoint = `https://api.spotify.com/v1/search?type=artist&q=${query}&limit=10`; + const outcome = await searchSpotifyArtist(artistName, accessToken); + results.set(artistName, outcome); - const result = await fetchWithRetry( - () => - fetch(endpoint, { - headers: { - Authorization: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }), - async (response) => response.json(), - { maxRetries: 2 }, + if (outcome.rateLimitRetryAfter !== undefined) { + fillRemainingWithRateLimit( + results, + artistNames, + outcome.rateLimitRetryAfter, ); + break; + } + } - 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, - }); - 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}:`, - result.error, - ); - results.set(artistName, { - candidates: [], - error: "Spotify search failed", - }); - } - continue; - } - - const rawData = result.data; + return results; +} - const parseResponse = SpotifySearchResponseSchema.safeParse(rawData); - if (!parseResponse.success) { - console.error( - `[searchSpotify] Validation error for artist ${artistName}:`, - { - error: parseResponse.error, - }, - ); - results.set(artistName, { - candidates: [], - error: "Spotify search failed", - }); - continue; - } +async function searchSpotifyArtist( + artistName: string, + accessToken: string, +): Promise { + try { + console.log(`[searchSpotify] Searching for artist: ${artistName}`); - const searchData = parseResponse.data; - const candidates = (searchData.artists?.items || []).map( - normalizeSpotifySearchResult, - ); + const endpoint = `/search?type=artist&q=${encodeURIComponent(artistName)}&limit=10`; + const result = await fetchSpotifyAPI( + endpoint, + accessToken, + SpotifySearchResponseSchema, + ); - results.set(artistName, { candidates }); - console.log( - `[searchSpotify] Found ${candidates.length} candidates for ${artistName}`, - ); - } catch (error) { - console.error( - `[searchSpotify] Error searching for artist ${artistName}:`, - error, - ); - results.set(artistName, { - candidates: [], - error: error instanceof Error ? error.message : "Spotify search failed", - }); + if (!result.success) { + return handleSearchFailure(artistName, result); } + + const candidates = (result.data.artists?.items || []).map( + normalizeSpotifySearchResult, + ); + console.log( + `[searchSpotify] Found ${candidates.length} candidates for ${artistName}`, + ); + return { candidates }; + } catch (error) { + console.error( + `[searchSpotify] Error searching for artist ${artistName}:`, + error, + ); + return { + candidates: [], + error: error instanceof Error ? error.message : "Spotify search failed", + }; } +} - return results; +function handleSearchFailure( + artistName: string, + result: Extract, { success: false }>, +): ProviderSearchOutcome { + if (result.type === "rate-limit") { + console.error( + `[searchSpotify] Rate limited for artist ${artistName}, retry after ${result.retryAfterSeconds}s`, + ); + return { + candidates: [], + error: "Spotify rate limited", + rateLimitRetryAfter: result.retryAfterSeconds, + }; + } + + console.error( + `[searchSpotify] Error searching for artist ${artistName}:`, + result.error, + ); + return { candidates: [], error: "Spotify search failed" }; +} + +function fillRemainingWithRateLimit( + results: Map, + artistNames: string[], + retryAfterSeconds: number, +): void { + for (const artistName of artistNames) { + if (results.has(artistName)) continue; + results.set(artistName, { + candidates: [], + error: "Spotify rate limited", + rateLimitRetryAfter: retryAfterSeconds, + }); + } } From 5a6c4f051dd791c765aee0442d0ad9a42a7a3eb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 05:25:15 +0000 Subject: [PATCH 13/14] fix(test): tick FakeTime once per sequential retry in 429-exhaustion test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FakeTime.tickAsync jumps straight to the target time after a single microtask flush, so a single big tick only fires the first of two sequential retry sleeps — the second sleep is scheduled only once the fake clock has already jumped past it, so it never fires and the test hangs. That leaves the FakeTime instance's using-disposal stuck mid-await, leaking faked globals into every later test in the file (the "Cannot construct FakeTime: time is already faked" failures). Ticking once per expected sleep fixes all five cascading failures. --- supabase/functions/_shared/retry-utils.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts index 2f60650d..dcf9fd8f 100644 --- a/supabase/functions/_shared/retry-utils.test.ts +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -49,7 +49,13 @@ Deno.test( initialDelayMs: 10, maxDelayMs: 100, }); - await time.tickAsync(1000); + // Two retries means two sequential sleeps; each is only scheduled once + // the previous one's continuation has run, so tickAsync must be called + // once per sleep instead of a single large jump (a single big tick fires + // the first sleep but "now" jumps past where the second sleep, which is + // registered only afterward, ends up scheduled). + await time.tickAsync(200); + await time.tickAsync(200); const result = await resultPromise; assertEquals(result.success, false); From 863d4c3d58f402b22734e4c6f73196ccf4a3972b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 05:40:12 +0000 Subject: [PATCH 14/14] style: shorten FakeTime tick comment to one line --- supabase/functions/_shared/retry-utils.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/supabase/functions/_shared/retry-utils.test.ts b/supabase/functions/_shared/retry-utils.test.ts index dcf9fd8f..23464a2f 100644 --- a/supabase/functions/_shared/retry-utils.test.ts +++ b/supabase/functions/_shared/retry-utils.test.ts @@ -49,11 +49,7 @@ Deno.test( initialDelayMs: 10, maxDelayMs: 100, }); - // Two retries means two sequential sleeps; each is only scheduled once - // the previous one's continuation has run, so tickAsync must be called - // once per sleep instead of a single large jump (a single big tick fires - // the first sleep but "now" jumps past where the second sleep, which is - // registered only afterward, ends up scheduled). + // Two retries need two ticks: a single big jump skips the second sleep, which is only registered after the first resolves. await time.tickAsync(200); await time.tickAsync(200); const result = await resultPromise;