diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index 743b86d..a03f847 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -1,73 +1,164 @@ -import { fetchIndexableReputationHandles } from "./api"; - -/** - * The privacy filter in fetchIndexableReputationHandles is the point where a - * crawlable directory of contributor earnings is prevented from existing at - * all, so it's tested against the real HTTP path rather than a mock. - */ -describe("fetchIndexableReputationHandles", () => { - const originalFetch = global.fetch; - - beforeEach(() => { - jest.clearAllMocks(); - jest.spyOn(console, "error").mockImplementation(() => {}); - }); - - afterAll(() => { - global.fetch = originalFetch; - }); - - function mockUsersResponse(users: unknown) { - global.fetch = jest.fn().mockResolvedValue({ - ok: true, - json: async () => users, - }) as unknown as typeof fetch; - } - - it("returns only profiles that explicitly opted in", async () => { - mockUsersResponse([ - { id: "1", username: "public-priya", isProfilePublic: true }, - { id: "2", username: "private-koda", isProfilePublic: false }, - { id: "3", username: "unspecified-ana" }, - { id: "4", username: "null-flag-marcus", isProfilePublic: null }, - ]); - - const result = await fetchIndexableReputationHandles([]); - - expect(result.source).toBe("live"); - expect(result.data).toEqual(["public-priya"]); - }); - - it("returns nothing when the backend omits the flag entirely", async () => { - // The current state of mergefi-backend: no isProfilePublic field, so the - // safe default applies and no profile is submitted for indexing. - mockUsersResponse([ - { id: "1", username: "a" }, - { id: "2", username: "b" }, - ]); - - const result = await fetchIndexableReputationHandles(["fallback-handle"]); - - expect(result.data).toEqual([]); - }); - - it("drops entries with a blank username", async () => { - mockUsersResponse([ - { id: "1", username: "", isProfilePublic: true }, - { id: "2", username: "real", isProfilePublic: true }, - ]); - - const result = await fetchIndexableReputationHandles([]); - - expect(result.data).toEqual(["real"]); - }); - - it("falls back to the caller's list when the backend is unreachable", async () => { - global.fetch = jest.fn().mockRejectedValue(new Error("offline")) as unknown as typeof fetch; - - const result = await fetchIndexableReputationHandles(["mock-handle"]); - - expect(result.source).toBe("mock"); - expect(result.data).toEqual(["mock-handle"]); - }); -}); +import { fetchIndexableReputationHandles, apiRequest, ApiRequestError } from "./api"; + +/** + * The privacy filter in fetchIndexableReputationHandles is the point where a + * crawlable directory of contributor earnings is prevented from existing at + * all, so it's tested against the real HTTP path rather than a mock. + */ +describe("fetchIndexableReputationHandles", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + function mockUsersResponse(users: unknown) { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: async () => users, + }) as unknown as typeof fetch; + } + + it("returns only profiles that explicitly opted in", async () => { + mockUsersResponse([ + { id: "1", username: "public-priya", isProfilePublic: true }, + { id: "2", username: "private-koda", isProfilePublic: false }, + { id: "3", username: "unspecified-ana" }, + { id: "4", username: "null-flag-marcus", isProfilePublic: null }, + ]); + + const result = await fetchIndexableReputationHandles([]); + + expect(result.source).toBe("live"); + expect(result.data).toEqual(["public-priya"]); + }); + + it("returns nothing when the backend omits the flag entirely", async () => { + // The current state of mergefi-backend: no isProfilePublic field, so the + // safe default applies and no profile is submitted for indexing. + mockUsersResponse([ + { id: "1", username: "a" }, + { id: "2", username: "b" }, + ]); + + const result = await fetchIndexableReputationHandles(["fallback-handle"]); + + expect(result.data).toEqual([]); + }); + + it("drops entries with a blank username", async () => { + mockUsersResponse([ + { id: "1", username: "", isProfilePublic: true }, + { id: "2", username: "real", isProfilePublic: true }, + ]); + + const result = await fetchIndexableReputationHandles([]); + + expect(result.data).toEqual(["real"]); + }); + + it("falls back to the caller's list when the backend is unreachable", async () => { + global.fetch = jest.fn().mockRejectedValue(new Error("offline")) as unknown as typeof fetch; + + const result = await fetchIndexableReputationHandles(["mock-handle"]); + + expect(result.source).toBe("mock"); + expect(result.data).toEqual(["mock-handle"]); + }); +}); + +describe("apiRequest 429 rate-limit handling (#572)", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + // Mirrors the shape of a real fetch Response for the 429 branch; the body is + // never read in that path because apiRequest throws before reaching res.text(). + function mock429(retryAfter: string | null) { + global.fetch = jest.fn().mockResolvedValue({ + ok: false, + status: 429, + statusText: "Too Many Requests", + headers: { + get: (h: string) => (h.toLowerCase() === "retry-after" ? retryAfter : null), + }, + json: async () => ({}), + }) as unknown as typeof fetch; + } + + it("parses a delay-seconds Retry-After and surfaces retryAfter to callers", async () => { + mock429("120"); + let caught: unknown; + try { + await apiRequest("/r572-seconds"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiRequestError); + const e = caught as ApiRequestError; + expect(e.status).toBe(429); + expect(e.retryAfter).toBe(120); + expect(e.message).toBe( + "You're doing that too fast. Please wait 120 seconds before trying again.", + ); + }); + + it("parses an HTTP-date Retry-After (RFC 9110 §10.2.3) into seconds", async () => { + const future = new Date(Date.now() + 90_000).toUTCString(); + mock429(future); + let caught: unknown; + try { + await apiRequest("/r572-httpdate"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiRequestError); + const e = caught as ApiRequestError; + expect(e.status).toBe(429); + expect(Number.isFinite(e.retryAfter)).toBe(true); + expect(e.retryAfter).toBeGreaterThan(0); + expect(e.retryAfter).toBeLessThanOrEqual(95); + }); + + it("uses the plain fallback message when Retry-After is missing", async () => { + mock429(null); + let caught: unknown; + try { + await apiRequest("/r572-missing"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiRequestError); + const e = caught as ApiRequestError; + expect(e.retryAfter).toBeUndefined(); + expect(e.message).toBe( + "You're doing that too fast. Rate limited. Please try again later.", + ); + }); + + it("treats a garbage Retry-After as missing rather than NaN", async () => { + mock429("not-a-number"); + let caught: unknown; + try { + await apiRequest("/r572-garbage"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiRequestError); + const e = caught as ApiRequestError; + expect(e.retryAfter).toBeUndefined(); + expect(e.message).toContain("Rate limited. Please try again later."); + }); +}); diff --git a/src/lib/api.ts b/src/lib/api.ts index 4efa30f..3a890e8 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -1,264 +1,278 @@ -import { API_BASE_URL } from "./config"; -import { getToken } from "./auth"; -import { - adaptBounty, - adaptMilestone, - adaptMaintenancePool, - adaptReputation, - type RawBounty, - type RawMilestone, - type RawMaintenancePool, - type RawReputationSnapshot, - type RawUserProfile, -} from "./adapters"; -import type { Bounty, Milestone, MaintenancePool, ReputationProfile } from "@/types"; - -export class ApiUnavailableError extends Error {} - -export class ApiRequestError extends Error { - constructor( - message: string, - public status: number, - public retryAfter?: number, - ) { - super(message); - } -} - -/** - * Distinguishes live backend data from mock fallback so callers can surface - * a visible indicator without relying on fragile reference-identity checks. - */ -export interface FetchResult { - data: T; - source: "live" | "mock"; -} - -function logFetchError(path: string, kind: "network" | "http" | "parse", detail: string) { - console.error(`[api] ${kind} error on ${path}: ${detail}`); -} - -async function request(path: string, init?: RequestInit): Promise { - const timeout = AbortSignal.timeout(REQUEST_TIMEOUT_MS); - let res: Response; - try { - res = await fetch(`${API_BASE_URL}${path}`, { - cache: "no-store", - ...init, - headers: { "Content-Type": "application/json", ...init?.headers }, - signal: init?.signal ?? timeout, - }); - } catch (err) { - if (err instanceof DOMException && err.name === "TimeoutError") { - logFetchError(path, "network", "Request timed out"); - throw new ApiUnavailableError(`Request to ${path} timed out`); - } - logFetchError(path, "network", err instanceof Error ? err.message : String(err)); - throw new ApiUnavailableError(`Network error on ${path}`); - } - if (!res.ok) { - logFetchError(path, "http", `${res.status} ${res.statusText}`); - throw new ApiUnavailableError(`Request to ${path} failed: ${res.status}`); - } - try { - return (await res.json()) as T; - } catch (err) { - logFetchError(path, "parse", err instanceof Error ? err.message : String(err)); - throw new ApiUnavailableError(`Invalid JSON from ${path}`); - } -} - -const REQUEST_TIMEOUT_MS = 20_000; - -// --------------------------------------------------------------------------- -// In-flight request deduplication (#44) -// --------------------------------------------------------------------------- -// Maps a request signature (method + path + body hash) to the in-flight -// Promise so rapid double-clicks on the same mutation coalesce into one -// network request instead of two. - -const inflight = new Map>(); - -function requestKey(method: string, path: string, body?: string): string { - return `${method}:${path}:${body ?? ""}`; -} - -async function dedupedFetch( - key: string, - fn: () => Promise, -): Promise { - const existing = inflight.get(key); - if (existing) return existing as Promise; - const promise = fn().finally(() => inflight.delete(key)); - inflight.set(key, promise); - return promise; -} - -/** - * Client-side call that attaches the signed-in user's JWT (if any) and - * surfaces backend error bodies instead of silently falling back — used for - * actions the user explicitly triggers (claim, fund, deposit, ...), where - * hiding a failure behind mock data would be misleading. - */ -export async function apiRequest( - path: string, - init?: RequestInit, -): Promise { - const bodyStr = init?.body != null ? String(init.body) : undefined; - const key = requestKey(init?.method ?? "GET", path, bodyStr); - - return dedupedFetch(key, async () => { - const token = getToken(); - const timeout = AbortSignal.timeout(REQUEST_TIMEOUT_MS); - let res: Response; - try { - res = await fetch(`${API_BASE_URL}${path}`, { - ...init, - headers: { - "Content-Type": "application/json", - ...(token ? { Authorization: `Bearer ${token}` } : {}), - ...init?.headers, - }, - signal: init?.signal ?? timeout, - }); - } catch (err) { - if (err instanceof DOMException && err.name === "TimeoutError") { - throw new ApiRequestError("Request timed out — please try again.", 0); - } - throw err; - } - - // --- Rate-limit handling (#44) --- - if (res.status === 429) { - const retryAfter = res.headers.get("Retry-After"); - const seconds = retryAfter ? parseInt(retryAfter, 10) : NaN; - const waitMsg = Number.isFinite(seconds) - ? ` Please wait ${seconds} second${seconds === 1 ? "" : "s"} before trying again.` - : ""; - throw new ApiRequestError( - `You're doing that too fast.${waitMsg}` || `Rate limited. Please try again later.`, - 429, - Number.isFinite(seconds) ? seconds : undefined, - ); - } - - if (!res.ok) { - const body = await res.text(); - let message = body; - try { - const parsed = JSON.parse(body); - // A JSON body without a `.message` (e.g. a NestJS validation error - // shaped like `{statusCode,error,details}`) must not fall back to the - // raw JSON text — that would get rendered verbatim in the UI (#187). - message = parsed.message ?? `Request failed (${res.status})`; - } catch { - // plain-text error body, use as-is - } - throw new ApiRequestError(message || `Request failed (${res.status})`, res.status); - } - if (res.status === 204) return undefined as T; - return res.json() as Promise; - }); -} - -export function apiPost(path: string, body?: unknown): Promise { - return apiRequest(path, { - method: "POST", - body: body !== undefined ? JSON.stringify(body) : undefined, - }); -} - -/** - * Live-data fetchers that adapt mergefi-backend's nested TypeORM entity JSON - * into the flat shapes the UI renders, falling back to mock data (already in - * the target shape) when the backend is unreachable. - */ -export async function fetchBounties(fallback: Bounty[]): Promise> { - try { - const raw = await request("/bounties"); - return { data: raw.map(adaptBounty), source: "live" }; - } catch { - return { data: fallback, source: "mock" }; - } -} - -export async function fetchBounty( - id: string, - fallback: Bounty | undefined, -): Promise> { - try { - const raw = await request(`/bounties/${id}`); - return { data: adaptBounty(raw), source: "live" }; - } catch { - return { data: fallback, source: "mock" }; - } -} - -export async function fetchMilestones(fallback: Milestone[]): Promise> { - try { - const raw = await request("/milestones"); - return { data: raw.map(adaptMilestone), source: "live" }; - } catch { - return { data: fallback, source: "mock" }; - } -} - -export async function fetchMaintenancePools( - fallback: MaintenancePool[], -): Promise> { - try { - const raw = await request("/maintenance-pools"); - return { data: raw.map(adaptMaintenancePool), source: "live" }; - } catch { - return { data: fallback, source: "mock" }; - } -} - -export async function fetchReputationByUsername( - username: string, - fallback: ReputationProfile | null, -): Promise> { - try { - const users = await request<(RawUserProfile & { id: string })[]>("/users"); - const target = username.toLowerCase(); - const user = users.find((u) => u.username.toLowerCase() === target); - if (!user) return { data: fallback, source: "mock" }; - const snapshot = await request( - `/reputation/${user.id}`, - ); - return { data: adaptReputation(user, snapshot), source: "live" }; - } catch { - return { data: fallback, source: "mock" }; - } -} - -/** - * Handles eligible to appear in the sitemap — i.e. profiles whose owner has - * opted into search-engine indexing. - * - * The filter is the enforcement point for the privacy policy documented in - * src/lib/seo-policy.ts: without it, calling the /users endpoint to enumerate - * handles builds a crawlable directory of who earns what, tied to real GitHub - * identities. Anything other than an explicit `isProfilePublic: true` is - * excluded, so the endpoint omitting the field (as it does today) means - * nothing is indexed rather than everything. - */ -export async function fetchIndexableReputationHandles( - fallback: string[], -): Promise> { - try { - const users = await request<(RawUserProfile & { id: string })[]>("/users"); - return { - data: users - .filter((user) => user.isProfilePublic === true) - .map((user) => user.username) - .filter(Boolean), - source: "live", - }; - } catch { - // The mock fixtures represent contributors who have opted in, so local - // development exercises the same code path production will take once the - // backend ships the flag. - return { data: fallback, source: "mock" }; - } -} +import { API_BASE_URL } from "./config"; +import { getToken } from "./auth"; +import { + adaptBounty, + adaptMilestone, + adaptMaintenancePool, + adaptReputation, + type RawBounty, + type RawMilestone, + type RawMaintenancePool, + type RawReputationSnapshot, + type RawUserProfile, +} from "./adapters"; +import type { Bounty, Milestone, MaintenancePool, ReputationProfile } from "@/types"; + +export class ApiUnavailableError extends Error {} + +export class ApiRequestError extends Error { + constructor( + message: string, + public status: number, + public retryAfter?: number, + ) { + super(message); + } +} + +/** + * Distinguishes live backend data from mock fallback so callers can surface + * a visible indicator without relying on fragile reference-identity checks. + */ +export interface FetchResult { + data: T; + source: "live" | "mock"; +} + +function logFetchError(path: string, kind: "network" | "http" | "parse", detail: string) { + console.error(`[api] ${kind} error on ${path}: ${detail}`); +} + +async function request(path: string, init?: RequestInit): Promise { + const timeout = AbortSignal.timeout(REQUEST_TIMEOUT_MS); + let res: Response; + try { + res = await fetch(`${API_BASE_URL}${path}`, { + cache: "no-store", + ...init, + headers: { "Content-Type": "application/json", ...init?.headers }, + signal: init?.signal ?? timeout, + }); + } catch (err) { + if (err instanceof DOMException && err.name === "TimeoutError") { + logFetchError(path, "network", "Request timed out"); + throw new ApiUnavailableError(`Request to ${path} timed out`); + } + logFetchError(path, "network", err instanceof Error ? err.message : String(err)); + throw new ApiUnavailableError(`Network error on ${path}`); + } + if (!res.ok) { + logFetchError(path, "http", `${res.status} ${res.statusText}`); + throw new ApiUnavailableError(`Request to ${path} failed: ${res.status}`); + } + try { + return (await res.json()) as T; + } catch (err) { + logFetchError(path, "parse", err instanceof Error ? err.message : String(err)); + throw new ApiUnavailableError(`Invalid JSON from ${path}`); + } +} + +const REQUEST_TIMEOUT_MS = 20_000; + +// --------------------------------------------------------------------------- +// In-flight request deduplication (#44) +// --------------------------------------------------------------------------- +// Maps a request signature (method + path + body hash) to the in-flight +// Promise so rapid double-clicks on the same mutation coalesce into one +// network request instead of two. + +const inflight = new Map>(); + +function requestKey(method: string, path: string, body?: string): string { + return `${method}:${path}:${body ?? ""}`; +} + +async function dedupedFetch( + key: string, + fn: () => Promise, +): Promise { + const existing = inflight.get(key); + if (existing) return existing as Promise; + const promise = fn().finally(() => inflight.delete(key)); + inflight.set(key, promise); + return promise; +} + +/** + * Client-side call that attaches the signed-in user's JWT (if any) and + * surfaces backend error bodies instead of silently falling back — used for + * actions the user explicitly triggers (claim, fund, deposit, ...), where + * hiding a failure behind mock data would be misleading. + */ +export async function apiRequest( + path: string, + init?: RequestInit, +): Promise { + const bodyStr = init?.body != null ? String(init.body) : undefined; + const key = requestKey(init?.method ?? "GET", path, bodyStr); + + return dedupedFetch(key, async () => { + const token = getToken(); + const timeout = AbortSignal.timeout(REQUEST_TIMEOUT_MS); + let res: Response; + try { + res = await fetch(`${API_BASE_URL}${path}`, { + ...init, + headers: { + "Content-Type": "application/json", + ...(token ? { Authorization: `Bearer ${token}` } : {}), + ...init?.headers, + }, + signal: init?.signal ?? timeout, + }); + } catch (err) { + if (err instanceof DOMException && err.name === "TimeoutError") { + throw new ApiRequestError("Request timed out — please try again.", 0); + } + throw err; + } + + // --- Rate-limit handling (#44, #572) --- + // RFC 9110 §10.2.3 allows Retry-After to be either delay-seconds OR an + // HTTP-date (which proxies/CDNs commonly send). The previous parseInt() + // silently dropped the date form, so callers reading `retryAfter` to + // throttle retries got `undefined`. Parse both forms; the left-hand `||` + // fallback message was also dead code because the template literal is + // always truthy, so the "no wait info" wording was unreachable. + if (res.status === 429) { + const retryAfter = res.headers.get("Retry-After"); + let seconds: number | undefined; + if (retryAfter) { + const trimmed = retryAfter.trim(); + if (/^\d+$/.test(trimmed)) { + seconds = parseInt(trimmed, 10); + } else { + const dateMs = Date.parse(trimmed); + if (!Number.isNaN(dateMs)) { + seconds = Math.max(0, Math.ceil((dateMs - Date.now()) / 1000)); + } + } + } + const message = + seconds != null + ? `You're doing that too fast. Please wait ${seconds} second${seconds === 1 ? "" : "s"} before trying again.` + : `You're doing that too fast. Rate limited. Please try again later.`; + throw new ApiRequestError(message, 429, seconds); + } + + if (!res.ok) { + const body = await res.text(); + let message = body; + try { + const parsed = JSON.parse(body); + // A JSON body without a `.message` (e.g. a NestJS validation error + // shaped like `{statusCode,error,details}`) must not fall back to the + // raw JSON text — that would get rendered verbatim in the UI (#187). + message = parsed.message ?? `Request failed (${res.status})`; + } catch { + // plain-text error body, use as-is + } + throw new ApiRequestError(message || `Request failed (${res.status})`, res.status); + } + if (res.status === 204) return undefined as T; + return res.json() as Promise; + }); +} + +export function apiPost(path: string, body?: unknown): Promise { + return apiRequest(path, { + method: "POST", + body: body !== undefined ? JSON.stringify(body) : undefined, + }); +} + +/** + * Live-data fetchers that adapt mergefi-backend's nested TypeORM entity JSON + * into the flat shapes the UI renders, falling back to mock data (already in + * the target shape) when the backend is unreachable. + */ +export async function fetchBounties(fallback: Bounty[]): Promise> { + try { + const raw = await request("/bounties"); + return { data: raw.map(adaptBounty), source: "live" }; + } catch { + return { data: fallback, source: "mock" }; + } +} + +export async function fetchBounty( + id: string, + fallback: Bounty | undefined, +): Promise> { + try { + const raw = await request(`/bounties/${id}`); + return { data: adaptBounty(raw), source: "live" }; + } catch { + return { data: fallback, source: "mock" }; + } +} + +export async function fetchMilestones(fallback: Milestone[]): Promise> { + try { + const raw = await request("/milestones"); + return { data: raw.map(adaptMilestone), source: "live" }; + } catch { + return { data: fallback, source: "mock" }; + } +} + +export async function fetchMaintenancePools( + fallback: MaintenancePool[], +): Promise> { + try { + const raw = await request("/maintenance-pools"); + return { data: raw.map(adaptMaintenancePool), source: "live" }; + } catch { + return { data: fallback, source: "mock" }; + } +} + +export async function fetchReputationByUsername( + username: string, + fallback: ReputationProfile | null, +): Promise> { + try { + const users = await request<(RawUserProfile & { id: string })[]>("/users"); + const target = username.toLowerCase(); + const user = users.find((u) => u.username.toLowerCase() === target); + if (!user) return { data: fallback, source: "mock" }; + const snapshot = await request( + `/reputation/${user.id}`, + ); + return { data: adaptReputation(user, snapshot), source: "live" }; + } catch { + return { data: fallback, source: "mock" }; + } +} + +/** + * Handles eligible to appear in the sitemap — i.e. profiles whose owner has + * opted into search-engine indexing. + * + * The filter is the enforcement point for the privacy policy documented in + * src/lib/seo-policy.ts: without it, calling the /users endpoint to enumerate + * handles builds a crawlable directory of who earns what, tied to real GitHub + * identities. Anything other than an explicit `isProfilePublic: true` is + * excluded, so the endpoint omitting the field (as it does today) means + * nothing is indexed rather than everything. + */ +export async function fetchIndexableReputationHandles( + fallback: string[], +): Promise> { + try { + const users = await request<(RawUserProfile & { id: string })[]>("/users"); + return { + data: users + .filter((user) => user.isProfilePublic === true) + .map((user) => user.username) + .filter(Boolean), + source: "live", + }; + } catch { + // The mock fixtures represent contributors who have opted in, so local + // development exercises the same code path production will take once the + // backend ships the flag. + return { data: fallback, source: "mock" }; + } +}