diff --git a/.env.example b/.env.example index b213d91..8654eae 100644 --- a/.env.example +++ b/.env.example @@ -15,6 +15,7 @@ NEXT_PUBLIC_STELLAR_NETWORK=PUBLIC NEXT_PUBLIC_HORIZON_URL=https://horizon.stellar.org NEXT_PUBLIC_SOROBAN_RPC_URL= NEXT_PUBLIC_CONTRACT_ID= +NEXT_PUBLIC_API_REQUEST_TIMEOUT_MS=15000 # --- Deploy / ops (server or local shell ONLY — never NEXT_PUBLIC_) --- # Populate these in your shell or a gitignored `.env.local`, never in git. diff --git a/docs/frontend-api-timeouts.md b/docs/frontend-api-timeouts.md new file mode 100644 index 0000000..7b0fbde --- /dev/null +++ b/docs/frontend-api-timeouts.md @@ -0,0 +1,36 @@ +# Bounty discovery API timeout and Retry + +The bounty-discovery GET `/api/tasks` and the wallet submission-history GET +`/api/my-submissions` requests have a **15-second** deadline by default, +covering both the network request and its JSON parsing. Override the +frontend deadline at **build time** with `NEXT_PUBLIC_API_REQUEST_TIMEOUT_MS` +(e.g. `NEXT_PUBLIC_API_REQUEST_TIMEOUT_MS=10000`). Values must be whole +milliseconds from 1 to 120000; missing or invalid values use the safe default. + +`frontend/src/lib/request-timeout.ts` exports a reusable +`withRequestTimeout(operation, { signal, timeoutMs })` helper. On deadline, it +rejects with `ApiRequestTimeoutError` and aborts the transport; on caller +cancellation (filter change or component unmount), it raises `AbortError` and +does not display an error. A timed-out operation cannot subsequently resolve +its wrapper, **even when a fetch adapter ignores abort**. The bounded operation +includes parsing, not just obtaining a response header. + +The `useBountyDiscovery` hook retries only when the user clicks **Retry +loading bounties**, preserving the current filters and page. The submission +history page likewise offers **Retry loading history**, retaining the current +connected wallet address. It does not +silently retry POST or other potentially non-idempotent requests. A loading +status is displayed until success or an error, and failed requests show an +accessible message with the Retry button. Query changes cancel the previous +request to stop stale responses from replacing new results. + +To verify locally, run from the `frontend/` directory: + +```bash +pnpm exec vitest run src/lib/request-timeout.test.ts src/components/BountyList.test.tsx +``` + +The test suite exercises a never-resolving request, stalled response parsing, +caller cancellation, timeouts that ignore abort signals, late results, and +success on a manual retry. This pattern can be reused by other **read-only** +API screens that need bounded loading states. diff --git a/frontend/.env.example b/frontend/.env.example index 30023cd..4928105 100644 --- a/frontend/.env.example +++ b/frontend/.env.example @@ -29,6 +29,10 @@ NEXT_PUBLIC_CONTRACT_ID= # Optional app environment label for diagnostics (not a secret). # NODE_ENV is set automatically by Next.js (development | production | test). +# Bounded GET request deadline for bounty discovery and submission history, in ms. +# Default 15000; allowed 1..120000. This is a public, build-time setting. +NEXT_PUBLIC_API_REQUEST_TIMEOUT_MS=15000 + # Set to true to auto-seed the in-memory demo data when the local task API is first read. # This is ignored in production. NEXT_PUBLIC_SEED_DEMO_DATA=false diff --git a/frontend/src/app/(dashboard)/bounties/page.tsx b/frontend/src/app/(dashboard)/bounties/page.tsx index 317c95b..8df40bd 100644 --- a/frontend/src/app/(dashboard)/bounties/page.tsx +++ b/frontend/src/app/(dashboard)/bounties/page.tsx @@ -6,7 +6,7 @@ import { BountyList } from "@/components/BountyList"; import { useBountyDiscovery } from "@/hooks/useBountyDiscovery"; export default function BountiesPage() { - const { tasks, pagination, isLoading, error, filters, setFilters, resetFilters, setPage } = + const { tasks, pagination, isLoading, error, filters, setFilters, resetFilters, setPage, retry } = useBountyDiscovery(); return ( @@ -21,6 +21,7 @@ export default function BountiesPage() { isLoading={isLoading} error={error} onPageChange={setPage} + onRetry={retry} /> ); diff --git a/frontend/src/app/(dashboard)/my-submissions/page.tsx b/frontend/src/app/(dashboard)/my-submissions/page.tsx index c0b9038..32f21a6 100644 --- a/frontend/src/app/(dashboard)/my-submissions/page.tsx +++ b/frontend/src/app/(dashboard)/my-submissions/page.tsx @@ -5,6 +5,7 @@ import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/com import { Badge } from "@/components/ui/badge"; import { ChevronDown, Clock, FileText, Inbox, Link2, MessageSquare } from "lucide-react"; import { getPublicKey } from "@/hooks/stellar-wallets-kit"; +import { withRequestTimeout } from "@/lib/request-timeout"; import type { SubmissionStatus, TaskStatus } from "@/types/task-workflow"; interface SubmissionHistoryRow { @@ -226,6 +227,7 @@ export default function MySubmissionsPage() { const [submissions, setSubmissions] = useState([]); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); + const [retryToken, setRetryToken] = useState(0); const [walletAddress, setWalletAddress] = useState(null); const [statusTab, setStatusTab] = useState("all"); const [expandedId, setExpandedId] = useState(null); @@ -265,15 +267,16 @@ export default function MySubmissionsPage() { try { const params = new URLSearchParams({ contributor: walletAddress! }); - const response = await fetch(`/api/my-submissions?${params.toString()}`, { - signal: controller.signal, - }); - const body = await response.json(); - - if (!response.ok || !body.ok) { - throw new Error(body.error ?? "Failed to load submission history."); - } + const body = await withRequestTimeout(async (signal) => { + const response = await fetch(`/api/my-submissions?${params.toString()}`, { signal }); + const result = await response.json(); + if (!response.ok || !result.ok) { + throw new Error(result.error ?? "Failed to load submission history."); + } + return result; + }, { signal: controller.signal }); + if (controller.signal.aborted) return; setSubmissions(body.submissions); } catch (err) { if (controller.signal.aborted) return; @@ -288,7 +291,7 @@ export default function MySubmissionsPage() { run(); return () => controller.abort(); - }, [walletAddress]); + }, [walletAddress, retryToken]); const counts = useMemo(() => { const byStatus: Record = { @@ -341,7 +344,16 @@ export default function MySubmissionsPage() { {walletAddress && !isLoading && error && ( - {error} + +

{error}

+ + )} diff --git a/frontend/src/components/BountyList.test.tsx b/frontend/src/components/BountyList.test.tsx new file mode 100644 index 0000000..162be9d --- /dev/null +++ b/frontend/src/components/BountyList.test.tsx @@ -0,0 +1,39 @@ +import React from "react"; +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { BountyList } from "./BountyList"; + +const defaults = { + tasks: [], + pagination: { page: 1, pageSize: 10, total: 0, totalPages: 1 }, + onPageChange: vi.fn(), + onRetry: vi.fn(), +}; + +describe("BountyList timeout and retry feedback", () => { + it("exposes a visible, accessible retry action after a request error", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain('role="alert"'); + expect(html).toContain("Request timed out"); + expect(html).toContain("Retry loading bounties"); + expect(html).toContain(" { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain('role="status"'); + expect(html).toContain("Loading bounties"); + }); + + it("does not offer retry for a legitimate empty search result", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("No bounties found"); + expect(html).not.toContain("Retry loading bounties"); + }); +}); diff --git a/frontend/src/components/BountyList.tsx b/frontend/src/components/BountyList.tsx index 622664c..595e4ca 100644 --- a/frontend/src/components/BountyList.tsx +++ b/frontend/src/components/BountyList.tsx @@ -29,14 +29,26 @@ interface BountyListProps { isLoading: boolean; error: string | null; onPageChange: (page: number) => void; + onRetry: () => void; } -export function BountyList({ tasks, pagination, isLoading, error, onPageChange }: BountyListProps) { +export function BountyList({ tasks, pagination, isLoading, error, onPageChange, onRetry }: BountyListProps) { if (error) { return (

Couldn't load bounties

-

{error}

+

{error}

+ +
+ ); + } + + if (isLoading && tasks.length === 0) { + return ( +
+ Loading bounties…
); } diff --git a/frontend/src/hooks/useBountyDiscovery.ts b/frontend/src/hooks/useBountyDiscovery.ts index 51d52ec..9ffcf65 100644 --- a/frontend/src/hooks/useBountyDiscovery.ts +++ b/frontend/src/hooks/useBountyDiscovery.ts @@ -1,6 +1,7 @@ "use client"; import { useEffect, useMemo, useState } from "react"; +import { withRequestTimeout } from "@/lib/request-timeout"; import type { TaskDifficulty, TaskRecord, @@ -51,6 +52,8 @@ export interface UseBountyDiscoveryReturn { setFilters: (partial: Partial) => void; resetFilters: () => void; setPage: (page: number) => void; + /** Re-run the current filter/page query after an error or timeout. */ + retry: () => void; } const SEARCH_DEBOUNCE_MS = 300; @@ -83,6 +86,7 @@ export function useBountyDiscovery(): UseBountyDiscoveryReturn { const [pagination, setPagination] = useState(DEFAULT_PAGINATION); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); + const [retryToken, setRetryToken] = useState(0); // Debounce the free-text search field only; other filters (selects, number // inputs) apply immediately since they don't fire on every keystroke. @@ -111,15 +115,16 @@ export function useBountyDiscovery(): UseBountyDiscoveryReturn { setError(null); try { - const response = await fetch(`/api/tasks?${queryString}`, { - signal: controller.signal, - }); - const body = await response.json(); - - if (!response.ok || !body.ok) { - throw new Error(body.error ?? "Failed to load bounties."); - } + const body = await withRequestTimeout(async (signal) => { + const response = await fetch(`/api/tasks?${queryString}`, { signal }); + const result = await response.json(); + if (!response.ok || !result.ok) { + throw new Error(result.error ?? "Failed to load bounties."); + } + return result; + }, { signal: controller.signal }); + if (controller.signal.aborted) return; setTasks(body.tasks); setPagination(body.pagination); } catch (err) { @@ -135,7 +140,7 @@ export function useBountyDiscovery(): UseBountyDiscoveryReturn { run(); return () => controller.abort(); - }, [queryString]); + }, [queryString, retryToken]); const setFilters = (partial: Partial) => { setFiltersState((prev) => ({ ...prev, ...partial })); @@ -152,6 +157,8 @@ export function useBountyDiscovery(): UseBountyDiscoveryReturn { setPageState(Math.max(1, Math.floor(nextPage))); }; + const retry = () => setRetryToken((current) => current + 1); + return { tasks, pagination, @@ -161,5 +168,6 @@ export function useBountyDiscovery(): UseBountyDiscoveryReturn { setFilters, resetFilters, setPage, + retry, }; } diff --git a/frontend/src/lib/request-timeout.test.ts b/frontend/src/lib/request-timeout.test.ts new file mode 100644 index 0000000..3774fe3 --- /dev/null +++ b/frontend/src/lib/request-timeout.test.ts @@ -0,0 +1,121 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + apiRequestTimeoutMs, + ApiRequestTimeoutError, + DEFAULT_API_TIMEOUT_MS, + withRequestTimeout, +} from "./request-timeout"; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason: Error) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); +}); + +describe("API request timeout configuration", () => { + it("has a safe default and accepts a valid build-time override", () => { + expect(apiRequestTimeoutMs(undefined)).toBe(DEFAULT_API_TIMEOUT_MS); + expect(apiRequestTimeoutMs("5000")).toBe(5000); + expect(apiRequestTimeoutMs("120000")).toBe(120000); + }); + + it.each(["", "0", "-1", "oops", "2.5", "Infinity", "120001", "99999999999999999999"])( + "falls back for invalid value %j", + (value) => expect(apiRequestTimeoutMs(value)).toBe(DEFAULT_API_TIMEOUT_MS), + ); + + it.each([0, -1, 120001, 1.5, Number.NaN])("rejects invalid per-call limit %s", async (timeoutMs) => { + await expect(withRequestTimeout(() => Promise.resolve("ok"), { timeoutMs })).rejects.toThrow(RangeError); + }); +}); + +describe("withRequestTimeout", () => { + it("returns the result of a fast request and removes its timer", async () => { + const operation = vi.fn().mockResolvedValue({ ok: true }); + await expect(withRequestTimeout(operation, { timeoutMs: 250 })).resolves.toEqual({ ok: true }); + expect(operation).toHaveBeenCalledOnce(); + expect(vi.getTimerCount()).toBe(0); + }); + + it("times out a never-settling fetch and aborts its signal", async () => { + let receivedSignal: AbortSignal | undefined; + const pending = withRequestTimeout(async (signal) => { + receivedSignal = signal; + return new Promise(() => {}); + }, { timeoutMs: 500 }); + const asserted = expect(pending).rejects.toBeInstanceOf(ApiRequestTimeoutError); + await vi.advanceTimersByTimeAsync(500); + await asserted; + expect(receivedSignal?.aborted).toBe(true); + expect(vi.getTimerCount()).toBe(0); + }); + + it("bounds stalled JSON parsing as well as a stalled initial fetch", async () => { + const pending = withRequestTimeout(async () => { + const response = { json: () => new Promise(() => {}) }; + return response.json(); + }, { timeoutMs: 75 }); + const asserted = expect(pending).rejects.toThrow(/timed out after 1 seconds/); + await vi.advanceTimersByTimeAsync(75); + await asserted; + }); + + it("ignores a late transport result after the timeout has rejected", async () => { + const lateResponse = deferred(); + const pending = withRequestTimeout(() => lateResponse.promise, { timeoutMs: 50 }); + const asserted = expect(pending).rejects.toBeInstanceOf(ApiRequestTimeoutError); + await vi.advanceTimersByTimeAsync(50); + await asserted; + lateResponse.resolve("too late"); + await Promise.resolve(); + expect(vi.getTimerCount()).toBe(0); + }); + + it("distinguishes caller cancellation from a timeout, even when the operation ignores the signal", async () => { + const caller = new AbortController(); + let requestSignal: AbortSignal | undefined; + const pending = withRequestTimeout((signal) => { + requestSignal = signal; + return new Promise(() => {}); + }, { timeoutMs: 2000, signal: caller.signal }); + const asserted = expect(pending).rejects.toMatchObject({ name: "AbortError" }); + await Promise.resolve(); + caller.abort(); + await asserted; + expect(requestSignal?.aborted).toBe(true); + expect(vi.getTimerCount()).toBe(0); + }); + + it("does not run an operation if the caller was already aborted", async () => { + const caller = new AbortController(); + caller.abort(); + const operation = vi.fn().mockResolvedValue("unexpected"); + await expect(withRequestTimeout(operation, { signal: caller.signal })).rejects.toMatchObject({ name: "AbortError" }); + expect(operation).not.toHaveBeenCalled(); + }); + + it("returns the original server error instead of calling it a timeout", async () => { + const failure = new Error("HTTP 503"); + await expect(withRequestTimeout(async () => { throw failure; }, { timeoutMs: 100 })).rejects.toBe(failure); + expect(vi.getTimerCount()).toBe(0); + }); + + it("keeps a successful retry independent of an earlier timed-out request", async () => { + const never = new Promise(() => {}); + const original = withRequestTimeout(() => never, { timeoutMs: 50 }); + const originalRejected = expect(original).rejects.toBeInstanceOf(ApiRequestTimeoutError); + await vi.advanceTimersByTimeAsync(50); + await originalRejected; + await expect(withRequestTimeout(() => Promise.resolve("retry worked"), { timeoutMs: 50 })).resolves.toBe("retry worked"); + }); +}); diff --git a/frontend/src/lib/request-timeout.ts b/frontend/src/lib/request-timeout.ts new file mode 100644 index 0000000..8034916 --- /dev/null +++ b/frontend/src/lib/request-timeout.ts @@ -0,0 +1,89 @@ +/** + * Bound async API reads (including body parsing), even when a fetch adapter + * ignores AbortSignal. Caller cancellation and timeouts have different errors. + * Never blindly retry writes: this only controls one request's lifetime. + */ +export const DEFAULT_API_TIMEOUT_MS = 15_000; +const MAX_API_TIMEOUT_MS = 120_000; + +/** + * Client-side override at build time, e.g. NEXT_PUBLIC_API_REQUEST_TIMEOUT_MS=10000. + * Reject invalid/missing settings instead of accidentally disabling the limit. + */ +export function apiRequestTimeoutMs( + configured: string | undefined = process.env.NEXT_PUBLIC_API_REQUEST_TIMEOUT_MS, +): number { + if (!configured) return DEFAULT_API_TIMEOUT_MS; + const parsed = Number(configured); + return Number.isSafeInteger(parsed) && parsed > 0 && parsed <= MAX_API_TIMEOUT_MS + ? parsed + : DEFAULT_API_TIMEOUT_MS; +} + +export class ApiRequestTimeoutError extends Error { + constructor(milliseconds: number) { + super(`Request timed out after ${Math.ceil(milliseconds / 1000)} seconds. Please try again.`); + this.name = "ApiRequestTimeoutError"; + } +} + +function abortError(): Error { + const error = new Error("Request was cancelled."); + error.name = "AbortError"; + return error; +} + +export interface RequestTimeoutOptions { + /** Maximum time for the whole operation, including parsing the response. */ + timeoutMs?: number; + /** Optional external signal for route changes or component unmount. */ + signal?: AbortSignal; +} + +/** + * Resolve with the operation's result, or reject on timeout/caller abort. + * Late responses cannot settle this promise after the deadline, even if the + * underlying fetch ignores the abort; the supplied signal cancels real fetches. + */ +export async function withRequestTimeout( + operation: (signal: AbortSignal) => Promise, + { timeoutMs = apiRequestTimeoutMs(), signal }: RequestTimeoutOptions = {}, +): Promise { + if (!Number.isSafeInteger(timeoutMs) || timeoutMs <= 0 || timeoutMs > MAX_API_TIMEOUT_MS) { + throw new RangeError("timeoutMs must be a positive integer at most 120000"); + } + if (signal?.aborted) throw abortError(); + + const controller = new AbortController(); + let deadline: ReturnType | undefined; + let cancelRequest: ((reason: Error) => void) | undefined; + + const cancelled = new Promise((_resolve, reject) => { + cancelRequest = reject; + }); + const onCallerAbort = () => { + cancelRequest?.(abortError()); + controller.abort(); + }; + signal?.addEventListener("abort", onCallerAbort, { once: true }); + + const timedOut = new Promise((_resolve, reject) => { + deadline = setTimeout(() => { + reject(new ApiRequestTimeoutError(timeoutMs)); + controller.abort(); + }, timeoutMs); + }); + + try { + // Deferring the operation ensures cancellation can win if the caller + // aborts between registration and the next microtask. + const running = Promise.resolve().then(() => { + if (controller.signal.aborted) throw abortError(); + return operation(controller.signal); + }); + return await Promise.race([running, timedOut, cancelled]); + } finally { + if (deadline !== undefined) clearTimeout(deadline); + signal?.removeEventListener("abort", onCallerAbort); + } +}