feat(standards): enforcement ratchets — error-shape + no-console; client toApiError
- 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 <noreply@anthropic.com>
This commit is contained in:
parent
c481a5bdbf
commit
68ca7cbb43
|
|
@ -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<T = unknown>(
|
|||
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;
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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**
|
||||
|
|
|
|||
|
|
@ -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.',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue