From 68ca7cbb43b63311f8b1558a5079cbd34d241ccd Mon Sep 17 00:00:00 2001 From: null Date: Fri, 10 Jul 2026 17:20:42 -0500 Subject: [PATCH] =?UTF-8?q?feat(standards):=20enforcement=20ratchets=20?= =?UTF-8?q?=E2=80=94=20error-shape=20+=20no-console;=20client=20toApiError?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - eslint: no-console error on all server dirs (sanctioned exceptions: utils/logger.cts sink, utils/env.cts bootstrap, utils/apiError.cts fallback); no-restricted-imports bans errorFormatter in routes/** so hand-rolled standardizeError bodies can't return (transactions/import are the two documented exception files) - client/api.ts: 6 copy-pasted ApiError constructions collapsed into one toApiError() helper - CODE_STANDARDS.md: 'Error responses' and 'Logging' flipped target -> ENFORCED, with the full documented-exceptions list and the client error-shape/mutation-pattern conventions written down Lint 0 errors, client 52/52, typecheck clean. Co-Authored-By: Claude Fable 5 --- client/api.ts | 55 +++++++++++++++--------------------------- docs/CODE_STANDARDS.md | 31 +++++++++++++++++------- eslint.config.mjs | 33 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 45 deletions(-) diff --git a/client/api.ts b/client/api.ts index 4bd386f..6675833 100644 --- a/client/api.ts +++ b/client/api.ts @@ -51,6 +51,19 @@ export interface ApiError extends Error { code?: string; } +/** Build the ApiError every non-ok response throws — one shape, one place. */ +function toApiError( + res: Response, + data: { message?: string; error?: string; code?: string; details?: unknown[] } | null, +): ApiError { + const err = new Error(data?.message || data?.error || `HTTP ${res.status}`) as ApiError; + err.status = res.status; + err.data = data || {}; + err.details = data?.details || []; + err.code = data?.code; + return err; +} + /** Result of toggling a tracker row paid/unpaid. */ export interface TogglePaidResult { paymentId?: number; @@ -141,12 +154,7 @@ async function _fetch( if (res.status === 401 && !path.startsWith('/auth/')) { window.dispatchEvent(new Event('auth:expired')); } - const err = new Error(data?.message || data?.error || `HTTP ${res.status}`) as ApiError; - err.status = res.status; - err.data = data || {}; - err.details = data?.details || []; - err.code = data?.code; - throw err; + throw toApiError(res, data); } return (data ?? {}) as T; } @@ -269,12 +277,7 @@ export const api = { }); const data = await res.json(); if (!res.ok) { - const err = new Error(data.message || data.error || `HTTP ${res.status}`) as ApiError; - err.status = res.status; - err.data = data; - err.details = data.details || []; - err.code = data.code; - throw err; + throw toApiError(res, data); } return data; }, @@ -511,12 +514,7 @@ export const api = { }); const data = await res.json(); if (!res.ok) { - const err = new Error(data.message || data.error || `HTTP ${res.status}`) as ApiError; - err.status = res.status; - err.data = data; - err.details = data.details || []; - err.code = data.code; - throw err; + throw toApiError(res, data); } return data; }, @@ -535,12 +533,7 @@ export const api = { }); const data = await res.json(); if (!res.ok) { - const err = new Error(data.message || data.error || `HTTP ${res.status}`) as ApiError; - err.status = res.status; - err.data = data; - err.details = data.details || []; - err.code = data.code; - throw err; + throw toApiError(res, data); } return data; }, @@ -559,12 +552,7 @@ export const api = { }); const data = await res.json(); if (!res.ok) { - const err = new Error(data.message || data.error || `HTTP ${res.status}`) as ApiError; - err.status = res.status; - err.data = data; - err.details = data.details || []; - err.code = data.code; - throw err; + throw toApiError(res, data); } return data; }, @@ -628,12 +616,7 @@ export const api = { }); const data = await res.json(); if (!res.ok) { - const err = new Error(data.message || data.error || `HTTP ${res.status}`) as ApiError; - err.status = res.status; - err.data = data; - err.details = data.details || []; - err.code = data.code; - throw err; + throw toApiError(res, data); } return data; }, diff --git a/docs/CODE_STANDARDS.md b/docs/CODE_STANDARDS.md index f9110e2..e510891 100644 --- a/docs/CODE_STANDARDS.md +++ b/docs/CODE_STANDARDS.md @@ -33,15 +33,28 @@ rolled out. Update this doc when a standard changes. - **Wire format:** JSON, **snake_case** field names (mirrors DB columns), money as integer cents. -- **Error responses (target):** every error body is `{ error, message, code, field? }` with +- **Error responses (enforced):** every error body is `{ error, message, code, field? }` with the correct HTTP status. Throw the factories from `utils/apiError.cts` - (`ValidationError`, `NotFoundError`, `ConflictError`, …) and let the single terminal error - handler format them — do not hand-roll `res.status().json({ error })`. Express 5 forwards - rejected async handlers automatically, so route bodies should not wrap everything in - `try/catch`. Clients read `message`/`code` + status. **Canonical example:** - `routes/categories.cts`. (The terminal handler already normalizes any legacy - `standardizeError`/`{ error }` body to the same shape, so converted and not-yet-converted - routes emit identical responses — convert opportunistically.) + (`ValidationError`, `NotFoundError`, `ConflictError`, `AuthError`, `ForbiddenError`, or + `new ApiError(code, message, status, { field })` for anything else) and let the single + terminal error handler format them — do not hand-roll `res.status().json({ error })`. + Express 5 forwards rejected async handlers automatically, so route bodies do not wrap + everything in `try/catch`; a thrown `ApiError` keeps its message on the wire while any + other 5xx is masked + logged by the terminal handler (that masking is the whitelist: + wrap a message in `ApiError` only when it is deliberately user-facing). Clients read + `message`/`code` + status. **Canonical example:** `routes/categories.cts`. + **Enforced by:** the eslint `no-restricted-imports` ratchet on `routes/**` (all 29 route + files converted 2026-07). **Documented exceptions** (each commented at the site): + `routes/transactions.cts` (internal `{ error }` parse-result convention, rethrown at the + boundary via `throwStandardized`), `routes/import.cts` (`sendImportError` error-id + envelope), `routes/admin.cts` `sendError` (backup download can fail mid-stream after + headers are sent), payments' `DUPLICATE_SUSPECTED` 409 and transactions' bulk-unmatch 500 + (both carry extra payload fields the client renders), `routes/calendarFeed.cts` + (text/plain for ICS clients). +- **Client error shape:** `client/api.ts` builds every thrown error through `toApiError()` — + one construction site; on a 401 outside `/auth/*` it dispatches `auth:expired` so + `AuthProvider`/`RequireAuth` redirect to login. Mutations follow the React Query patterns + in `client/hooks/useQueries.ts` / `usePaymentActions.ts` (invalidate + toast on error). - **Success envelope (target):** reads return the bare resource/list; mutations return `{ success: true, … }`. - **Validation (target):** validate inputs with the shared validators (throwing @@ -56,7 +69,7 @@ rolled out. Update this doc when a standard changes. - Never log secrets, tokens, or raw financial values. - Fail fast at boot on missing required config (e.g. `TOKEN_ENCRYPTION_KEY` in production). -## Logging (target) +## Logging (enforced) - Use `utils/logger.cts` (`const { log } = require('.../utils/logger.cts')`) — `log.debug/info/warn/error`, console-compatible. Level via `LOG_LEVEL` (default: debug in dev, info in prod). It **redacts** diff --git a/eslint.config.mjs b/eslint.config.mjs index 909d88f..c856629 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -51,6 +51,39 @@ export default [ 'no-irregular-whitespace': 'warn', // CSV/import parsers handle exotic whitespace 'no-misleading-character-class': 'warn', 'no-extra-boolean-cast': 'warn', + // Pillar D ratchet: server code logs through utils/logger.cts only. + // The two sanctioned exceptions (logger itself + env bootstrap, which runs + // before the logger exists) are carved out in the override block below. + 'no-console': 'error', + }, + }, + { + // Sanctioned console users: the logger's own sink and fatal-config + // bootstrap output that must work before the logger is importable. + files: ['utils/logger.cts', 'utils/env.cts', 'utils/apiError.cts'], + rules: { 'no-console': 'off' }, + }, + { + // Error-shape ratchet (CODE_STANDARDS "Error responses", now enforced): + // routes throw utils/apiError.cts factories; hand-rolled standardizeError + // bodies can't come back. transactions.cts (internal {error} result + // convention rethrown at the boundary) and import.cts (sendImportError + // error-id envelope) are the two documented exceptions. + files: ['routes/**/*.cts'], + ignores: ['routes/transactions.cts', 'routes/import.cts'], + rules: { + 'no-restricted-imports': [ + 'error', + { + paths: [ + { + name: '../middleware/errorFormatter.cts', + message: + 'Routes emit errors by throwing utils/apiError.cts factories (see CODE_STANDARDS.md); the terminal handler formats them.', + }, + ], + }, + ], }, }, {