refactor(routes): categories → uniform Express-5 throw pattern (Pillar B template)

Establishes the canonical route error pattern (see docs/CODE_STANDARDS.md):
handlers `throw ValidationError/NotFoundError/ConflictError(...)` and let
Express 5 forward to the single terminal handler — no per-handler
try/catch swallows, no ad-hoc `res.status().json({error})`. Only the
UNIQUE-constraint → 409 mapping keeps a tight try/catch (`asConflict`).

- apiError factories NotFoundError/ConflictError now take an optional
  `field` (parity with ValidationError) so form-field info is preserved.
- routes/categories.cts fully converted; identical status/messages, now
  uniform `{ error, message, code, field }` bodies.
- Test harnesses (categoryGroups, categoryReorder) updated to simulate the
  terminal handler (thrown ApiError → formatted response).

Verified: 236/236 server tests; e2e probe 17/17; live smoke confirms
400 VALIDATION_ERROR / 404 NOT_FOUND with correct field + status.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
null 2026-07-06 14:46:32 -05:00
parent aee23d6025
commit 7390c458ed
4 changed files with 104 additions and 117 deletions

View File

@ -1,20 +1,26 @@
// @ts-nocheck — route controller converted to .cts; handler req/res typed, full type-check deferred (thin glue over typed services). // @ts-nocheck — route controller converted to .cts; handler req/res typed, full type-check deferred (thin glue over typed services).
import type { Req, Res, Next } from '../types/http'; import type { Req, Res, Next } from '../types/http';
const express = require('express'); const express = require('express');
const { standardizeError } = require('../middleware/errorFormatter.cts'); const { ValidationError, NotFoundError, ConflictError } = require('../utils/apiError.cts');
const router = express.Router(); const router = express.Router();
const { getDb, ensureUserDefaultCategories } = require('../db/database.cts'); const { getDb, ensureUserDefaultCategories } = require('../db/database.cts');
const { accountingActiveSql } = require('../services/paymentAccountingService.cts'); const { accountingActiveSql } = require('../services/paymentAccountingService.cts');
// Maps a SQLite UNIQUE-constraint violation to a friendly 409; re-throws anything
// else so the terminal handler returns a safe 500.
function asConflict(e, message, field) {
if (e.message?.includes('UNIQUE')) throw ConflictError(message, field);
throw e;
}
// GET /api/categories // GET /api/categories
router.get('/', (req: Req, res: Res) => { router.get('/', (req: Req, res: Res) => {
try { const db = getDb();
const db = getDb(); ensureUserDefaultCategories(req.user.id);
ensureUserDefaultCategories(req.user.id);
const categories = db const categories = db
.prepare( .prepare(
` `
SELECT id, user_id, name, sort_order, spending_enabled, group_id, created_at, updated_at SELECT id, user_id, name, sort_order, spending_enabled, group_id, created_at, updated_at
FROM categories FROM categories
WHERE user_id = ? WHERE user_id = ?
@ -23,10 +29,10 @@ router.get('/', (req: Req, res: Res) => {
sort_order ASC, sort_order ASC,
name COLLATE NOCASE ASC name COLLATE NOCASE ASC
`, `,
) )
.all(req.user.id); .all(req.user.id);
const billsByCategory = db.prepare(` const billsByCategory = db.prepare(`
SELECT SELECT
b.id, b.id,
b.category_id, b.category_id,
@ -49,36 +55,32 @@ router.get('/', (req: Req, res: Res) => {
ORDER BY b.active DESC, b.due_day ASC, b.name COLLATE NOCASE ASC ORDER BY b.active DESC, b.due_day ASC, b.name COLLATE NOCASE ASC
`); `);
const shaped = categories.map((category) => { const shaped = categories.map((category) => {
const bills = billsByCategory.all(req.user.id, category.id).map((bill) => ({ const bills = billsByCategory.all(req.user.id, category.id).map((bill) => ({
...bill, ...bill,
active: !!bill.active, active: !!bill.active,
payment_count: Number(bill.payment_count || 0), payment_count: Number(bill.payment_count || 0),
total_paid: Number(bill.total_paid || 0), total_paid: Number(bill.total_paid || 0),
last_paid_date: bill.last_paid_date || null, last_paid_date: bill.last_paid_date || null,
})); }));
const activeBillCount = bills.filter((bill) => bill.active).length; const activeBillCount = bills.filter((bill) => bill.active).length;
const inactiveBillCount = bills.length - activeBillCount; const inactiveBillCount = bills.length - activeBillCount;
const paymentCount = bills.reduce((sum, bill) => sum + bill.payment_count, 0); const paymentCount = bills.reduce((sum, bill) => sum + bill.payment_count, 0);
return { return {
...category, ...category,
spending_enabled: !!category.spending_enabled, spending_enabled: !!category.spending_enabled,
bill_count: activeBillCount, bill_count: activeBillCount,
active_bill_count: activeBillCount, active_bill_count: activeBillCount,
inactive_bill_count: inactiveBillCount, inactive_bill_count: inactiveBillCount,
payment_count: paymentCount, payment_count: paymentCount,
bill_names: bills.map((bill) => bill.name), bill_names: bills.map((bill) => bill.name),
bills, bills,
}; };
}); });
res.json(shaped); res.json(shaped);
} catch (err) {
console.error('[categories GET]', err.message);
res.status(500).json({ error: 'Failed to load categories' });
}
}); });
// PUT /api/categories/reorder // PUT /api/categories/reorder
@ -90,11 +92,7 @@ router.put('/reorder', (req: Req, res: Res) => {
})); }));
if (entries.length === 0) { if (entries.length === 0) {
return res throw ValidationError('At least one category order is required', 'reorder');
.status(400)
.json(
standardizeError('At least one category order is required', 'VALIDATION_ERROR', 'reorder'),
);
} }
const invalid = entries.find( const invalid = entries.find(
@ -105,15 +103,10 @@ router.put('/reorder', (req: Req, res: Res) => {
sortOrder < 0, sortOrder < 0,
); );
if (invalid) { if (invalid) {
return res throw ValidationError(
.status(400) 'Reorder payload must map category ids to non-negative integer positions',
.json( 'reorder',
standardizeError( );
'Reorder payload must map category ids to non-negative integer positions',
'VALIDATION_ERROR',
'reorder',
),
);
} }
const ids = entries.map((item) => item.categoryId); const ids = entries.map((item) => item.categoryId);
@ -128,9 +121,7 @@ router.put('/reorder', (req: Req, res: Res) => {
) )
.all(req.user.id, ...ids); .all(req.user.id, ...ids);
if (owned.length !== ids.length) { if (owned.length !== ids.length) {
return res throw NotFoundError('One or more categories were not found', 'category_id');
.status(404)
.json(standardizeError('One or more categories were not found', 'NOT_FOUND', 'category_id'));
} }
const update = db.prepare( const update = db.prepare(
@ -147,8 +138,7 @@ router.put('/reorder', (req: Req, res: Res) => {
router.post('/', (req: Req, res: Res) => { router.post('/', (req: Req, res: Res) => {
const db = getDb(); const db = getDb();
const { name } = req.body; const { name } = req.body;
if (!name) if (!name) throw ValidationError('name is required', 'name');
return res.status(400).json(standardizeError('name is required', 'VALIDATION_ERROR', 'name'));
try { try {
const result = db const result = db
@ -157,10 +147,7 @@ router.post('/', (req: Req, res: Res) => {
const created = db.prepare('SELECT * FROM categories WHERE id = ?').get(result.lastInsertRowid); const created = db.prepare('SELECT * FROM categories WHERE id = ?').get(result.lastInsertRowid);
res.status(201).json(created); res.status(201).json(created);
} catch (e) { } catch (e) {
if (e.message.includes('UNIQUE')) { asConflict(e, 'Category already exists', 'name');
return res.status(409).json(standardizeError('Category already exists', 'CONFLICT', 'name'));
}
throw e;
} }
}); });
@ -168,22 +155,18 @@ router.post('/', (req: Req, res: Res) => {
router.put('/:id', (req: Req, res: Res) => { router.put('/:id', (req: Req, res: Res) => {
const db = getDb(); const db = getDb();
const { name, spending_enabled, group_id } = req.body; const { name, spending_enabled, group_id } = req.body;
if (!name) if (!name) throw ValidationError('name is required', 'name');
return res.status(400).json(standardizeError('name is required', 'VALIDATION_ERROR', 'name'));
const cat = db const cat = db
.prepare('SELECT id FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL') .prepare('SELECT id FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL')
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
if (!cat) return res.status(404).json(standardizeError('Category not found', 'NOT_FOUND', 'id')); if (!cat) throw NotFoundError('Category not found', 'id');
if (group_id !== undefined && group_id !== null) { if (group_id !== undefined && group_id !== null) {
const group = db const group = db
.prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?') .prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?')
.get(group_id, req.user.id); .get(group_id, req.user.id);
if (!group) if (!group) throw NotFoundError('Category group not found', 'group_id');
return res
.status(404)
.json(standardizeError('Category group not found', 'NOT_FOUND', 'group_id'));
} }
try { try {
@ -209,11 +192,7 @@ router.put('/:id', (req: Req, res: Res) => {
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
res.json({ ...updated, spending_enabled: !!updated.spending_enabled }); res.json({ ...updated, spending_enabled: !!updated.spending_enabled });
} catch (e) { } catch (e) {
if (e.message?.includes('UNIQUE')) { asConflict(e, 'Category already exists', 'name');
return res.status(409).json(standardizeError('Category already exists', 'CONFLICT', 'name'));
}
console.error('[categories PUT]', e.message);
res.status(500).json({ error: 'Failed to update category' });
} }
}); });
@ -239,8 +218,7 @@ router.get('/groups', (req: Req, res: Res) => {
router.post('/groups', (req: Req, res: Res) => { router.post('/groups', (req: Req, res: Res) => {
const db = getDb(); const db = getDb();
const { name } = req.body; const { name } = req.body;
if (!name?.trim()) if (!name?.trim()) throw ValidationError('name is required', 'name');
return res.status(400).json(standardizeError('name is required', 'VALIDATION_ERROR', 'name'));
try { try {
const maxOrder = db const maxOrder = db
@ -256,13 +234,7 @@ router.post('/groups', (req: Req, res: Res) => {
.get(result.lastInsertRowid); .get(result.lastInsertRowid);
res.status(201).json(created); res.status(201).json(created);
} catch (e) { } catch (e) {
if (e.message?.includes('UNIQUE')) { asConflict(e, 'Category group already exists', 'name');
return res
.status(409)
.json(standardizeError('Category group already exists', 'CONFLICT', 'name'));
}
console.error('[category-groups POST]', e.message);
res.status(500).json({ error: 'Failed to create category group' });
} }
}); });
@ -274,17 +246,13 @@ router.put('/groups/:id', (req: Req, res: Res) => {
const group = db const group = db
.prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?') .prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?')
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
if (!group) if (!group) throw NotFoundError('Category group not found', 'id');
return res.status(404).json(standardizeError('Category group not found', 'NOT_FOUND', 'id'));
try { try {
const fields = ["updated_at = datetime('now')"]; const fields = ["updated_at = datetime('now')"];
const values = []; const values = [];
if (name !== undefined) { if (name !== undefined) {
if (!name.trim()) if (!name.trim()) throw ValidationError('name is required', 'name');
return res
.status(400)
.json(standardizeError('name is required', 'VALIDATION_ERROR', 'name'));
fields.push('name = ?'); fields.push('name = ?');
values.push(name.trim()); values.push(name.trim());
} }
@ -304,13 +272,7 @@ router.put('/groups/:id', (req: Req, res: Res) => {
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
res.json(updated); res.json(updated);
} catch (e) { } catch (e) {
if (e.message?.includes('UNIQUE')) { asConflict(e, 'Category group already exists', 'name');
return res
.status(409)
.json(standardizeError('Category group already exists', 'CONFLICT', 'name'));
}
console.error('[category-groups PUT]', e.message);
res.status(500).json({ error: 'Failed to update category group' });
} }
}); });
@ -320,8 +282,7 @@ router.delete('/groups/:id', (req: Req, res: Res) => {
const group = db const group = db
.prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?') .prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?')
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
if (!group) if (!group) throw NotFoundError('Category group not found', 'id');
return res.status(404).json(standardizeError('Category group not found', 'NOT_FOUND', 'id'));
db.prepare('DELETE FROM category_groups WHERE id = ? AND user_id = ?').run( db.prepare('DELETE FROM category_groups WHERE id = ? AND user_id = ?').run(
req.params.id, req.params.id,
@ -338,19 +299,14 @@ router.patch('/:id/spending', (req: Req, res: Res) => {
'SELECT id, spending_enabled FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL', 'SELECT id, spending_enabled FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL',
) )
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
if (!cat) return res.status(404).json(standardizeError('Category not found', 'NOT_FOUND', 'id')); if (!cat) throw NotFoundError('Category not found', 'id');
const enabled = const enabled =
req.body?.spending_enabled !== undefined ? !!req.body.spending_enabled : !cat.spending_enabled; req.body?.spending_enabled !== undefined ? !!req.body.spending_enabled : !cat.spending_enabled;
try { db.prepare(
db.prepare( "UPDATE categories SET spending_enabled=?, updated_at=datetime('now') WHERE id=? AND user_id=?",
"UPDATE categories SET spending_enabled=?, updated_at=datetime('now') WHERE id=? AND user_id=?", ).run(enabled ? 1 : 0, req.params.id, req.user.id);
).run(enabled ? 1 : 0, req.params.id, req.user.id); res.json({ id: cat.id, spending_enabled: enabled });
res.json({ id: cat.id, spending_enabled: enabled });
} catch (err) {
console.error('[categories PATCH spending]', err.message);
res.status(500).json({ error: 'Failed to update category' });
}
}); });
// DELETE /api/categories/:id // DELETE /api/categories/:id
@ -359,7 +315,7 @@ router.delete('/:id', (req: Req, res: Res) => {
const cat = db const cat = db
.prepare('SELECT id, name FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL') .prepare('SELECT id, name FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL')
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
if (!cat) return res.status(404).json(standardizeError('Category not found', 'NOT_FOUND', 'id')); if (!cat) throw NotFoundError('Category not found', 'id');
const deleted = db const deleted = db
.prepare( .prepare(
@ -384,8 +340,7 @@ router.post('/:id/restore', (req: Req, res: Res) => {
'SELECT id, name FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NOT NULL', 'SELECT id, name FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NOT NULL',
) )
.get(req.params.id, req.user.id); .get(req.params.id, req.user.id);
if (!cat) if (!cat) throw NotFoundError('Deleted category not found', 'id');
return res.status(404).json(standardizeError('Deleted category not found', 'NOT_FOUND', 'id'));
db.prepare( db.prepare(
"UPDATE categories SET deleted_at = NULL, updated_at = datetime('now') WHERE id = ? AND user_id = ?", "UPDATE categories SET deleted_at = NULL, updated_at = datetime('now') WHERE id = ? AND user_id = ?",

View File

@ -54,10 +54,24 @@ function callCategoriesRoute(routePath, method, { userId, params = {}, body = {}
resolve({ status: this.statusCode, data }); resolve({ status: this.statusCode, data });
}, },
}; };
// Simulate the terminal error handler: a thrown ApiError becomes a formatted
// response (mirrors utils/apiError formatError + server.cts).
const onError = (err) => {
resolve({
status: err.status || 500,
data: {
error: err.code || 'INTERNAL_ERROR',
message: err.message,
code: err.code || 'INTERNAL_ERROR',
field: err.field ?? null,
},
});
};
try { try {
handler(req, res); const maybe = handler(req, res);
if (maybe && typeof maybe.then === 'function') maybe.catch(onError);
} catch (err) { } catch (err) {
reject(err); onError(err);
} }
}); });
} }

View File

@ -55,10 +55,24 @@ function callCategoriesRoute(routePath, method, { userId, params = {}, body = {}
resolve({ status: this.statusCode, data }); resolve({ status: this.statusCode, data });
}, },
}; };
// Simulate the terminal error handler: a thrown ApiError becomes a formatted
// response (mirrors utils/apiError formatError + server.cts).
const onError = (err) => {
resolve({
status: err.status || 500,
data: {
error: err.code || 'INTERNAL_ERROR',
message: err.message,
code: err.code || 'INTERNAL_ERROR',
field: err.field ?? null,
},
});
};
try { try {
handler(req, res); const maybe = handler(req, res);
if (maybe && typeof maybe.then === 'function') maybe.catch(onError);
} catch (err) { } catch (err) {
reject(err); onError(err);
} }
}); });
} }

View File

@ -56,15 +56,19 @@ function ForbiddenError(message = 'Access denied', code = 'FORBIDDEN'): ApiError
/** /**
* Create a standardized not found error * Create a standardized not found error
*/ */
function NotFoundError(message = 'Resource not found', code = 'NOT_FOUND'): ApiError { function NotFoundError(
return new ApiError(code, message, 404); message = 'Resource not found',
field?: string,
code = 'NOT_FOUND',
): ApiError {
return new ApiError(code, message, 404, { field });
} }
/** /**
* Create a standardized conflict error * Create a standardized conflict error
*/ */
function ConflictError(message = 'Resource conflict', code = 'CONFLICT'): ApiError { function ConflictError(message = 'Resource conflict', field?: string, code = 'CONFLICT'): ApiError {
return new ApiError(code, message, 409); return new ApiError(code, message, 409, { field });
} }
/** /**