From 961febafe576c5b2005429a6e97a6166de8954c3 Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 4 Aug 2026 18:11:25 -0400 Subject: [PATCH] refactor(errors): type the ?error= codes on /login, /account and /admin/accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #79. #85 did this for the two payout pages and named what it skipped: these three still carried untyped `Record` maps, which is the thing that defeats the type system — it accepts any key, so `keyof typeof` yields `string` and derives nothing. A code with no entry in the destination page's map renders nothing at all. Unlike payouts there was no chokepoint to type: six producers wrote the URL by hand, and two of them pick between two DIFFERENT destination pages in a single ternary. So the helpers had to be introduced before anything could be typed. Shape: typed URL BUILDERS returning a string, not payouts' `: never` redirectors. Two incompatible mechanisms consume these — server actions and page guards call `redirect()` (returns `never`, which `denyAdmin` and `redirectOnMutationError` both depend on for their exhaustiveness over a service error union), while the OAuth callbacks are route handlers that must return a Response. A `: never` helper cannot be returned from a route handler, and a Response-returning one would destroy the `never` the two switches rely on. A string sits underneath both. It also lets one call site pick between two destinations with each branch checked against its own page's union. Home: `src/lib/error-redirects.ts`, not `src/app/`. `src/lib/admin-guard.ts` produces one of these codes, and nothing outside `src/app/` imports `src/app/` anywhere in this repo; colocating would have made admin-guard the first inversion, for a UI string. admin-guard already hardcoded that URL and already imports next/navigation. `src/app/payouts/errors.ts` stays where it is — it has no `src/lib` producer, so colocation is still right there. `lookupErrorMessage` moves to the shared module and payouts re-exports it. `redirectOnMutationError` is now exhaustive on a second, independent axis and its docblock says so: the service union says which failures exist, the code union says which ones the page can explain. `adminAccountsErrorUrl` preserves `?tier=pending` via URLSearchParams, the way `createFailed` does. Non-goal honoured: codes are NOT globally unique. `not_admin` stays in both /account and /admin/accounts with different copy — one is the de-roled admin's destination, the other a stale-tab race — and `session_expired` reaches /login from four unrelated producers. No live bug surfaced: every emitted code already had an entry in its destination map. Copy moved byte-for-byte (extracted map bodies diffed against HEAD: all three IDENTICAL) and every generated URL matches the literal it replaced, `?tier=pending&error=` param order included. Also fixes two stale comment references from #85 naming an `ERRORS` symbol that no longer exists in payouts (dropped.ts, e2e/payouts.spec.ts) — names only. No migration. No change to OAuth state consumption, cookie handling or consumeOauthTransaction; only which URL a failure redirects to. --- e2e/payouts.spec.ts | 7 +- src/app/account/actions.ts | 7 +- src/app/account/page.tsx | 30 ++--- src/app/admin/accounts/actions.ts | 18 ++- src/app/admin/accounts/page.tsx | 22 ++-- src/app/auth/discord/callback/route.ts | 15 ++- src/app/auth/eve/callback/route.ts | 15 +-- src/app/login/page.tsx | 16 +-- src/app/payouts/dropped.ts | 6 +- src/app/payouts/errors.ts | 22 ++-- src/lib/admin-guard.ts | 5 +- src/lib/error-redirects.ts | 145 +++++++++++++++++++++++++ 12 files changed, 217 insertions(+), 91 deletions(-) create mode 100644 src/lib/error-redirects.ts diff --git a/e2e/payouts.spec.ts b/e2e/payouts.spec.ts index 6f2b9422..89277c37 100644 --- a/e2e/payouts.spec.ts +++ b/e2e/payouts.spec.ts @@ -597,9 +597,10 @@ async function bypassClientGuard(input: Locator, value: string): Promise { * percentage that copy is a lie, and the form's contents went with it. Same * conversion `requireAdminAction` already went through (see e2e/admin.spec.ts). * - * A code with no entry in either ERRORS map renders nothing at all, which is - * the one failure these pages cannot show the operator, so each is checked by - * name. `p.notice--bad`, never getByRole("alert"): arriving here from a server + * A code with no entry in either error map (NEW_OPERATION_ERRORS / + * OPERATION_ERRORS) renders nothing at all, which is the one failure these + * pages cannot show the operator, so each is checked by name. `p.notice--bad`, + * never getByRole("alert"): arriving here from a server * action is a soft navigation, so Next's route announcer is populated and also * carries role="alert". */ diff --git a/src/app/account/actions.ts b/src/app/account/actions.ts index 6340480d..ea84de5a 100644 --- a/src/app/account/actions.ts +++ b/src/app/account/actions.ts @@ -7,6 +7,7 @@ import { cookies } from "next/headers"; import { getConfig } from "@/config"; import { getDb } from "@/db"; import { character } from "@/db/schema"; +import { accountErrorUrl, loginErrorUrl } from "@/lib/error-redirects"; import { setMainCharacter, unlinkCharacter, wakeSelf } from "@/services/accounts"; import { getSessionAccount } from "@/services/session"; @@ -18,7 +19,7 @@ async function requireAccount(): Promise { // while this page is still open — the exact "alt-tabbed at 1am" session // PRODUCT.md describes. That is an expected end state, not a bug: send the // member back to sign in instead of throwing to the error boundary for it. - if (!sess) redirect("/login?error=session_expired"); + if (!sess) redirect(loginErrorUrl("session_expired")); return sess.accountId; } @@ -32,7 +33,7 @@ export async function setMainAction(characterId: number): Promise { // reclaim (background token-health job) can pull this character off the // account first. That is a race the member just needs a fresh render for, // not a bug worth an error boundary. - redirect("/account?error=stale_character"); + redirect(accountErrorUrl("stale_character")); } revalidatePath("/account"); } @@ -50,7 +51,7 @@ export async function unlinkAction(characterId: number): Promise { .select() .from(character) .where(and(eq(character.id, characterId), eq(character.accountId, accountId))); - if (owned.length === 0) redirect("/account?error=stale_character"); + if (owned.length === 0) redirect(accountErrorUrl("stale_character")); await db.transaction(async (dbtx) => { // A last_character / not_owned rejection is a silent no-op here: the page // hides the unlink control for the final character, and a reclaim race diff --git a/src/app/account/page.tsx b/src/app/account/page.tsx index 4b1aa434..e1780a52 100644 --- a/src/app/account/page.tsx +++ b/src/app/account/page.tsx @@ -4,6 +4,7 @@ import { redirect } from "next/navigation"; import { cookies } from "next/headers"; import { getConfig } from "@/config"; import { getDb } from "@/db"; +import { ACCOUNT_ERRORS, loginErrorUrl, lookupErrorMessage } from "@/lib/error-redirects"; import { getAccountView, type PushStatus } from "@/services/account-view"; import { canReadPayouts } from "@/services/payouts"; import { listAccountPayouts } from "@/services/payout-view"; @@ -42,27 +43,10 @@ export const metadata: Metadata = { title: "Your account", }; -// Every code here is emitted by a callback route redirect. The distinction the -// copy has to carry is "retry works" (expired/failed) versus "retrying will do -// the same thing" (already_linked). Sign-in links expire 10 minutes after you -// start them (src/services/oauth-tx.ts). -const ERRORS: Record = { - already_linked: - "That character belongs to an account with its own history, so it can't be merged automatically. Ask an admin.", - discord_already_linked: "That Discord account is already linked to another account.", - discord_denied: "Discord authorization was cancelled.", - discord_expired: - "That Discord link expired before it finished. Nothing changed. Start it again below.", - discord_failed: - "Discord couldn't be reached, so the link didn't finish. Nothing changed. Try again.", - link_expired: - "That character link expired before it finished. Nothing changed. Start it again below.", - link_failed: - "EVE couldn't be reached, so the character didn't finish linking. Nothing changed. Try again.", - stale_character: - "That character isn't on this account anymore. The page below is current.", - not_admin: "Your admin access was removed. This is your account page.", -}; +// Every code this page renders lives in src/lib/error-redirects.ts, beside the +// builder its producers go through. Nearly all are emitted by a callback route +// redirect; the distinction the copy has to carry is "retry works" +// (expired/failed) versus "retrying will do the same thing" (already_linked). /** * The id the CONTACTS column header points `aria-describedby` at. The note is @@ -116,10 +100,10 @@ export default async function AccountPage({ // cookie at all is a first-time visitor, who must not be told a session they // never had has ended. `resolveAdmin` draws the same line for the admin // pages. - if (!sess) redirect(sid ? "/login?error=session_expired" : "/login"); + if (!sess) redirect(sid ? loginErrorUrl("session_expired") : "/login"); const view = await getAccountView(getDb(), cfg, sess.accountId); const { error } = await searchParams; - const message = error ? ERRORS[error] : undefined; + const message = lookupErrorMessage(ACCOUNT_ERRORS, error); const now = Date.now(); // Same tier-only gate the payouts pages themselves re-check — this only // decides whether the link appears, never whether the route is reachable. diff --git a/src/app/admin/accounts/actions.ts b/src/app/admin/accounts/actions.ts index deffdee2..eb11a771 100644 --- a/src/app/admin/accounts/actions.ts +++ b/src/app/admin/accounts/actions.ts @@ -4,6 +4,7 @@ import { revalidatePath } from "next/cache"; import { redirect } from "next/navigation"; import { getDb } from "@/db"; import { requireAdminAction } from "@/lib/admin-guard"; +import { adminAccountsErrorUrl } from "@/lib/error-redirects"; import { demoteAdmin, promoteAdmin } from "@/services/accounts"; import { approveAccount, @@ -21,7 +22,7 @@ import { enqueueSync } from "@/services/outbox"; // Redirect to the styled notice rather than throw, same as demoteAdminAction's // `last_admin` case below. function redirectNotAdmin(): never { - redirect("/admin/accounts?error=not_admin"); + redirect(adminAccountsErrorUrl("not_admin")); } /** @@ -46,6 +47,13 @@ function redirectNotAdmin(): never { * the unfiltered list: only approveAction's callers were looking at that * filter when they clicked. `not_pending` always goes there regardless of the * flag, since it can only ever be produced by approveAccount. + * + * Exhaustive on a SECOND axis since the destination URLs moved behind + * `adminAccountsErrorUrl`: the `?error=` codes below are `keyof + * ADMIN_ACCOUNTS_ERRORS`, so a code with no entry in the page's map — which + * would redirect and then render nothing at all — fails typecheck here too. + * The two axes are independent: the service union says which failures exist, + * the code union says which ones the page can explain. */ function redirectOnMutationError( error: "not_authorized" | "not_found" | "not_pending", @@ -57,12 +65,10 @@ function redirectOnMutationError( case "not_pending": // Two admins working the queue, or one with a stale tab: the account is // approved, just not by them. - return redirect("/admin/accounts?tier=pending&error=not_pending"); + return redirect(adminAccountsErrorUrl("not_pending", { tier: "pending" })); case "not_found": return redirect( - opts.fromQueue - ? "/admin/accounts?tier=pending&error=not_found" - : "/admin/accounts?error=not_found", + adminAccountsErrorUrl("not_found", opts.fromQueue ? { tier: "pending" } : {}), ); } } @@ -181,7 +187,7 @@ export async function demoteAdminAction(accountId: string): Promise { const result = await getDb().transaction((tx) => demoteAdmin(tx, actor, accountId)); if (!result.ok && result.error === "last_admin") { // Surface the service's protection instead of a 500 (carry-over). - redirect("/admin/accounts?error=last_admin"); + redirect(adminAccountsErrorUrl("last_admin")); } if (!result.ok && result.error === "not_authorized") redirectNotAdmin(); if (!result.ok) throw new Error(result.error); diff --git a/src/app/admin/accounts/page.tsx b/src/app/admin/accounts/page.tsx index b0e7f4dc..d2d34e33 100644 --- a/src/app/admin/accounts/page.tsx +++ b/src/app/admin/accounts/page.tsx @@ -2,6 +2,7 @@ import type { Metadata } from "next"; import { getConfig, type Config } from "@/config"; import { getDb } from "@/db"; import { requireAdminPage } from "@/lib/admin-guard"; +import { ADMIN_ACCOUNTS_ERRORS, lookupErrorMessage } from "@/lib/error-redirects"; import { isContactsTarget } from "@/services/desired"; import { countAccountsByTier, @@ -54,18 +55,10 @@ const TIERS = ["flygd", "blue", "green"] as const; // have to be findable. Drives the ?tier= whitelist and the filter chips only. const TIER_FILTERS = ["pending", ...TIERS] as const; -const ERRORS: Record = { - last_admin: "Cannot demote the last admin.", - not_admin: - "Your admin access changed since this page loaded. Refresh to see the current state.", - not_pending: - "That account was already approved by someone else. Refresh to see its current tier.", - // Shared by every admin mutation, not just approval (actions.ts): the merge - // feature can delete the row an admin's control targeted between page - // render and click, regardless of which action they clicked. - not_found: - "That account is gone: its character was linked to another account and merged in. There's nothing left to act on.", -}; +// Every code this page renders lives in src/lib/error-redirects.ts, beside the +// builder actions.ts and admin-guard.ts go through. All of them are races +// between two legitimate admins rather than faults, which is why they are +// notices on a refreshed list and not error-boundary throws. // The columns after the sortable ones, in render order. A list rather than a // count because three separate things depend on the table's width — the @@ -125,6 +118,7 @@ export default async function AdminAccountsPage({ // call assembles a full row (five unbounded scans plus per-character work) // for every account just to get thrown away for a length. const pendingCount = await countAccountsByTier(getDb(), "pending"); + const errorMessage = lookupErrorMessage(ADMIN_ACCOUNTS_ERRORS, params.error); const qs = (over: Record) => { const p = new URLSearchParams(); @@ -145,9 +139,7 @@ export default async function AdminAccountsPage({

- {params.error && ERRORS[params.error] && ( - {ERRORS[params.error]} - )} + {errorMessage && {errorMessage}} {params.queued === "account" && ( Sync queued. The worker picks it up within a few seconds. diff --git a/src/app/auth/discord/callback/route.ts b/src/app/auth/discord/callback/route.ts index 8937af8d..746e68e9 100644 --- a/src/app/auth/discord/callback/route.ts +++ b/src/app/auth/discord/callback/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getConfig } from "@/config"; import { getDb } from "@/db"; import { exchangeDiscordCode, fetchDiscordUser } from "@/lib/discord/oauth"; +import { accountErrorUrl, loginErrorUrl } from "@/lib/error-redirects"; import { getRequestAccount } from "@/lib/request-session"; import { DiscordLinkConflictError, linkDiscord } from "@/services/discord-link"; import { consumeOauthTransaction } from "@/services/oauth-tx"; @@ -12,21 +13,23 @@ export async function GET(req: NextRequest) { const to = (path: string) => NextResponse.redirect(new URL(path, cfg.appBaseUrl)); // Provider denial (user declined the authorization): error param, no code - if (req.nextUrl.searchParams.get("error")) return to("/account?error=discord_denied"); + if (req.nextUrl.searchParams.get("error")) return to(accountErrorUrl("discord_denied")); const code = req.nextUrl.searchParams.get("code"); const state = req.nextUrl.searchParams.get("state"); // Only reachable from the account page, so /account is right for every // failure here except a missing session. - if (!code || !state) return to("/account?error=discord_failed"); + if (!code || !state) return to(accountErrorUrl("discord_failed")); const tx = await consumeOauthTransaction(db, state, ["link-discord"]); - if (!tx) return to("/account?error=discord_expired"); + if (!tx) return to(accountErrorUrl("discord_expired")); const sess = await getRequestAccount(req); if (!sess || sess.sessionId !== tx.sessionId || sess.accountId !== tx.accountId) { // The transaction is consumed above, so neither destination can be replayed. - return to(sess ? "/account?error=discord_expired" : "/login?error=session_expired"); + return to( + sess ? accountErrorUrl("discord_expired") : loginErrorUrl("session_expired"), + ); } // Route handlers are not covered by app/error.tsx, so an uncaught throw from @@ -44,12 +47,12 @@ export async function GET(req: NextRequest) { if (err instanceof DiscordLinkConflictError) ok = false; else throw err; } - return to(ok ? "/account" : "/account?error=discord_already_linked"); + return to(ok ? "/account" : accountErrorUrl("discord_already_linked")); } catch (err) { // Message only, for the reason spelled out in the EVE callback: a Postgres // error object can carry the failing query and its parameters alongside the // message, and those rows hold token material. console.error("discord callback failed", err instanceof Error ? err.message : err); - return to("/account?error=discord_failed"); + return to(accountErrorUrl("discord_failed")); } } diff --git a/src/app/auth/eve/callback/route.ts b/src/app/auth/eve/callback/route.ts index 79b8075d..63acace0 100644 --- a/src/app/auth/eve/callback/route.ts +++ b/src/app/auth/eve/callback/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getConfig } from "@/config"; import { getDb } from "@/db"; import { exchangeEveCode, verifyEveAccessToken } from "@/lib/esi/sso"; +import { accountErrorUrl, loginErrorUrl } from "@/lib/error-redirects"; import { getRequestAccount } from "@/lib/request-session"; import { sessionCookieAttrs } from "@/lib/session-cookie"; import { @@ -18,19 +19,19 @@ export async function GET(req: NextRequest) { const to = (path: string) => NextResponse.redirect(new URL(path, cfg.appBaseUrl)); // Provider denial (e.g. user clicked "cancel"): no code arrives, just error= - if (req.nextUrl.searchParams.get("error")) return to("/login?error=oauth_denied"); + if (req.nextUrl.searchParams.get("error")) return to(loginErrorUrl("oauth_denied")); const code = req.nextUrl.searchParams.get("code"); const state = req.nextUrl.searchParams.get("state"); // Without state there is no transaction, so nothing tells us whether this was // a login or a character link. /login is the only destination we can be sure // is correct for either. - if (!code || !state) return to("/login?error=oauth_failed"); + if (!code || !state) return to(loginErrorUrl("oauth_failed")); // Only EVE intents are consumable here; a link-discord transaction is // rejected WITHOUT being consumed. All binding checks run before any EVE call. const tx = await consumeOauthTransaction(db, state, ["login", "link-character"]); - if (!tx) return to("/login?error=oauth_expired"); + if (!tx) return to(loginErrorUrl("oauth_expired")); const sess = await getRequestAccount(req); if ( @@ -41,7 +42,7 @@ export async function GET(req: NextRequest) { // replayed. Signed in but holding someone else's (or a stale) transaction // means retrying from the account page; no session at all means the session // is the thing that's missing. - return to(sess ? "/account?error=link_expired" : "/login?error=session_expired"); + return to(sess ? accountErrorUrl("link_expired") : loginErrorUrl("session_expired")); } // Everything past here talks to EVE or the database, and route handlers are @@ -62,7 +63,7 @@ export async function GET(req: NextRequest) { const result = await db.transaction((dbtx) => linkCharacter(dbtx, cfg, sess!.accountId, ch), ); - return to(result.ok ? "/account" : "/account?error=already_linked"); + return to(result.ok ? "/account" : accountErrorUrl("already_linked")); } const { accountId } = await db.transaction((dbtx) => handleEveLogin(dbtx, cfg, ch)); @@ -82,8 +83,8 @@ export async function GET(req: NextRequest) { console.error("eve callback failed", err instanceof Error ? err.message : err); return to( tx.intent === "link-character" - ? "/account?error=link_failed" - : "/login?error=oauth_failed", + ? accountErrorUrl("link_failed") + : loginErrorUrl("oauth_failed"), ); } } diff --git a/src/app/login/page.tsx b/src/app/login/page.tsx index 5507eacd..609cbd92 100644 --- a/src/app/login/page.tsx +++ b/src/app/login/page.tsx @@ -1,6 +1,7 @@ import type { Metadata } from "next"; import { Notice } from "@/app/_components/ui"; import { getConfig } from "@/config"; +import { LOGIN_ERRORS, lookupErrorMessage } from "@/lib/error-redirects"; // `await searchParams` below already forces dynamic rendering, so the // getConfig() call cannot run at build time today. Declared anyway, matching @@ -15,16 +16,9 @@ export const metadata: Metadata = { // Codes reaching this page: oauth_denied / oauth_expired / oauth_failed from // the EVE callback, session_expired from account/actions.ts, account/page.tsx, -// and either callback when the session is gone mid-link. Sign-in links expire -// 10 minutes after you start them (src/services/oauth-tx.ts). -const ERRORS: Record = { - oauth_denied: "Nothing changed. No access was granted. Sign in whenever you're ready.", - oauth_expired: - "That sign-in link expired before you finished. They last 10 minutes. Start again below.", - oauth_failed: - "EVE couldn't be reached, so sign-in didn't finish. Nothing changed. Try again.", - session_expired: "Your session ended. Sign in again to pick up where you left off.", -}; +// admin-guard.ts, and either callback when the session is gone mid-link. The +// map itself lives in src/lib/error-redirects.ts, beside the builder every one +// of those producers now goes through — see that file for why. export default async function LoginPage({ searchParams, @@ -32,7 +26,7 @@ export default async function LoginPage({ searchParams: Promise<{ error?: string }>; }) { const { error } = await searchParams; - const message = error ? ERRORS[error] : undefined; + const message = lookupErrorMessage(LOGIN_ERRORS, error); const cfg = getConfig(); const scopes = cfg.eveSso.scopes; const label = cfg.standings.label; diff --git a/src/app/payouts/dropped.ts b/src/app/payouts/dropped.ts index f846482b..5438d2df 100644 --- a/src/app/payouts/dropped.ts +++ b/src/app/payouts/dropped.ts @@ -52,9 +52,9 @@ export function encodeDropped(dropped: DroppedLootLine[]): string { /** * Null for anything this page cannot faithfully render — an absent param, a - * hand-typed one, a truncated one. Same rule the `ERRORS` map follows for an - * unrecognized `?error=` code: degrade to the plain page, never to an empty - * or half-filled notice. + * hand-typed one, a truncated one. Same rule the `OPERATION_ERRORS` map follows + * for an unrecognized `?error=` code: degrade to the plain page, never to an + * empty or half-filled notice. */ export function decodeDropped(raw: string | undefined): DroppedReport | null { if (!raw) return null; diff --git a/src/app/payouts/errors.ts b/src/app/payouts/errors.ts index 25c466e4..7e03d7ff 100644 --- a/src/app/payouts/errors.ts +++ b/src/app/payouts/errors.ts @@ -101,15 +101,13 @@ export const OPERATION_ERRORS = { export type NewOperationErrorCode = keyof typeof NEW_OPERATION_ERRORS; export type OperationErrorCode = keyof typeof OPERATION_ERRORS; -/** Reads a code that came off the query string, where it is a `string` and not - * yet one of ours — anyone can type `?error=nonsense`. An unrecognized code - * yields no message and the page renders without a notice, exactly as it does - * today (`src/app/payouts/dropped.ts` takes the same stance on its payload). - * What changed is that a code WE emit can no longer be the unrecognized one: - * `operationFailed` and `createFailed` only accept keys of these maps. */ -export function lookupErrorMessage( - map: Readonly>, - code: string | undefined, -): string | undefined { - return code !== undefined && Object.hasOwn(map, code) ? map[code] : undefined; -} +/** Re-exported so the payout pages keep importing it from beside their own + * maps. It lives in `src/lib/error-redirects.ts` because `/login`, `/account` + * and `/admin/accounts` need the same reader and one of their producers is + * itself under `src/lib/` — see that file for why the codes could not follow + * the payouts precedent and colocate. An unrecognized code still yields no + * message and the page renders without a notice, exactly as it does today + * (`src/app/payouts/dropped.ts` takes the same stance on its payload); what + * the types add is that a code WE emit can no longer be the unrecognized one, + * since `operationFailed` and `createFailed` only accept keys of these maps. */ +export { lookupErrorMessage } from "@/lib/error-redirects"; diff --git a/src/lib/admin-guard.ts b/src/lib/admin-guard.ts index a7a42d71..f0f79877 100644 --- a/src/lib/admin-guard.ts +++ b/src/lib/admin-guard.ts @@ -4,6 +4,7 @@ import { redirect } from "next/navigation"; import { getConfig } from "@/config"; import { getDb, type Db } from "@/db"; import { account } from "@/db/schema"; +import { accountErrorUrl, loginErrorUrl } from "@/lib/error-redirects"; import { getSessionAccount } from "@/services/session"; export type AdminContext = { accountId: string }; @@ -78,9 +79,9 @@ function denyAdmin(reason: AdminDenial): never { // second hop: requireAdminPage re-guards `/admin/accounts` itself, so a // non-admin who lands there bounces onward to this same `/account` // redirect. - return redirect("/account?error=not_admin"); + return redirect(accountErrorUrl("not_admin")); case "session-expired": - return redirect("/login?error=session_expired"); + return redirect(loginErrorUrl("session_expired")); case "no-session": // Never signed in — most often someone who guessed /admin, or a crawler. // "Your session ended" would name an event that never happened, so this diff --git a/src/lib/error-redirects.ts b/src/lib/error-redirects.ts new file mode 100644 index 00000000..47ee9f0b --- /dev/null +++ b/src/lib/error-redirects.ts @@ -0,0 +1,145 @@ +/** + * The `?error=` codes `/login`, `/account` and `/admin/accounts` can render, + * the copy each one shows, and the typed URL builders that produce them. + * + * Same invariant `src/app/payouts/errors.ts` holds for the payout pages: a code + * with no entry in the destination page's map renders nothing at all — the page + * loads unchanged with no explanation, which is the one failure these pages + * cannot show a member. Every producer now goes through a builder typed to its + * destination's own union, so a bad code is a typecheck failure rather than a + * deploy. + * + * WHY `src/lib/` AND NOT `src/app/`. Payouts could colocate because every + * producer was already under `src/app/`. These cannot: `src/lib/admin-guard.ts` + * redirects to `/account?error=not_admin`, and nothing outside `src/app/` + * imports `src/app/` anywhere in this repo. Putting these under `src/app/` + * would make admin-guard the first inversion, for a UI string. admin-guard + * already hardcodes that exact URL and already imports `next/navigation`; this + * module only names what was there, and itself imports nothing. + * + * WHY BUILDERS RETURNING A STRING, and not payouts' `: never` redirectors. + * Two incompatible mechanisms consume these. Server actions and page guards + * call `redirect()` from `next/navigation`, which returns `never` — and + * `denyAdmin` and `redirectOnMutationError` both depend on that `never` for + * their exhaustiveness over a service error union. The OAuth callbacks are + * route handlers that must *return a Response*, built by a local `to()` + * wrapping `NextResponse.redirect`. A `: never` helper cannot be returned from + * a route handler, and a Response-returning one would destroy the `never` the + * two switches rely on. A string sits underneath both, and neither loses + * anything. It also lets a single call site pick between two DESTINATIONS — + * `to(sess ? accountErrorUrl("link_expired") : loginErrorUrl("session_expired"))` + * — with each branch checked against its own page's union. A per-file helper + * could not: the code and its destination have to be typed together. + * + * THREE MAPS, NOT ONE, and codes are deliberately NOT globally unique. + * `not_admin` appears in both `/account` and `/admin/accounts` with different + * copy, and both are correct for where they land: `/account` is where a + * genuinely de-roled admin is sent ("your admin access was removed"), while + * `/admin/accounts` is reached by a stale tab whose actor lost the bit between + * render and click ("refresh to see the current state"). One message would be + * wrong on one of them, and `not_admin_account` / `not_admin_admin` would be + * uniqueness as bookkeeping with no property gained. `session_expired` likewise + * reaches `/login` from four unrelated producers. Each page's map is its + * namespace; the types are what make that namespace enforceable. + */ + +/** Codes reaching `/login`. `oauth_*` come from the EVE callback; + * `session_expired` from account/actions.ts, account/page.tsx, admin-guard.ts + * and either callback when the session is gone mid-link. Sign-in links expire + * 10 minutes after you start them (src/services/oauth-tx.ts). */ +export const LOGIN_ERRORS = { + oauth_denied: "Nothing changed. No access was granted. Sign in whenever you're ready.", + oauth_expired: + "That sign-in link expired before you finished. They last 10 minutes. Start again below.", + oauth_failed: + "EVE couldn't be reached, so sign-in didn't finish. Nothing changed. Try again.", + session_expired: "Your session ended. Sign in again to pick up where you left off.", +} as const; + +/** Codes reaching `/account`. All but `not_admin` and `stale_character` are + * emitted by a callback route redirect. The distinction the copy has to carry + * is "retry works" (expired/failed) versus "retrying will do the same thing" + * (already_linked). Sign-in links expire 10 minutes after you start them + * (src/services/oauth-tx.ts). */ +export const ACCOUNT_ERRORS = { + already_linked: + "That character belongs to an account with its own history, so it can't be merged automatically. Ask an admin.", + discord_already_linked: "That Discord account is already linked to another account.", + discord_denied: "Discord authorization was cancelled.", + discord_expired: + "That Discord link expired before it finished. Nothing changed. Start it again below.", + discord_failed: + "Discord couldn't be reached, so the link didn't finish. Nothing changed. Try again.", + link_expired: + "That character link expired before it finished. Nothing changed. Start it again below.", + link_failed: + "EVE couldn't be reached, so the character didn't finish linking. Nothing changed. Try again.", + stale_character: + "That character isn't on this account anymore. The page below is current.", + not_admin: "Your admin access was removed. This is your account page.", +} as const; + +/** Codes reaching `/admin/accounts`. Every one of these is a race between two + * legitimate admins rather than a fault, which is why they are notices on a + * refreshed list and not error-boundary throws. */ +export const ADMIN_ACCOUNTS_ERRORS = { + last_admin: "Cannot demote the last admin.", + not_admin: + "Your admin access changed since this page loaded. Refresh to see the current state.", + not_pending: + "That account was already approved by someone else. Refresh to see its current tier.", + // Shared by every admin mutation, not just approval (actions.ts): the merge + // feature can delete the row an admin's control targeted between page + // render and click, regardless of which action they clicked. + not_found: + "That account is gone: its character was linked to another account and merged in. There's nothing left to act on.", +} as const; + +export type LoginErrorCode = keyof typeof LOGIN_ERRORS; +export type AccountErrorCode = keyof typeof ACCOUNT_ERRORS; +export type AdminAccountsErrorCode = keyof typeof ADMIN_ACCOUNTS_ERRORS; + +/** `/login?error=`. */ +export function loginErrorUrl(code: LoginErrorCode): string { + return `/login?error=${code}`; +} + +/** `/account?error=`. */ +export function accountErrorUrl(code: AccountErrorCode): string { + return `/account?error=${code}`; +} + +/** + * `/admin/accounts?[tier=…&]error=`. + * + * The only one of the three that carries a second param: `not_pending` and a + * `not_found` raised from the approval queue both send the admin back to the + * pending FILTER they were working, not the unfiltered list. Built with + * `URLSearchParams` rather than concatenated — same reason `createFailed` does + * (src/app/payouts/actions.ts) — so the extra param is preserved by + * construction instead of by remembering to include it. `tier` is emitted + * before `error`, matching the URLs these call sites wrote by hand. + */ +export function adminAccountsErrorUrl( + code: AdminAccountsErrorCode, + params: { tier?: "pending" } = {}, +): string { + const search = new URLSearchParams(); + if (params.tier) search.set("tier", params.tier); + search.set("error", code); + return `/admin/accounts?${search.toString()}`; +} + +/** Reads a code that came off the query string, where it is a `string` and not + * yet one of ours — anyone can type `?error=nonsense`. An unrecognized code + * yields no message and the page renders without a notice, exactly as it did + * before these maps were typed. What changed is that a code WE emit can no + * longer be the unrecognized one: the builders above only accept keys of these + * maps. Shared with `src/app/payouts/errors.ts`, which holds the same + * invariant for the payout pages. */ +export function lookupErrorMessage( + map: Readonly>, + code: string | undefined, +): string | undefined { + return code !== undefined && Object.hasOwn(map, code) ? map[code] : undefined; +}