rbrooks/Quest-Board

Watch

Fork 0

feat(frontend): shared API client with session-expiry, abort and dedup (#103) #181

Merged

claude-bot merged 1 commit from feat/103-shared-api-client into main

2026-07-17 02:10:10 +00:00

claude-bot commented

2026-07-17 01:32:52 +00:00

Contributor

Fixes #103. Second of the v3.5.0 (Frontend Platform & PWA) milestone, after #180.

Why

request() was copy-pasted across campaigns.js, sessions.js, users.js, planning.js, with more variants inlined in auth.js. That duplication was the visible problem; the invisible ones were worse:

No global 401 handling. Auth was checked once at mount, so when the 8h session expired mid-use, every request failed while AuthGuard carried on rendering a dead page. The user was stuck rather than sent to login.
No AbortController anywhere. A slow response from a route you'd already left could resolve late and overwrite fresher state.
The dashboard fired one uncached next-session request per campaign card, every mount.

What changed

New src/api/client.js; every src/api/* module routes through it.

Session expiry — a 401 on an authenticated request fires a handler registered by AuthProvider (clear user → redirect /login). The client exposes a registration callback rather than importing auth state, because the api modules it serves are themselves imported by useAuth — importing back would be circular.
Abort — callers pass an AbortSignal; Dashboard, CampaignDetail and WikiArticle abort in effect cleanup (8 effects). Every fetching effect swallows AbortError instead of treating navigation as a failure.
Dedup + short-TTL GET cache — hand-rolled as specified. No React Query.

Two subtleties worth reviewing

1. Abort and dedup interact badly if done naively. When two callers share one in-flight request, letting either one's signal abort the underlying fetch breaks the other caller. Subscribers are refcounted instead: your signal rejects your promise only, and the shared fetch is aborted just when the last subscriber leaves. Pinned by a test ("one subscriber aborting does not break another sharing the request").

2. A blanket TTL cache would introduce a bug the old code couldn't have. Create a campaign → return to the dashboard inside the window → your new campaign is missing (today every mount refetches). So any successful mutation clears the cached GETs. Blunt, but it beats shipping a regression. Also pinned by a test.

Two endpoints must NOT get blanket 401 treatment

Both opt out explicitly — these were easy to get wrong:

/api/me answers 401 simply because nobody is logged in yet. Firing the session-expiry handler there would redirect-loop every anonymous visitor. → handleUnauthorized: false
/public/analytics/<token> is a genuinely public share link fetched without credentials, rendered for logged-out viewers. → auth: false

There's a test for each.

Acceptance criteria

Test: a mocked expired session (401) clears auth and redirects to login — src/hooks/useAuth.test.jsx
Test with delayed mocks: rapid route flips do not apply stale data — "a stale route's abort does not clobber the newer route's data"
Dashboard issues one request per unique campaign per TTL window, verified with mock call counts — Dashboard.test.jsx drives the real client so the cache genuinely runs: fetchNextSession is called 4× (2 per mount, 2 mounts) but only 2 HTTP requests are made
All src/api/* import the shared client; grep "async function request" in src/api/ returns nothing (and no fetch( remains outside client.js)
Error messages still surface the backend detail string

Verification

Run in node:20, matching CI:

npm run lint → exit 0, no output (exhaustive-deps is genuinely enforced now, thanks to #21)
npx vitest run → 97/97 passing across 13 files (was 69; +25 client tests, +2 useAuth, +1 dashboard dedup). No regressions.
npx vite build → succeeds

Entry chunk is currently 685.50 kB (172.36 kB gzip) — that's the baseline for #105 (route-level code splitting), next in this milestone.

🤖 Generated with Claude Code

Fixes #103. Second of the v3.5.0 (Frontend Platform & PWA) milestone, after #180. ## Why `request()` was copy-pasted across `campaigns.js`, `sessions.js`, `users.js`, `planning.js`, with more variants inlined in `auth.js`. That duplication was the visible problem; the invisible ones were worse: - **No global 401 handling.** Auth was checked once at mount, so when the 8h session expired mid-use, every request failed while `AuthGuard` carried on rendering a **dead page**. The user was stuck rather than sent to login. - **No `AbortController` anywhere.** A slow response from a route you'd already left could resolve late and overwrite fresher state. - **The dashboard fired one uncached next-session request per campaign card, every mount.** ## What changed New `src/api/client.js`; every `src/api/*` module routes through it. - **Session expiry** — a 401 on an authenticated request fires a handler registered by `AuthProvider` (clear user → redirect `/login`). The client exposes a *registration callback* rather than importing auth state, because the api modules it serves are themselves imported by `useAuth` — importing back would be circular. - **Abort** — callers pass an `AbortSignal`; Dashboard, CampaignDetail and WikiArticle abort in effect cleanup (8 effects). Every fetching effect swallows `AbortError` instead of treating navigation as a failure. - **Dedup + short-TTL GET cache** — hand-rolled as specified. No React Query. ## Two subtleties worth reviewing **1. Abort and dedup interact badly if done naively.** When two callers share one in-flight request, letting either one's signal abort the underlying fetch **breaks the other caller**. Subscribers are refcounted instead: your signal rejects *your* promise only, and the shared fetch is aborted just when the last subscriber leaves. Pinned by a test (*"one subscriber aborting does not break another sharing the request"*). **2. A blanket TTL cache would introduce a bug the old code couldn't have.** Create a campaign → return to the dashboard inside the window → your new campaign is missing (today every mount refetches). So **any successful mutation clears the cached GETs**. Blunt, but it beats shipping a regression. Also pinned by a test. ## Two endpoints must NOT get blanket 401 treatment Both opt out explicitly — these were easy to get wrong: - **`/api/me`** answers 401 simply because nobody is logged in yet. Firing the session-expiry handler there would **redirect-loop every anonymous visitor**. → `handleUnauthorized: false` - **`/public/analytics/<token>`** is a genuinely public share link fetched **without credentials**, rendered for logged-out viewers. → `auth: false` There's a test for each. ## Acceptance criteria - [x] Test: a mocked expired session (401) clears auth and redirects to login — `src/hooks/useAuth.test.jsx` - [x] Test with delayed mocks: rapid route flips do not apply stale data — *"a stale route's abort does not clobber the newer route's data"* - [x] Dashboard issues one request per unique campaign per TTL window, verified with mock call counts — `Dashboard.test.jsx` drives the **real** client so the cache genuinely runs: `fetchNextSession` is called 4× (2 per mount, 2 mounts) but only **2** HTTP requests are made - [x] All `src/api/*` import the shared client; `grep "async function request"` in `src/api/` returns **nothing** (and no `fetch(` remains outside `client.js`) - [x] Error messages still surface the backend `detail` string ## Verification Run in **node:20**, matching CI: - `npm run lint` → **exit 0, no output** (`exhaustive-deps` is genuinely enforced now, thanks to #21) - `npx vitest run` → **97/97 passing** across 13 files (was 69; +25 client tests, +2 useAuth, +1 dashboard dedup). No regressions. - `npx vite build` → succeeds Entry chunk is currently **685.50 kB (172.36 kB gzip)** — that's the baseline for **#105 (route-level code splitting)**, next in this milestone. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-07-17 01:32:53 +00:00

feat(frontend): shared API client with session-expiry, abort and dedup (#103 )

CI / Frontend tests, audit, and build (pull_request) Successful in 1m20s

Details

CI / Docker image build (pull_request) Successful in 25s

Details

CI / Backend lint (ruff) (pull_request) Successful in 1m9s

Details

CI / Bot tests and audit (pull_request) Successful in 9m30s

Details

CI / Backend migration, tests, and audit (pull_request) Successful in 13m3s

Details

b0ea1713ca

The `request()` fetch wrapper was copy-pasted across campaigns.js, sessions.js,
users.js and planning.js, with more variants inlined in auth.js. Nothing handled
401 globally: auth was checked once at mount, so when the 8h session expired
mid-use every request failed while AuthGuard happily kept rendering a dead page.
There was no AbortController anywhere, so a slow response from a route the user
had already left could resolve late and overwrite fresher state. And the
dashboard fired one uncached next-session request per campaign card, every
mount.

Add src/api/client.js and route every api module through it:

- Session expiry: a 401 on an authenticated request fires a handler registered
  by AuthProvider, which clears the user and redirects to /login. The client
  exposes a registration callback rather than importing auth state, because the
  api modules it serves are themselves imported by useAuth.
- Abort: callers pass an AbortSignal; the three heaviest pages (Dashboard,
  CampaignDetail, WikiArticle) abort in effect cleanup. Every fetching effect
  swallows AbortError rather than treating navigation as a failure.
- Dedup + short-TTL GET cache, hand-rolled as specified (no React Query).

Two subtleties worth calling out:

Abort and dedup interact badly if done naively. When two callers share one
in-flight request, letting either one's signal abort the underlying fetch breaks
the other. Subscribers are refcounted instead: a caller's signal rejects only
that caller's promise, and the shared fetch is aborted just when the last
subscriber leaves.

A blanket TTL cache would have introduced a bug the old code couldn't have:
create a campaign, return to the dashboard inside the window, and the new
campaign is missing. Any successful mutation therefore clears the cached GETs.

Two endpoints must not get blanket 401 treatment, and opt out explicitly:
/api/me answers 401 simply because nobody is logged in yet (redirecting on that
would loop every anonymous visitor), and /public/analytics/<token> is a genuinely
public share link fetched without credentials.

Verified in node:20: lint clean, 97/97 tests (25 new client tests plus
session-expiry and dashboard-dedup acceptance tests), build succeeds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

claude-bot merged commit 4328226039 into main

2026-07-17 02:10:10 +00:00

claude-bot deleted branch feat/103-shared-api-client

2026-07-17 02:10:10 +00:00

claude-bot referenced this pull request from a commit

2026-07-17 02:10:12 +00:00

Merge pull request 'feat(frontend): shared API client with session-expiry, abort and dedup (#103)' (#181) from feat/103-shared-api-client into main

claude-bot referenced this pull request

2026-07-17 02:17:44 +00:00

feat(frontend): route-level code splitting (#105) #183

claude-bot referenced this pull request

2026-07-17 02:38:21 +00:00