fix(resilience): client fetch timeout + global-handler headersSent guard (Phase 1b)

Client: _fetch had no timeout — a hung request (server accepted the
connection but never responds) spun the UI forever with no error. Now
wrapped in AbortSignal.timeout(120s, generous enough for the slowest
legit import/sync/backup on a LAN) and both timeout and network failure
map to a clean ApiError (REQUEST_TIMEOUT / NETWORK_ERROR) so they surface
as a normal error toast instead of a freeze or raw 'Failed to fetch'.

Server: the global error handler now returns next(err) when res.headersSent,
so an error after a partial response can't throw 'cannot set headers'.

(The async-handler-hang gap is folded into the Express 5 upgrade in Phase
2.2, which forwards async rejections natively — no throwaway wrapper.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
null 2026-07-05 13:10:00 -05:00
parent 896dfd8230
commit 398caec29d
2 changed files with 23 additions and 1 deletions

View File

@ -48,6 +48,12 @@ const MUTATING_METHODS = ['POST', 'PUT', 'DELETE', 'PATCH'];
// Parse a response body without assuming it is JSON. Returns null when the // Parse a response body without assuming it is JSON. Returns null when the
// body is empty (204) or not valid JSON (e.g. an HTML error page from a proxy). // body is empty (204) or not valid JSON (e.g. an HTML error page from a proxy).
// A hung request (server accepted the connection but never responds) would
// otherwise spin the UI forever. 120s is generous enough for the slowest
// legitimate op (large import / bank sync / backup) on a self-hosted LAN while
// still bounding a truly dead request so it surfaces as an error, not a freeze.
const REQUEST_TIMEOUT_MS = 120_000;
async function parseJsonSafe(res: Response): Promise<any> { async function parseJsonSafe(res: Response): Promise<any> {
if (res.status === 204) return null; if (res.status === 204) return null;
const text = await res.text(); const text = await res.text();
@ -66,7 +72,20 @@ async function _fetch<T = unknown>(method: string, path: string, body?: unknown,
} }
} }
if (body !== undefined) opts.body = JSON.stringify(body); if (body !== undefined) opts.body = JSON.stringify(body);
const res = await fetch('/api' + path, opts); let res: Response;
try {
res = await fetch('/api' + path, { ...opts, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS) });
} catch (e) {
// Timeout (AbortSignal.timeout → TimeoutError) or network failure (TypeError)
// — both would otherwise leave the UI hung or show a raw "Failed to fetch".
const timedOut = (e as Error)?.name === 'TimeoutError';
const err = new Error(timedOut
? 'The request timed out — the server may be busy or unreachable. Please try again.'
: 'Network error — could not reach the server. Check your connection and try again.') as ApiError;
err.status = timedOut ? 408 : 0;
err.code = timedOut ? 'REQUEST_TIMEOUT' : 'NETWORK_ERROR';
throw err;
}
const data = await parseJsonSafe(res); const data = await parseJsonSafe(res);
if (!res.ok) { if (!res.ok) {
// Stale CSRF token (cookie rotated/expired since first fetch): refresh the // Stale CSRF token (cookie rotated/expired since first fetch): refresh the

View File

@ -180,6 +180,9 @@ app.get('*', (req, res) => {
// ── Global error handler ────────────────────────────────────────────────────── // ── Global error handler ──────────────────────────────────────────────────────
// Never expose stack traces, internal paths, or raw error objects in responses. // Never expose stack traces, internal paths, or raw error objects in responses.
app.use((err, req, res, next) => { app.use((err, req, res, next) => {
// If a response was already (partially) sent, we can't write a new body/status —
// delegate to Express's default handler, which closes the connection cleanly.
if (res.headersSent) return next(err);
recordError('Express', err); recordError('Express', err);
console.error('[error]', err.message || String(err)); console.error('[error]', err.message || String(err));