diff --git a/.cursor/rules/api-routes.mdc b/.cursor/rules/api-routes.mdc index 79d428c..42c9f31 100644 --- a/.cursor/rules/api-routes.mdc +++ b/.cursor/rules/api-routes.mdc @@ -45,9 +45,19 @@ Keep new routes within this shape so auth, config, and validation stay uniform. 4. **Prisma access** via `import { prisma } from "lib/server/db"`. Do not instantiate `PrismaClient` directly. -5. **Responses** via `NextResponse.json(...)`. Shared shapes (`dbUnavailable`) - live in `lib/server/responses.ts`; add new shared responses there when a - pattern repeats in two routes. +5. **Responses** via `NextResponse.json(...)`. Shared shapes + (`dbUnavailable`, `unauthorized`, `notFound`, `rateLimited`, + `serverMisconfigured`, `internalError`) and the generic `errorJson(code, + message, status, opts?)` live in `lib/server/responses.ts`. Add new + shared responses there when a pattern repeats in two routes. + +6. **Errors + observability.** All 4xx/5xx bodies use the canonical shape + `{ error: { code, message }, details? }` with codes from the + `ApiErrorCode` union in `lib/server/responses.ts`. Wrap handlers with + `apiRoute("scope.name", async (req, ctx, { requestId }) => { ... })` + from `lib/server/apiRoute.ts` so an `x-request-id` is generated / + forwarded onto every response and uncaught throws return a canonical + 500 with the id logged via `lib/logger`. # Server-only isolation @@ -68,9 +78,8 @@ instead of introducing new patterns: - **Rate limiting.** `lib/server/rateLimit.ts` is an in-memory stopgap marked for replacement. Reuse `rateLimitKey()` where limiting is needed; don't - design a new limiter. -- **Error response shape.** Currently `{ error: string }` + HTTP status. No - error codes yet — don't add a taxonomy until one is designed. + design a new limiter. When returning 429, prefer `rateLimited(retryAfterMs)` + from `responses.ts` so the body and `Retry-After` header stay uniform. - **Pagination / filtering.** Only `rules/route.ts` paginates (`take` capped at 100). Mirror it if you add list endpoints; don't invent cursors or offset contracts unilaterally. diff --git a/app/api/auth/logout/route.ts b/app/api/auth/logout/route.ts index 2602fb9..dfa2936 100644 --- a/app/api/auth/logout/route.ts +++ b/app/api/auth/logout/route.ts @@ -2,12 +2,13 @@ import { NextResponse } from "next/server"; import { isDatabaseConfigured } from "../../../../lib/server/env"; import { dbUnavailable } from "../../../../lib/server/responses"; import { destroySessionFromRequest } from "../../../../lib/server/session"; +import { apiRoute } from "../../../../lib/server/apiRoute"; -export async function POST() { +export const POST = apiRoute("auth.logout", async () => { if (!isDatabaseConfigured()) { return dbUnavailable(); } await destroySessionFromRequest(); return NextResponse.json({ ok: true }); -} +}); diff --git a/app/api/auth/magic-link/request/route.ts b/app/api/auth/magic-link/request/route.ts index 067f2b6..c91a166 100644 --- a/app/api/auth/magic-link/request/route.ts +++ b/app/api/auth/magic-link/request/route.ts @@ -10,13 +10,20 @@ import { } from "../../../../../lib/server/hash"; import { sendMagicLinkEmail } from "../../../../../lib/server/mail"; import { rateLimitKey } from "../../../../../lib/server/rateLimit"; -import { dbUnavailable } from "../../../../../lib/server/responses"; -import { logger } from "../../../../../lib/logger"; +import { + dbUnavailable, + errorJson, + rateLimited, + serverMisconfigured, +} from "../../../../../lib/server/responses"; +import { logRouteError } from "../../../../../lib/server/requestId"; +import { apiRoute } from "../../../../../lib/server/apiRoute"; import { safeInternalPath } from "../../../../../lib/safeInternalPath"; const MAGIC_LINK_TTL_MS = 15 * 60 * 1000; const EMAIL_MIN_INTERVAL_MS = 60 * 1000; const IP_MIN_INTERVAL_MS = 20 * 1000; +const SCOPE = "auth.magicLink.request"; function normalizeEmail(raw: unknown): string | null { if (typeof raw !== "string") return null; @@ -32,7 +39,7 @@ function readNextPath(body: unknown): string | null { return safeInternalPath(n); } -export async function POST(request: NextRequest) { +export const POST = apiRoute(SCOPE, async (request: NextRequest, _ctx, { requestId }) => { if (!isDatabaseConfigured()) { return dbUnavailable(); } @@ -41,7 +48,7 @@ export async function POST(request: NextRequest) { try { body = await request.json(); } catch { - return NextResponse.json({ error: "Invalid JSON" }, { status: 400 }); + return errorJson("invalid_json", "Invalid JSON", 400); } const email = normalizeEmail( @@ -50,10 +57,7 @@ export async function POST(request: NextRequest) { : null, ); if (!email) { - return NextResponse.json( - { error: "Valid email required" }, - { status: 400 }, - ); + return errorJson("validation_error", "Valid email required", 400); } const ip = @@ -63,28 +67,19 @@ export async function POST(request: NextRequest) { const rlEmail = rateLimitKey(`magic-email:${email}`, EMAIL_MIN_INTERVAL_MS); if (rlEmail.ok === false) { - return NextResponse.json( - { error: "Too many requests", retryAfterMs: rlEmail.retryAfterMs }, - { status: 429 }, - ); + return rateLimited(rlEmail.retryAfterMs); } const rlIp = rateLimitKey(`magic-ip:${ip}`, IP_MIN_INTERVAL_MS); if (rlIp.ok === false) { - return NextResponse.json( - { error: "Too many requests", retryAfterMs: rlIp.retryAfterMs }, - { status: 429 }, - ); + return rateLimited(rlIp.retryAfterMs); } let pepper: string; try { pepper = getSessionPepper(); } catch { - return NextResponse.json( - { error: "Server misconfiguration" }, - { status: 500 }, - ); + return serverMisconfigured(); } const token = newSessionToken(); @@ -108,13 +103,10 @@ export async function POST(request: NextRequest) { try { await sendMagicLinkEmail(email, verifyUrl); } catch (err) { - logger.error("sendMagicLinkEmail failed:", err); + logRouteError(SCOPE, requestId, err, { phase: "sendMagicLinkEmail", email }); await prisma.magicLinkToken.deleteMany({ where: { email } }); - return NextResponse.json( - { error: "Could not send email" }, - { status: 502 }, - ); + return errorJson("mail_failed", "Could not send email", 502); } return NextResponse.json({ ok: true }); -} +}); diff --git a/app/api/auth/magic-link/verify/route.ts b/app/api/auth/magic-link/verify/route.ts index a8e2261..02f24cd 100644 --- a/app/api/auth/magic-link/verify/route.ts +++ b/app/api/auth/magic-link/verify/route.ts @@ -10,52 +10,83 @@ import { setSessionCookie, } from "../../../../../lib/server/session"; import { dbUnavailable } from "../../../../../lib/server/responses"; +import { + REQUEST_ID_HEADER, + getOrCreateRequestId, + logRouteError, +} from "../../../../../lib/server/requestId"; import { safeInternalPath } from "../../../../../lib/safeInternalPath"; +const SCOPE = "auth.magicLink.verify"; + export async function GET(request: NextRequest) { + const requestId = getOrCreateRequestId(request); + if (!isDatabaseConfigured()) { - return dbUnavailable(); + const res = dbUnavailable(); + res.headers.set(REQUEST_ID_HEADER, requestId); + return res; } - const token = request.nextUrl.searchParams.get("token"); - if (!token || token.length < 10) { - return NextResponse.redirect( - new URL("/login?error=invalid_link", request.url), - ); - } - - let pepper: string; try { - pepper = getSessionPepper(); - } catch { - return NextResponse.redirect(new URL("/login?error=server", request.url)); - } + const token = request.nextUrl.searchParams.get("token"); + if (!token || token.length < 10) { + return redirectWithRequestId( + request, + "/login?error=invalid_link", + requestId, + ); + } - const tokenHash = hashSessionToken(token, pepper); + let pepper: string; + try { + pepper = getSessionPepper(); + } catch (err) { + logRouteError(SCOPE, requestId, err, { phase: "getSessionPepper" }); + return redirectWithRequestId(request, "/login?error=server", requestId); + } - const row = await prisma.magicLinkToken.findUnique({ - where: { tokenHash }, - }); + const tokenHash = hashSessionToken(token, pepper); - if (!row || row.expiresAt < new Date()) { - return NextResponse.redirect( - new URL("/login?error=expired_link", request.url), + const row = await prisma.magicLinkToken.findUnique({ + where: { tokenHash }, + }); + + if (!row || row.expiresAt < new Date()) { + return redirectWithRequestId( + request, + "/login?error=expired_link", + requestId, + ); + } + + await prisma.magicLinkToken.delete({ where: { id: row.id } }); + + const user = await prisma.user.upsert({ + where: { email: row.email }, + create: { email: row.email }, + update: {}, + }); + + const { token: sessionToken, expiresAt } = await createSessionForUser( + user.id, ); + await setSessionCookie(sessionToken, expiresAt); + + const dest = safeInternalPath(row.nextPath); + return redirectWithRequestId(request, dest, requestId); + } catch (err) { + logRouteError(SCOPE, requestId, err); + return redirectWithRequestId(request, "/login?error=server", requestId); } - - await prisma.magicLinkToken.delete({ where: { id: row.id } }); - - const user = await prisma.user.upsert({ - where: { email: row.email }, - create: { email: row.email }, - update: {}, - }); - - const { token: sessionToken, expiresAt } = await createSessionForUser( - user.id, - ); - await setSessionCookie(sessionToken, expiresAt); - - const dest = safeInternalPath(row.nextPath); - return NextResponse.redirect(new URL(dest, request.url)); +} + +function redirectWithRequestId( + request: NextRequest, + path: string, + requestId: string, +): NextResponse { + const res = NextResponse.redirect(new URL(path, request.url)); + res.headers.set(REQUEST_ID_HEADER, requestId); + return res; } diff --git a/app/api/auth/session/route.ts b/app/api/auth/session/route.ts index 430262b..fb4aed7 100644 --- a/app/api/auth/session/route.ts +++ b/app/api/auth/session/route.ts @@ -2,8 +2,9 @@ import { NextResponse } from "next/server"; import { isDatabaseConfigured } from "../../../../lib/server/env"; import { dbUnavailable } from "../../../../lib/server/responses"; import { getSessionUser } from "../../../../lib/server/session"; +import { apiRoute } from "../../../../lib/server/apiRoute"; -export async function GET() { +export const GET = apiRoute("auth.session", async () => { if (!isDatabaseConfigured()) { return dbUnavailable(); } @@ -16,4 +17,4 @@ export async function GET() { return NextResponse.json({ user: { id: user.id, email: user.email }, }); -} +}); diff --git a/app/api/drafts/me/route.ts b/app/api/drafts/me/route.ts index dbde8ca..667cc9d 100644 --- a/app/api/drafts/me/route.ts +++ b/app/api/drafts/me/route.ts @@ -2,20 +2,24 @@ import type { Prisma } from "@prisma/client"; import { NextRequest, NextResponse } from "next/server"; import { prisma } from "../../../../lib/server/db"; import { isDatabaseConfigured } from "../../../../lib/server/env"; -import { dbUnavailable } from "../../../../lib/server/responses"; +import { + dbUnavailable, + unauthorized, +} from "../../../../lib/server/responses"; import { getSessionUser } from "../../../../lib/server/session"; +import { apiRoute } from "../../../../lib/server/apiRoute"; import { putDraftBodySchema } from "../../../../lib/server/validation/createFlowSchemas"; import { readLimitedJson } from "../../../../lib/server/validation/requestBody"; import { jsonFromZodError } from "../../../../lib/server/validation/zodHttp"; -export async function GET() { +export const GET = apiRoute("drafts.me.get", async () => { if (!isDatabaseConfigured()) { return dbUnavailable(); } const user = await getSessionUser(); if (!user) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + return unauthorized(); } const draft = await prisma.ruleDraft.findUnique({ @@ -27,16 +31,16 @@ export async function GET() { ? { payload: draft.payload, updatedAt: draft.updatedAt } : null, }); -} +}); -export async function PUT(request: NextRequest) { +export const PUT = apiRoute("drafts.me.put", async (request: NextRequest) => { if (!isDatabaseConfigured()) { return dbUnavailable(); } const user = await getSessionUser(); if (!user) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + return unauthorized(); } const parsedBody = await readLimitedJson(request); @@ -67,16 +71,16 @@ export async function PUT(request: NextRequest) { return NextResponse.json({ draft: { payload: draft.payload, updatedAt: draft.updatedAt }, }); -} +}); -export async function DELETE() { +export const DELETE = apiRoute("drafts.me.delete", async () => { if (!isDatabaseConfigured()) { return dbUnavailable(); } const user = await getSessionUser(); if (!user) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + return unauthorized(); } // Idempotent: missing draft is a no-op so callers can fire-and-forget after @@ -84,4 +88,4 @@ export async function DELETE() { await prisma.ruleDraft.deleteMany({ where: { userId: user.id } }); return NextResponse.json({ ok: true }); -} +}); diff --git a/app/api/rules/[id]/route.ts b/app/api/rules/[id]/route.ts index 9b7330e..5d6e497 100644 --- a/app/api/rules/[id]/route.ts +++ b/app/api/rules/[id]/route.ts @@ -1,21 +1,25 @@ import { NextResponse } from "next/server"; import { isDatabaseConfigured } from "../../../../lib/server/env"; -import { dbUnavailable } from "../../../../lib/server/responses"; +import { dbUnavailable, notFound } from "../../../../lib/server/responses"; import { getPublicPublishedRuleById } from "../../../../lib/server/publishedRules"; +import { apiRoute } from "../../../../lib/server/apiRoute"; type RouteContext = { params: Promise<{ id: string }> }; -export async function GET(_request: Request, context: RouteContext) { - if (!isDatabaseConfigured()) { - return dbUnavailable(); - } +export const GET = apiRoute( + "rules.byId", + async (_request, context) => { + if (!isDatabaseConfigured()) { + return dbUnavailable(); + } - const { id } = await context.params; + const { id } = await context.params; - const rule = await getPublicPublishedRuleById(id); - if (!rule) { - return NextResponse.json({ error: "Not found" }, { status: 404 }); - } + const rule = await getPublicPublishedRuleById(id); + if (!rule) { + return notFound(); + } - return NextResponse.json({ rule }); -} + return NextResponse.json({ rule }); + }, +); diff --git a/app/api/rules/route.ts b/app/api/rules/route.ts index 5b9d352..e003871 100644 --- a/app/api/rules/route.ts +++ b/app/api/rules/route.ts @@ -2,13 +2,17 @@ import type { Prisma } from "@prisma/client"; import { NextRequest, NextResponse } from "next/server"; import { prisma } from "../../../lib/server/db"; import { isDatabaseConfigured } from "../../../lib/server/env"; -import { dbUnavailable } from "../../../lib/server/responses"; +import { + dbUnavailable, + unauthorized, +} from "../../../lib/server/responses"; import { getSessionUser } from "../../../lib/server/session"; +import { apiRoute } from "../../../lib/server/apiRoute"; import { publishRuleBodySchema } from "../../../lib/server/validation/createFlowSchemas"; import { readLimitedJson } from "../../../lib/server/validation/requestBody"; import { jsonFromZodError } from "../../../lib/server/validation/zodHttp"; -export async function GET(request: NextRequest) { +export const GET = apiRoute("rules.list", async (request: NextRequest) => { if (!isDatabaseConfigured()) { return dbUnavailable(); } @@ -29,16 +33,16 @@ export async function GET(request: NextRequest) { }); return NextResponse.json({ rules }); -} +}); -export async function POST(request: NextRequest) { +export const POST = apiRoute("rules.publish", async (request: NextRequest) => { if (!isDatabaseConfigured()) { return dbUnavailable(); } const user = await getSessionUser(); if (!user) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + return unauthorized(); } const parsedBody = await readLimitedJson(request); @@ -70,4 +74,4 @@ export async function POST(request: NextRequest) { createdAt: rule.createdAt, }, }); -} +}); diff --git a/docs/guides/backend-linear-tickets.md b/docs/guides/backend-linear-tickets.md index 78f7328..4af47bc 100644 --- a/docs/guides/backend-linear-tickets.md +++ b/docs/guides/backend-linear-tickets.md @@ -11,7 +11,7 @@ A backend review was merged into **[docs/backend-roadmap.md](backend-roadmap.md) ### Audit note (Linear CR-72+ vs repo, 2026-04) - **Done in Linear and shipped:** **CR-72–CR-76**, **CR-77** (publish from create flow), **CR-78** (template seed), **CR-79**, **CR-88**, **CR-89**. The **CR-72 → CR-83** numbering is the original **sequential plan**, not current blocking order; the **core product vertical** through publish + templates is effectively complete in-repo. -- **Backlog (still open):** **CR-80** (web vitals — file-based route remains), **CR-81** (public rule detail — no `GET /api/rules/[id]` or marketing detail page yet), **CR-82** (CI migrate smoke), **CR-84** / **CR-85** (parallel hygiene), **CR-86** (profile + account + draft resume — UI mostly placeholder), **CR-90** / **CR-91**, **CR-93** (template grid facets on marketing). +- **Backlog (still open):** **CR-80** (web vitals — file-based route remains), **CR-81** (public rule detail — no `GET /api/rules/[id]` or marketing detail page yet), **CR-82** (CI migrate smoke), **CR-85** (parallel hygiene — session lifecycle), **CR-86** (profile + account + draft resume — UI mostly placeholder), **CR-90** / **CR-91**, **CR-93** (template grid facets on marketing). **CR-84 Done** — canonical error contract `{ error: { code, message }, details? }` and `x-request-id` propagation shipped via `lib/server/{responses,requestId,apiRoute}.ts`; auth + drafts + rules routes migrated, remaining `app/api/*` are a follow-up pass. - **CR-83 Done (admin handoff scope):** [`docs/guides/ops-backend-deploy.md`](ops-backend-deploy.md) shipped as the **admin handoff sheet** (access, env vars, platform settings, open decisions). The full deploy runbook is intentionally split out — see the new follow-up tickets in [Ticket 12 / CR-83 follow-ups](#follow-up-tickets-filed-under-cr-83) below. - **CR-86** is **no longer blocked** by publish — **CR-77** is **Done**; profile work is gated by **implementation**, not waiting on publish wiring. - **Not in this ticket list** but called out in **[docs/backend-roadmap.md](backend-roadmap.md):** shared **rate-limit store** (e.g. Redis) before multi-instance; **`GET /api/create-flow/methods`** exists for facet scoring (Ticket 16 / CR-88) but is not duplicated as a separate doc ticket. @@ -607,12 +607,12 @@ All six are titled `[Backend] …`, assigned to Vinod, in the **community-rule** **Acceptance criteria:** -- [ ] At least auth + draft + rules routes return the agreed shape for new code paths. -- [ ] Errors in logs include request id when available. +- [x] At least auth + draft + rules routes return the agreed shape for new code paths. +- [x] Errors in logs include request id when available. -**Files:** `lib/server/` (new helper), selected `app/api/**/route.ts`, optional tests. +**Files:** [`lib/server/responses.ts`](lib/server/responses.ts), [`lib/server/requestId.ts`](lib/server/requestId.ts), [`lib/server/apiRoute.ts`](lib/server/apiRoute.ts); migrated `app/api/auth/**/route.ts`, `app/api/drafts/me/route.ts`, `app/api/rules/route.ts`, `app/api/rules/[id]/route.ts`; tests in `tests/unit/{responses,requestId,apiRoute,draftsMeRoute,rulesByIdRoute}.test.ts`. -**Linear:** [CR-84](https://linear.app/community-rule/issue/CR-84/backend-api-error-contract-request-id-logging) (**CR-73** Done — ready to pick up). +**Linear:** [CR-84](https://linear.app/community-rule/issue/CR-84/backend-api-error-contract-request-id-logging) **Done** (**CR-73** Done — was ready to pick up). --- diff --git a/docs/guides/backend-roadmap.md b/docs/guides/backend-roadmap.md index 9071a3b..b735d88 100644 --- a/docs/guides/backend-roadmap.md +++ b/docs/guides/backend-roadmap.md @@ -115,9 +115,9 @@ Match the current API behavior; tighten as product evolves: ## 7. API responses, errors, and observability -**Error JSON (target):** Prefer a stable shape, e.g. `{ "error": { "code": "string", "message": "string" }, "details"?: ... }` for 4xx/5xx, instead of only `{ "error": "string" }`. Validation errors can map into `details`. Implement gradually in route handlers. +**Error JSON (implemented):** 4xx/5xx bodies use the canonical shape `{ "error": { "code": "string", "message": "string" }, "details"?: ... }`. Codes come from the `ApiErrorCode` union in [`lib/server/responses.ts`](../lib/server/responses.ts) (helpers: `errorJson`, `dbUnavailable`, `unauthorized`, `notFound`, `rateLimited`, `serverMisconfigured`, `internalError`); validation failures use [`jsonFromZodError`](../lib/server/validation/zodHttp.ts) and surface flattened issues in `details`. **Migrated:** auth (`/api/auth/*`), drafts (`/api/drafts/me`), rules (`/api/rules`, `/api/rules/[id]`). Remaining `app/api/*` handlers (e.g. `web-vitals`, `templates`, `create-flow/methods`, `health`) are a follow-up pass; new routes should adopt the helpers from day one. -**Logging:** Use the shared [`lib/logger.ts`](../lib/logger.ts) where possible. Include a **request correlation id** (reuse `x-request-id` if present, else generate) on API routes and log it with errors so support can tie logs together. +**Logging:** Use the shared [`lib/logger.ts`](../lib/logger.ts) where possible. Wrap route handlers with [`apiRoute(scope, handler)`](../lib/server/apiRoute.ts) so a sanitized `x-request-id` is generated (or forwarded) onto every response and uncaught throws return the canonical 500 with the id logged via `logRouteError` ([`lib/server/requestId.ts`](../lib/server/requestId.ts)). Pass the `requestId` through to in-handler `logRouteError(scope, requestId, err, extra?)` calls when catching expected failures (e.g. mail send) so support can tie logs together. **Metrics:** No vendor required for v1; optional later: request duration, error counts. diff --git a/lib/server/apiRoute.ts b/lib/server/apiRoute.ts new file mode 100644 index 0000000..3c68be3 --- /dev/null +++ b/lib/server/apiRoute.ts @@ -0,0 +1,45 @@ +import type { NextRequest, NextResponse } from "next/server"; +import { internalError } from "./responses"; +import { + getOrCreateRequestId, + logRouteError, + withRequestId, +} from "./requestId"; + +export interface ApiRouteMeta { + requestId: string; +} + +export type ApiHandler = ( + request: NextRequest, + ctx: Ctx, + meta: ApiRouteMeta, +) => Promise | NextResponse; + +/** + * Minimal wrapper around a Route Handler that: + * + * - generates or forwards an `x-request-id`, + * - attaches that id to every response (success and error), + * - catches uncaught throws, logs them with the id via `lib/logger`, and + * returns the canonical 500 `internal_error` body. + * + * Pass a `scope` like `"auth.magicLink.request"` so logs are filterable per + * route. Handlers that don't need request-id correlation can skip the + * wrapper. + */ +export function apiRoute( + scope: string, + handler: ApiHandler, +): (request: NextRequest, ctx: Ctx) => Promise { + return async (request, ctx) => { + const requestId = getOrCreateRequestId(request); + try { + const res = await handler(request, ctx, { requestId }); + return withRequestId(res, requestId); + } catch (err) { + logRouteError(scope, requestId, err); + return withRequestId(internalError(), requestId); + } + }; +} diff --git a/lib/server/requestId.ts b/lib/server/requestId.ts new file mode 100644 index 0000000..8f865e1 --- /dev/null +++ b/lib/server/requestId.ts @@ -0,0 +1,70 @@ +import type { NextResponse } from "next/server"; +import { logger } from "../logger"; + +export const REQUEST_ID_HEADER = "x-request-id"; + +const MAX_REQUEST_ID_LENGTH = 128; +const REQUEST_ID_PATTERN = /^[A-Za-z0-9_.-]+$/; + +/** + * Returns the incoming `x-request-id` header (sanitized) when present and + * well-formed, otherwise generates a fresh UUID. Sanitization rejects + * oversized values and characters outside `[A-Za-z0-9_.-]` so log lines + * cannot be poisoned by client-controlled input. + */ +export function getOrCreateRequestId(request: Request): string { + const raw = request.headers.get(REQUEST_ID_HEADER); + if (raw) { + const trimmed = raw.trim(); + if ( + trimmed.length > 0 && + trimmed.length <= MAX_REQUEST_ID_LENGTH && + REQUEST_ID_PATTERN.test(trimmed) + ) { + return trimmed; + } + } + return crypto.randomUUID(); +} + +/** + * Attach the request id to a response so callers (and log drains) can + * correlate logs with the response. Returns the same response for chaining. + */ +export function withRequestId( + res: T, + requestId: string, +): T { + res.headers.set(REQUEST_ID_HEADER, requestId); + return res; +} + +interface ErrorLogPayload { + scope: string; + requestId: string; + message: string; + stack?: string; + [key: string]: unknown; +} + +/** + * Structured error log including the request id. Use from route handlers + * (and the `apiRoute` wrapper) so support can tie a 5xx back to log lines. + */ +export function logRouteError( + scope: string, + requestId: string, + err: unknown, + extra?: Record, +): void { + const payload: ErrorLogPayload = { + scope, + requestId, + message: err instanceof Error ? err.message : String(err), + ...(extra ?? {}), + }; + if (err instanceof Error && err.stack) { + payload.stack = err.stack; + } + logger.error(payload); +} diff --git a/lib/server/responses.ts b/lib/server/responses.ts index 0ccfe6a..9eed8f2 100644 --- a/lib/server/responses.ts +++ b/lib/server/responses.ts @@ -1,8 +1,83 @@ import { NextResponse } from "next/server"; +/** + * Canonical API error contract for `app/api/**`. + * + * Response body shape: `{ error: { code, message }, details? }`. + * + * Codes are kept intentionally small. Add a new code only when an existing + * one cannot describe the failure; route handlers should not invent codes + * inline (use `errorJson(code, …)` here so the union stays the source of + * truth). + */ +export type ApiErrorCode = + | "db_unavailable" + | "unauthorized" + | "forbidden" + | "not_found" + | "validation_error" + | "invalid_json" + | "payload_too_large" + | "rate_limited" + | "server_misconfigured" + | "mail_failed" + | "internal_error"; + +export interface ApiErrorBody { + error: { code: ApiErrorCode; message: string }; + details?: unknown; +} + +interface ErrorOpts { + details?: unknown; + headers?: HeadersInit; +} + +export function errorJson( + code: ApiErrorCode, + message: string, + status: number, + opts: ErrorOpts = {}, +): NextResponse { + const body: ApiErrorBody = { error: { code, message } }; + if (opts.details !== undefined) { + body.details = opts.details; + } + return NextResponse.json(body, { status, headers: opts.headers }); +} + export function dbUnavailable(): NextResponse { - return NextResponse.json( - { error: "Database is not configured (DATABASE_URL)." }, - { status: 503 }, + return errorJson( + "db_unavailable", + "Database is not configured (DATABASE_URL).", + 503, ); } + +export function unauthorized(message = "Unauthorized"): NextResponse { + return errorJson("unauthorized", message, 401); +} + +export function notFound(message = "Not found"): NextResponse { + return errorJson("not_found", message, 404); +} + +export function rateLimited(retryAfterMs: number): NextResponse { + const retryAfterSec = Math.max(1, Math.ceil(retryAfterMs / 1000)); + return errorJson("rate_limited", "Too many requests", 429, { + details: { retryAfterMs }, + headers: { "retry-after": String(retryAfterSec) }, + }); +} + +export function serverMisconfigured( + message = "Server misconfiguration", +): NextResponse { + return errorJson("server_misconfigured", message, 500); +} + +export function internalError( + message = "Internal server error", +): NextResponse { + return errorJson("internal_error", message, 500); +} diff --git a/tests/unit/apiRoute.test.ts b/tests/unit/apiRoute.test.ts new file mode 100644 index 0000000..429e2b8 --- /dev/null +++ b/tests/unit/apiRoute.test.ts @@ -0,0 +1,86 @@ +import { NextRequest, NextResponse } from "next/server"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +const errorMock = vi.fn(); +vi.mock("../../lib/logger", () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: (...args: unknown[]) => errorMock(...args), + }, +})); + +import { apiRoute } from "../../lib/server/apiRoute"; +import { REQUEST_ID_HEADER } from "../../lib/server/requestId"; + +afterEach(() => { + errorMock.mockReset(); +}); + +function makeReq(headers: Record = {}): NextRequest { + return new NextRequest("https://x.test/api/x", { headers }); +} + +describe("lib/server/apiRoute", () => { + it("attaches a generated x-request-id to a successful response", async () => { + const handler = apiRoute("test.scope", () => + NextResponse.json({ ok: true }), + ); + const res = await handler(makeReq(), undefined); + expect(res.status).toBe(200); + const id = res.headers.get(REQUEST_ID_HEADER); + expect(id).toBeTruthy(); + expect(id).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/, + ); + }); + + it("forwards an incoming x-request-id and exposes it to the handler", async () => { + const incoming = "req_forwarded-1"; + let seen: string | undefined; + const handler = apiRoute("test.scope", (_req, _ctx, { requestId }) => { + seen = requestId; + return NextResponse.json({ ok: true }); + }); + const res = await handler( + makeReq({ [REQUEST_ID_HEADER]: incoming }), + undefined, + ); + expect(seen).toBe(incoming); + expect(res.headers.get(REQUEST_ID_HEADER)).toBe(incoming); + }); + + it("returns canonical 500 + logs when the handler throws", async () => { + const handler = apiRoute("test.scope", () => { + throw new Error("boom"); + }); + const res = await handler(makeReq(), undefined); + expect(res.status).toBe(500); + const body = (await res.json()) as { + error: { code: string; message: string }; + }; + expect(body.error.code).toBe("internal_error"); + expect(res.headers.get(REQUEST_ID_HEADER)).toBeTruthy(); + + expect(errorMock).toHaveBeenCalledTimes(1); + const payload = errorMock.mock.calls[0][0] as Record; + expect(payload.scope).toBe("test.scope"); + expect(payload.requestId).toBe(res.headers.get(REQUEST_ID_HEADER)); + expect(payload.message).toBe("boom"); + }); + + it("passes the route ctx through to the handler", async () => { + type Ctx = { params: Promise<{ id: string }> }; + const handler = apiRoute("test.scope", async (_req, ctx) => { + const { id } = await ctx.params; + return NextResponse.json({ id }); + }); + const res = await handler(makeReq(), { + params: Promise.resolve({ id: "abc" }), + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { id: string }; + expect(body.id).toBe("abc"); + }); +}); diff --git a/tests/unit/draftsMeRoute.test.ts b/tests/unit/draftsMeRoute.test.ts new file mode 100644 index 0000000..456c680 --- /dev/null +++ b/tests/unit/draftsMeRoute.test.ts @@ -0,0 +1,100 @@ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const isDatabaseConfiguredMock = vi.fn(); +const getSessionUserMock = vi.fn(); +const findUniqueMock = vi.fn(); + +vi.mock("../../lib/server/env", () => ({ + isDatabaseConfigured: () => isDatabaseConfiguredMock(), +})); + +vi.mock("../../lib/server/db", () => ({ + prisma: { + ruleDraft: { + findUnique: (...args: unknown[]) => findUniqueMock(...args), + upsert: vi.fn(), + deleteMany: vi.fn(), + }, + }, +})); + +vi.mock("../../lib/server/session", () => ({ + getSessionUser: () => getSessionUserMock(), +})); + +import { GET } from "../../app/api/drafts/me/route"; + +beforeEach(() => { + isDatabaseConfiguredMock.mockReset(); + getSessionUserMock.mockReset(); + findUniqueMock.mockReset(); +}); + +describe("GET /api/drafts/me", () => { + it("returns 503 with the canonical shape when the database is not configured", async () => { + isDatabaseConfiguredMock.mockReturnValue(false); + const res = await GET( + new NextRequest("https://x.test/api/drafts/me"), + undefined, + ); + expect(res.status).toBe(503); + const body = (await res.json()) as { + error: { code: string; message: string }; + }; + expect(body.error.code).toBe("db_unavailable"); + expect(res.headers.get("x-request-id")).toBeTruthy(); + }); + + it("returns 401 unauthorized with the canonical shape when no session user", async () => { + isDatabaseConfiguredMock.mockReturnValue(true); + getSessionUserMock.mockResolvedValueOnce(null); + const res = await GET( + new NextRequest("https://x.test/api/drafts/me"), + undefined, + ); + expect(res.status).toBe(401); + const body = (await res.json()) as { + error: { code: string; message: string }; + }; + expect(body.error).toEqual({ + code: "unauthorized", + message: "Unauthorized", + }); + expect(res.headers.get("x-request-id")).toBeTruthy(); + expect(findUniqueMock).not.toHaveBeenCalled(); + }); + + it("forwards an incoming x-request-id on the response", async () => { + isDatabaseConfiguredMock.mockReturnValue(true); + getSessionUserMock.mockResolvedValueOnce(null); + const res = await GET( + new NextRequest("https://x.test/api/drafts/me", { + headers: { "x-request-id": "req_drafts-1" }, + }), + undefined, + ); + expect(res.headers.get("x-request-id")).toBe("req_drafts-1"); + }); + + it("returns the draft when present", async () => { + isDatabaseConfiguredMock.mockReturnValue(true); + getSessionUserMock.mockResolvedValueOnce({ + id: "u1", + email: "x@y.test", + }); + findUniqueMock.mockResolvedValueOnce({ + payload: { foo: 1 }, + updatedAt: new Date("2026-01-01T00:00:00Z"), + }); + const res = await GET( + new NextRequest("https://x.test/api/drafts/me"), + undefined, + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + draft: { payload: { foo: number } } | null; + }; + expect(body.draft?.payload).toEqual({ foo: 1 }); + }); +}); diff --git a/tests/unit/requestId.test.ts b/tests/unit/requestId.test.ts new file mode 100644 index 0000000..6df6ff9 --- /dev/null +++ b/tests/unit/requestId.test.ts @@ -0,0 +1,60 @@ +import { NextResponse } from "next/server"; +import { describe, expect, it } from "vitest"; +import { + REQUEST_ID_HEADER, + getOrCreateRequestId, + withRequestId, +} from "../../lib/server/requestId"; + +function reqWith(headers: Record): Request { + return new Request("https://x.test/api/x", { headers }); +} + +describe("lib/server/requestId", () => { + it("generates a UUID when no header is present", () => { + const id = getOrCreateRequestId(reqWith({})); + expect(id).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/, + ); + }); + + it("preserves a well-formed incoming x-request-id", () => { + const incoming = "req_abc-123.456"; + const id = getOrCreateRequestId(reqWith({ [REQUEST_ID_HEADER]: incoming })); + expect(id).toBe(incoming); + }); + + it("trims surrounding whitespace from a well-formed id", () => { + const id = getOrCreateRequestId( + reqWith({ [REQUEST_ID_HEADER]: " abc-123 " }), + ); + expect(id).toBe("abc-123"); + }); + + it("rejects oversized ids and falls back to a UUID", () => { + const huge = "a".repeat(200); + const id = getOrCreateRequestId(reqWith({ [REQUEST_ID_HEADER]: huge })); + expect(id).not.toBe(huge); + expect(id).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/, + ); + }); + + it("rejects ids with disallowed characters", () => { + // Spaces, semicolons, slashes are valid HTTP header bytes but disallowed + // by our `[A-Za-z0-9_.-]` pattern. + const bad = "abc def;