server: generic 500s, fix duplicate/invalid-id error handling

All admin API error responses (and the global error handler) now emit
RFC 7807 application/problem+json bodies (type/title/status/detail)
instead of the ad hoc { error: string } shape, per review feedback on
this PR. Scoped to admin.ts + the global handler in index.ts, since
that's what this PR already touches; public.ts's routes still use the
old shape pending a follow-up.

client/src/admin/Admin.tsx

@@ -7,10 +7,14 @@ async function j<T = any>(res: Response): Promise<T> {
   return res.json();
 }
 
-// Best-effort error message from a failed response ({ error } JSON or status text).
+// Best-effort error message from a failed response. Prefers RFC 7807
+// (application/problem+json) `detail`/`title` — what the admin routes send
+// — falls back to the older ad hoc `{ error }` shape (still used by a few
+// not-yet-converted endpoints), then plain status text.
 async function errText(res: Response): Promise<string> {
   try {
-    return (await res.json()).error ?? res.statusText;
+    const body = await res.json();
+    return body.detail ?? body.title ?? body.error ?? res.statusText;
   } catch {
     return res.statusText;
   }

server/src/index.ts

@@ -8,6 +8,7 @@ import { existsSync } from 'node:fs';
 import { openDb } from './db.js';
 import { registerPublicRoutes } from './routes/public.js';
 import { registerAdminRoutes } from './routes/admin.js';
+import { sendProblem } from './problem-details.js';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 
@@ -46,6 +47,38 @@ const app = Fastify({
   trustProxy: process.env.WUTZ_TRUST_PROXY === '1',
 });
 
+// RFC 7807 title for Fastify's own sub-500 errors (bad JSON body, schema
+// validation) — those never go through our own `sendProblem` call sites,
+// so this is the only place their status code needs a stable `title`.
+const STATUS_TITLES: Record<number, string> = {
+  400: 'Bad Request',
+  401: 'Unauthorized',
+  403: 'Forbidden',
+  404: 'Not Found',
+  409: 'Conflict',
+  413: 'Payload Too Large',
+  415: 'Unsupported Media Type',
+  429: 'Too Many Requests',
+};
+
+// Fastify's default error handler serialises a thrown error's `code` and
+// `message` straight into the response body — fine for our own explicit
+// `reply.code(4xx).send(...)` calls (those never reach this handler at
+// all), but any *unhandled* throw (a raw SQLite constraint error, for
+// instance) would otherwise hand the client a raw SQL message. Keep
+// Fastify's own sub-500 errors (bad JSON body, schema validation) as-is —
+// those messages are meant for the client — and generalise anything ≥ 500.
+// All bodies are RFC 7807 (application/problem+json) — see problem-details.ts.
+app.setErrorHandler((err, req, reply) => {
+  const statusCode = err.statusCode ?? 500;
+  if (statusCode < 500) {
+    sendProblem(req, reply, statusCode, STATUS_TITLES[statusCode] ?? 'Bad Request', err.message);
+    return;
+  }
+  req.log.error(err);
+  sendProblem(req, reply, 500, 'Internal Server Error', 'internal error');
+});
+
 await app.register(fastifyCookie);
 
 const db = openDb(DB_PATH);
@@ -65,7 +98,7 @@ if (existsSync(clientDist)) {
   });
   app.setNotFoundHandler((req, reply) => {
     if (req.url.startsWith('/api') || req.url.startsWith('/admin/api')) {
-      reply.code(404).send({ error: 'not found' });
+      sendProblem(req, reply, 404, 'Not Found', 'not found');
       return;
     }
     if (req.url.startsWith('/admin')) {

server/src/problem-details.ts

@@ -0,0 +1,31 @@
+import type { FastifyReply, FastifyRequest } from 'fastify';
+
+// RFC 7807 (application/problem+json) error body. `title` is the general
+// class of problem — stable for a given status/situation, safe to key UI
+// copy off of — while `detail` is specific to this occurrence (the old ad
+// hoc `{ error: string }` body's message lives on here so existing client
+// error-text handling degrades gracefully).
+export interface ProblemDetails {
+  type: string;
+  title: string;
+  status: number;
+  detail?: string;
+  instance?: string;
+}
+
+export function sendProblem(
+  req: FastifyRequest,
+  reply: FastifyReply,
+  status: number,
+  title: string,
+  detail?: string
+): void {
+  const body: ProblemDetails = {
+    type: 'about:blank',
+    title,
+    status,
+    ...(detail !== undefined ? { detail } : {}),
+    instance: req.url,
+  };
+  reply.code(status).header('Content-Type', 'application/problem+json').send(body);
+}

server/src/routes/admin.ts

@@ -2,6 +2,7 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
 import type { DB } from '../db.js';
 import { businessDay, formatLocal, parseDbTime } from '../time.js';
 import { createHash, randomBytes, timingSafeEqual } from 'node:crypto';
+import { sendProblem } from '../problem-details.js';
 
 const SESSION_COOKIE = 'wutz_admin';
 
@@ -35,16 +36,29 @@ function safeEqual(a: string, b: string): boolean {
 
 function requireAuth(req: FastifyRequest, reply: FastifyReply): boolean {
   if (isAuthed(req)) return true;
-  reply.code(401).send({ error: 'unauthorized' });
+  sendProblem(req, reply, 401, 'Unauthorized', 'unauthorized');
   return false;
 }
 
+// `Number(nonNumericString)` is NaN, and NaN binds as SQL NULL — an
+// `UPDATE ... WHERE id = NULL` silently matches zero rows instead of
+// throwing, so a bad :id param used to "succeed" the same way a genuinely
+// missing id does. Reject it explicitly instead.
+function parseId(raw: string, req: FastifyRequest, reply: FastifyReply): number | null {
+  const id = Number(raw);
+  if (!Number.isInteger(id)) {
+    sendProblem(req, reply, 400, 'Bad Request', 'invalid id');
+    return null;
+  }
+  return id;
+}
+
 export function registerAdminRoutes(app: FastifyInstance, db: DB) {
   app.post<{ Body: { password?: string } }>('/admin/login', async (req, reply) => {
     const expected = process.env.ADMIN_PASSWORD;
-    if (!expected) return reply.code(500).send({ error: 'ADMIN_PASSWORD not set' });
+    if (!expected) return sendProblem(req, reply, 500, 'Internal Server Error', 'ADMIN_PASSWORD not set');
     if (!safeEqual(req.body?.password ?? '', expected)) {
-      return reply.code(401).send({ error: 'wrong password' });
+      return sendProblem(req, reply, 401, 'Unauthorized', 'wrong password');
     }
     const token = randomBytes(32).toString('hex');
     validSessions.add(token);
@@ -79,11 +93,18 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
     async (req, reply) => {
       if (!requireAuth(req, reply)) return;
       const { name, price_cents } = req.body ?? ({} as any);
-      if (!name || !isValidCents(price_cents)) return reply.code(400).send({ error: 'invalid' });
-      const info = db
-        .prepare('INSERT INTO drinks (name, price_cents) VALUES (?, ?)')
-        .run(name, price_cents);
-      return { id: Number(info.lastInsertRowid) };
+      if (!name || !isValidCents(price_cents)) return sendProblem(req, reply, 400, 'Bad Request', 'invalid');
+      try {
+        const info = db
+          .prepare('INSERT INTO drinks (name, price_cents) VALUES (?, ?)')
+          .run(name, price_cents);
+        return { id: Number(info.lastInsertRowid) };
+      } catch (e: any) {
+        if (String(e.message).includes('UNIQUE') && String(e.message).includes('drinks.name')) {
+          return sendProblem(req, reply, 409, 'Conflict', 'name already exists');
+        }
+        throw e;
+      }
     }
   );
 
@@ -91,10 +112,11 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
     '/admin/api/drinks/:id',
     async (req, reply) => {
       if (!requireAuth(req, reply)) return;
-      const id = Number(req.params.id);
+      const id = parseId(req.params.id, req, reply);
+      if (id === null) return;
       const { name, price_cents, archived } = req.body ?? {};
       if (price_cents !== undefined && !isValidCents(price_cents)) {
-        return reply.code(400).send({ error: 'invalid price_cents' });
+        return sendProblem(req, reply, 400, 'Bad Request', 'invalid price_cents');
       }
       const sets: string[] = [];
       const args: any[] = [];
@@ -103,26 +125,33 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
       if (archived !== undefined) { sets.push('archived = ?'); args.push(archived ? 1 : 0); }
       if (sets.length === 0) return { ok: true };
       args.push(id);
-      db.prepare(`UPDATE drinks SET ${sets.join(', ')} WHERE id = ?`).run(...args);
+      const info = db.prepare(`UPDATE drinks SET ${sets.join(', ')} WHERE id = ?`).run(...args);
+      if (info.changes === 0) return sendProblem(req, reply, 404, 'Not Found', 'drink not found');
       return { ok: true };
     }
   );
 
   app.delete<{ Params: { id: string } }>('/admin/api/drinks/:id', async (req, reply) => {
     if (!requireAuth(req, reply)) return;
-    const id = Number(req.params.id);
+    const id = parseId(req.params.id, req, reply);
+    if (id === null) return;
     const sold = db
       .prepare('SELECT 1 FROM transaction_items WHERE drink_id = ? LIMIT 1')
       .get(id);
     if (sold) {
-      return reply
-        .code(409)
-        .send({ error: 'Getränk wurde bereits verkauft — bitte archivieren statt löschen.' });
+      return sendProblem(
+        req,
+        reply,
+        409,
+        'Conflict',
+        'Getränk wurde bereits verkauft — bitte archivieren statt löschen.'
+      );
     }
-    db.transaction(() => {
+    const changes = db.transaction(() => {
       db.prepare('DELETE FROM bar_drinks WHERE drink_id = ?').run(id);
-      db.prepare('DELETE FROM drinks WHERE id = ?').run(id);
+      return db.prepare('DELETE FROM drinks WHERE id = ?').run(id).changes;
     })();
+    if (changes === 0) return sendProblem(req, reply, 404, 'Not Found', 'drink not found');
     return { ok: true };
   });
 
@@ -145,9 +174,9 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
       if (!requireAuth(req, reply)) return;
       const name = req.body?.name?.trim();
       const pfand_cents = req.body?.pfand_cents ?? 0;
-      if (!name) return reply.code(400).send({ error: 'name required' });
+      if (!name) return sendProblem(req, reply, 400, 'Bad Request', 'name required');
       if (!isValidCents(pfand_cents)) {
-        return reply.code(400).send({ error: 'invalid pfand_cents' });
+        return sendProblem(req, reply, 400, 'Bad Request', 'invalid pfand_cents');
       }
       try {
         const info = db
@@ -156,7 +185,7 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
         return { id: Number(info.lastInsertRowid) };
       } catch (e: any) {
         if (String(e.message).includes('UNIQUE')) {
-          return reply.code(409).send({ error: 'name already exists' });
+          return sendProblem(req, reply, 409, 'Conflict', 'name already exists');
         }
         throw e;
       }
@@ -167,10 +196,16 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
     '/admin/api/bars/:id',
     async (req, reply) => {
       if (!requireAuth(req, reply)) return;
-      const id = Number(req.params.id);
+      const id = parseId(req.params.id, req, reply);
+      if (id === null) return;
       const { name, pfand_cents, drink_ids } = req.body ?? {};
       if (pfand_cents !== undefined && !isValidCents(pfand_cents)) {
-        return reply.code(400).send({ error: 'invalid pfand_cents' });
+        return sendProblem(req, reply, 400, 'Bad Request', 'invalid pfand_cents');
+      }
+      if (drink_ids !== undefined) {
+        if (!Array.isArray(drink_ids) || !drink_ids.every(did => Number.isInteger(did))) {
+          return sendProblem(req, reply, 400, 'Bad Request', 'invalid drink_ids');
+        }
       }
       try {
         db.transaction(() => {
@@ -187,14 +222,18 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
             const ins = db.prepare(
               'INSERT INTO bar_drinks (bar_id, drink_id, sort_order) VALUES (?, ?, ?)'
             );
-            drink_ids.forEach((did, idx) => ins.run(id, did, idx));
+            // Dedupe: a repeated id would otherwise trip bar_drinks' own
+            // (bar_id, drink_id) primary key and get mis-reported below as
+            // "name already exists" via the UNIQUE substring match.
+            [...new Set(drink_ids)].forEach((did, idx) => ins.run(id, did, idx));
           }
         })();
       } catch (e: any) {
-        if (String(e.message).includes('UNIQUE')) {
-          return reply.code(409).send({ error: 'name already exists' });
+        const msg = String(e.message);
+        if (msg.includes('UNIQUE') && msg.includes('bars.name')) {
+          return sendProblem(req, reply, 409, 'Conflict', 'name already exists');
         }
-        if (e.message === 'name empty') return reply.code(400).send({ error: 'name empty' });
+        if (msg === 'name empty') return sendProblem(req, reply, 400, 'Bad Request', 'name empty');
         throw e;
       }
       return { ok: true };
@@ -203,19 +242,25 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
 
   app.delete<{ Params: { id: string } }>('/admin/api/bars/:id', async (req, reply) => {
     if (!requireAuth(req, reply)) return;
-    const id = Number(req.params.id);
+    const id = parseId(req.params.id, req, reply);
+    if (id === null) return;
     const used = db
       .prepare('SELECT 1 FROM transactions WHERE bar_id = ? LIMIT 1')
       .get(id);
     if (used) {
-      return reply
-        .code(409)
-        .send({ error: 'Tresen hat Transaktionen — Löschen würde die Statistik verfälschen.' });
+      return sendProblem(
+        req,
+        reply,
+        409,
+        'Conflict',
+        'Tresen hat Transaktionen — Löschen würde die Statistik verfälschen.'
+      );
     }
-    db.transaction(() => {
+    const changes = db.transaction(() => {
       db.prepare('DELETE FROM bar_drinks WHERE bar_id = ?').run(id);
-      db.prepare('DELETE FROM bars WHERE id = ?').run(id);
+      return db.prepare('DELETE FROM bars WHERE id = ?').run(id).changes;
     })();
+    if (changes === 0) return sendProblem(req, reply, 404, 'Not Found', 'bar not found');
     return { ok: true };
   });