From 765dfa3bafda68ad3256076ad89182370fc1a813 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 05:14:24 +0000 Subject: [PATCH] fix(rest): route expected 4xx through one shared handleRouteError, not "[REST] Unhandled error" (#4886) The metadata routes had 29 catch blocks logging every thrown error unconditionally. Studio's designer probes GET /meta/:type/:name?state=draft on every panel, and "no draft exists" is the overwhelmingly common answer, so the structured { code: 'NO_DRAFT', status: 404 } printed a full stack trace per panel -- 45 in one browsing session, burying real errors and misreporting severity. The wire answer was already a correct, clean 404. The data routes already consulted isExpectedDataStatus / isExpectedQueryRejection but in four open-coded spellings across 12 sites, and the latter's docblock records an earlier lap of the same drift (the filter and sort codes shipped without joining the list). Both families now decide through one predicate behind one door: - resolveErrorResponse() -- split out of sendError so the logging decision reads the exact status/body the client gets, not a second opinion that can drift - isExpectedRouteError() -- the union of the three conditions: expected lifecycle statuses, the client-caused 400 query-rejection vocabulary, and VALIDATION_FAILED. Deliberately NOT "any 4xx": mapDataError degrades an unrecognised error to an un-coded 400, which is where a real handler bug lands, so that stays loud. - handleRouteError() -- resolve once, log only genuine faults, then send - logUnexpectedRouteError() -- the verdict alone, for the CRUD catches that must keep their own responder (one rewrites 400 to 404 on the wire) isExpectedDataStatus and isExpectedQueryRejection now have no other callers, so the families cannot drift apart again. No wire responses change; this only decides whether the log line is printed. Two operator-visible log deltas beyond the metadata fix: the transactional batch route judged on status >= 500 alone and so swallowed the un-coded 400 (a handler TypeError inside a batch transaction used to vanish, and now prints), and updateMany/deleteMany/clone/global-search/public-form stop logging normal 404s, 403s and query rejections. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VkPSGsX9o17MsGv3Lbxu2w --- ...st-expected-4xx-not-logged-as-unhandled.md | 51 ++++ .../src/rest-expected-error-logging.test.ts | 233 ++++++++++++++++++ packages/rest/src/rest-server.ts | 228 +++++++++-------- 3 files changed, 414 insertions(+), 98 deletions(-) create mode 100644 .changeset/rest-expected-4xx-not-logged-as-unhandled.md create mode 100644 packages/rest/src/rest-expected-error-logging.test.ts diff --git a/.changeset/rest-expected-4xx-not-logged-as-unhandled.md b/.changeset/rest-expected-4xx-not-logged-as-unhandled.md new file mode 100644 index 0000000000..b9be012f08 --- /dev/null +++ b/.changeset/rest-expected-4xx-not-logged-as-unhandled.md @@ -0,0 +1,51 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): expected 4xx no longer logged as "[REST] Unhandled error" with a stack (#4886) + +Opening Studio flooded the server log with stack traces. The designer probes +`GET /meta/:type/:name?state=draft` on every panel to decide whether to show +"unsaved draft" state, and "no draft exists" is the overwhelmingly common +answer — true of every artifact nobody is currently editing. `getMetaItem` +throws a structured `{ code: 'NO_DRAFT', status: 404 }`, the client got a clean +404 and handled it fine, but the route logged it anyway: + +``` +[REST] Unhandled error: Error: [no_draft] No pending draft exists for app/showcase_app. + at _ObjectStackProtocolImplementation.getMetaItem (…) { code: 'NO_DRAFT', status: 404 } +``` + +**45 of these in one browsing session** — by far the dominant entry in the log, +which is how a genuine 500 goes unnoticed, and it misreports severity: nothing +was broken. + +The metadata routes had 29 catch blocks logging unconditionally. The data +routes already consulted `isExpectedDataStatus` / `isExpectedQueryRejection` — +but in four different open-coded spellings across 12 sites, and +`isExpectedQueryRejection`'s docblock records an earlier lap of exactly this +drift (the filter and sort codes shipped without joining the list, so every +rejection they produced was logged as unhandled too). + +Both families now decide through one predicate behind one door, +`handleRouteError(res, error, object?)`: it resolves the response once — the +same structured-status passthrough or `mapDataError` envelope `sendError` +already produced — logs only when that resolved response is a genuine fault, +then sends it. `isExpectedDataStatus` and `isExpectedQueryRejection` have no +other callers left, so the two families cannot drift apart again. + +Expected now means an explicitly recognised client or lifecycle outcome: +403/404/409/502/503, the client-caused 400 query-rejection vocabulary, and +`VALIDATION_FAILED`. It deliberately does **not** mean "any 4xx" — +`mapDataError` degrades an error it recognised nothing about to an un-coded +400, and that bucket is where a real handler bug lands, so it stays loud. + +**No wire responses change** — every status and body is byte-for-byte what it +was; this only decides whether the log line is printed. Two operator-visible +log deltas beyond the metadata fix: + +- the cross-object transactional batch route judged on `status >= 500` alone, + which also swallowed that un-coded 400 — a handler `TypeError` inside a batch + transaction used to vanish, and now prints; +- `updateMany` / `deleteMany` / clone / global search / the public-form routes + stop logging normal 404s, 403s and query rejections. diff --git a/packages/rest/src/rest-expected-error-logging.test.ts b/packages/rest/src/rest-expected-error-logging.test.ts new file mode 100644 index 0000000000..6e34d8070e --- /dev/null +++ b/packages/rest/src/rest-expected-error-logging.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#4886] Expected 4xx must not be logged as "[REST] Unhandled error". +// +// The metadata routes logged EVERY thrown error unconditionally — 29 catch +// blocks doing `logError("[REST] Unhandled error:", error); sendError(...)`. +// Studio's designer probes `GET /meta/:type/:name?state=draft` on every panel +// to decide whether to show "unsaved draft" state, and "no draft exists" is the +// overwhelmingly common answer, so `getMetaItem` throwing its structured +// `{ code: 'NO_DRAFT', status: 404 }` printed a full stack trace per panel — +// 45 in one browsing session. The wire answer was always a correct, clean 404; +// only the logging was wrong. +// +// The data routes already consulted `isExpectedDataStatus` / +// `isExpectedQueryRejection` — but in four different open-coded spellings, and +// `isExpectedQueryRejection`'s docblock records an earlier lap of the same +// drift (the filter and sort codes shipped without joining the list). Both +// families now decide through ONE predicate behind ONE door +// (`handleRouteError`), which is what these tests pin. +// +// Both directions are pinned deliberately, and they are NOT symmetric: +// - the "quiet" tests go RED if the fix is reverted (the unconditional log +// comes back); +// - the "loud" tests stay GREEN under a revert — they exist to catch the +// OPPOSITE overreach, a predicate widened to "any 4xx is expected", which +// would silence the un-coded 400 that `mapDataError` degrades an +// unrecognised error (a handler `TypeError`) to. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { RestServer } from './rest-server'; + +const META_ITEM = '/api/v1/meta/:type/:name'; +const DATA_LIST = '/api/v1/data/:object'; + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.header = vi.fn(() => res); + res.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn(); res.send = vi.fn(); + return res; +} + +/** The exact error `metadata-protocol`'s `getMetaItem` throws for a draft probe. */ +function noDraftError(target: string) { + return Object.assign( + new Error(`[no_draft] No pending draft exists for ${target}.`), + { code: 'NO_DRAFT', status: 404 }, + ); +} + +function setup(protocolOverrides: Record = {}) { + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ + version: 'v0', endpoints: { data: '', metadata: '', ui: '', auth: '/auth' }, + }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue([{ name: 'showcase_account' }]), + getMetaItem: vi.fn().mockResolvedValue({}), + findData: vi.fn().mockResolvedValue([]), + ...protocolOverrides, + }; + const rest = new RestServer( + createMockServer() as any, + protocol, + { api: { requireAuth: false } } as any, + ); + // A resolved session — meta routes are behind an unconditional auth gate. + (rest as any).resolveExecCtx = async () => ({ userId: 'u1' }); + rest.registerRoutes(); + return { rest, protocol }; +} + +function findRoute(rest: any, method: string, path: string) { + const route = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + if (!route) throw new Error(`${method} ${path} route not registered`); + return route; +} + +async function callMetaItem(rest: any, params: any, query: any = {}) { + const res = makeRes(); + await findRoute(rest, 'GET', META_ITEM).handler( + { method: 'GET', params, query, headers: {} }, res, + ); + return res; +} + +async function callDataList(rest: any, object: string) { + const res = makeRes(); + await findRoute(rest, 'GET', DATA_LIST).handler( + { method: 'GET', params: { object }, query: {}, headers: {} }, res, + ); + return res; +} + +let errorSpy: ReturnType; + +/** Only the "[REST] Unhandled error" channel — other console.error noise is not this test's business. */ +const unhandledLogs = () => errorSpy.mock.calls.filter((c) => c[0] === '[REST] Unhandled error:'); + +beforeEach(() => { errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); }); +afterEach(() => { errorSpy.mockRestore(); }); + +describe('metadata routes — expected 4xx respond without an "Unhandled error" log (#4886)', () => { + it('NO_DRAFT from the designer draft probe logs NOTHING and still 404s cleanly', async () => { + const { rest } = setup({ + getMetaItem: vi.fn().mockRejectedValue(noDraftError('app/showcase_app')), + }); + + const res = await callMetaItem(rest, { type: 'app', name: 'showcase_app' }, { state: 'draft' }); + + // The whole point: no stack trace for the overwhelmingly common answer. + expect(unhandledLogs()).toHaveLength(0); + // ...and the wire answer is byte-for-byte what it always was. + expect(res.statusCode).toBe(404); + expect(res.body).toEqual({ + error: '[no_draft] No pending draft exists for app/showcase_app.', + code: 'NO_DRAFT', + }); + }); + + it('stays quiet across the sibling expected statuses, not just 404', async () => { + // 403 RBAC denial / 409 conflict / 503 provisioning are all normal + // outcomes `isExpectedDataStatus` already named for the data family. + for (const status of [403, 404, 409, 502, 503]) { + const { rest } = setup({ + getMetaItem: vi.fn().mockRejectedValue( + Object.assign(new Error('expected'), { code: 'SOME_CODE', status }), + ), + }); + const res = await callMetaItem(rest, { type: 'object', name: 'showcase_account' }); + expect(res.statusCode).toBe(status); + } + expect(unhandledLogs()).toHaveLength(0); + }); + + it('a VALIDATION_FAILED 400 is also expected (client-caused, body already explains it)', async () => { + const { rest } = setup({ + getMetaItem: vi.fn().mockRejectedValue( + Object.assign(new Error('bad'), { code: 'VALIDATION_FAILED', status: 400 }), + ), + }); + + const res = await callMetaItem(rest, { type: 'object', name: 'showcase_account' }); + + expect(unhandledLogs()).toHaveLength(0); + expect(res.statusCode).toBe(400); + }); +}); + +describe('metadata routes — genuine faults keep the loud log (#4886)', () => { + it('a 500 still logs the full error object', async () => { + const boom = Object.assign(new Error('driver exploded'), { status: 500 }); + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(boom) }); + + const res = await callMetaItem(rest, { type: 'object', name: 'showcase_account' }); + + expect(unhandledLogs()).toHaveLength(1); + // The error itself is logged, not a summary — the stack is the point here. + expect(unhandledLogs()[0][1]).toBe(boom); + expect(res.statusCode).toBe(500); + }); + + it('an UNRECOGNISED error (handler bug) stays loud even though it maps to 400', async () => { + // This is the case a blanket "any 4xx is expected" predicate would + // wrongly silence: `mapDataError` degrades anything it recognises + // nothing about to an UN-CODED 400, and that is where a real handler + // bug lands. Silencing it would be the mirror-image of #4886. + const bug = new TypeError('Cannot read properties of undefined (reading \'name\')'); + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(bug) }); + + const res = await callMetaItem(rest, { type: 'object', name: 'showcase_account' }); + + expect(unhandledLogs()).toHaveLength(1); + expect(unhandledLogs()[0][1]).toBe(bug); + expect(res.statusCode).toBe(400); + expect(res.body?.code).toBeUndefined(); + }); +}); + +describe('both route families share ONE verdict — the anti-drift pin (#4886)', () => { + it('the same structured 404 is silent on a metadata route AND a data route', async () => { + const meta = setup({ getMetaItem: vi.fn().mockRejectedValue(noDraftError('object/showcase_account')) }); + const metaRes = await callMetaItem(meta.rest, { type: 'object', name: 'showcase_account' }); + const afterMeta = unhandledLogs().length; + + const data = setup({ findData: vi.fn().mockRejectedValue(noDraftError('object/showcase_account')) }); + const dataRes = await callDataList(data.rest, 'showcase_account'); + const afterData = unhandledLogs().length; + + expect(metaRes.statusCode).toBe(404); + expect(dataRes.statusCode).toBe(404); + expect(afterMeta).toBe(0); + expect(afterData).toBe(0); + }); + + it('the same unrecognised fault is loud on a metadata route AND a data route', async () => { + const bug = new TypeError('boom'); + + const meta = setup({ getMetaItem: vi.fn().mockRejectedValue(bug) }); + await callMetaItem(meta.rest, { type: 'object', name: 'showcase_account' }); + expect(unhandledLogs()).toHaveLength(1); + + const data = setup({ findData: vi.fn().mockRejectedValue(bug) }); + await callDataList(data.rest, 'showcase_account'); + expect(unhandledLogs()).toHaveLength(2); + }); + + it('a client-caused query rejection is silent on the data list route (the earlier drift lap)', async () => { + // `isExpectedQueryRejection`'s docblock: the filter and sort codes + // shipped WITHOUT joining the expected list, so every rejection they + // produced was ALSO logged as an unhandled error. Same shape, and now + // the same single predicate for every family. + const { rest } = setup({ + findData: vi.fn().mockRejectedValue( + Object.assign(new Error('Unknown filter operator'), { code: 'INVALID_FILTER', status: 400 }), + ), + }); + + const res = await callDataList(rest, 'showcase_account'); + + expect(unhandledLogs()).toHaveLength(0); + expect(res.statusCode).toBe(400); + expect(res.body?.code).toBe('INVALID_FILTER'); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index bef46e191c..a0b23d19d4 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -766,6 +766,18 @@ export function mapDataError(error: any, object?: string): { status: number; bod * uniformly across CRUD, batch, metadata, UI and discovery routes. */ function sendError(res: any, error: any, object?: string): void { + const { status, body } = resolveErrorResponse(error, object); + res.status(status).json(body); +} + +/** + * The wire response `sendError` would emit for a thrown route error, WITHOUT + * emitting it. Split out of `sendError` so the logging decision + * (`handleRouteError`) reads the exact status/body the client is about to get + * instead of forming a second opinion that can drift from the responder — the + * drift this whole seam exists to prevent (#4886). + */ +function resolveErrorResponse(error: any, object?: string): { status: number; body: Record } { // [#3770] `OBJECT_NOT_FOUND` is deliberately excluded from this // status-passthrough: `mapDataError` owns its canonical envelope // (`OBJECT_NOT_FOUND`), and short-circuiting here would ship a second wire @@ -776,15 +788,16 @@ function sendError(res: any, error: any, object?: string): void { const safeMsg = typeof error.message === 'string' && error.message.length < 500 ? error.message : 'Request failed'; - res.status(error.status).json({ - error: safeMsg, - ...(error.code ? { code: error.code } : {}), - ...(Array.isArray(error.issues) ? { issues: error.issues } : {}), - }); - return; + return { + status: error.status, + body: { + error: safeMsg, + ...(error.code ? { code: error.code } : {}), + ...(Array.isArray(error.issues) ? { issues: error.issues } : {}), + }, + }; } - const mapped = mapDataError(error, object); - res.status(mapped.status).json(mapped.body); + return mapDataError(error, object); } /** @@ -820,6 +833,61 @@ function isExpectedQueryRejection(body: Record | undefined): bo || body?.code === 'INVALID_QUERY'; } +/** + * THE predicate. Whether a resolved error response is an *expected* outcome — + * something the client caused or a normal lifecycle state — rather than a + * server fault worth an "[REST] Unhandled error" line plus a stack trace. + * + * The union of the three conditions the data routes had each open-coded: + * - `isExpectedDataStatus` — 403/404/409/502/503 lifecycle outcomes + * - `isExpectedQueryRejection` — the client-caused 400 vocabulary + * - `VALIDATION_FAILED` — the per-field 400 envelope + * + * It is deliberately NOT "any 4xx". `mapDataError`'s final fallback degrades an + * error it recognised nothing about to an un-coded 400, and that bucket is + * where a genuine handler bug (a `TypeError`, say) lands — silencing it would + * be the mirror-image of the defect this fixes. + * + * [#4886] Every route catch now decides through this one function. Before, the + * metadata family logged unconditionally — the designer's `?state=draft` probe + * made `NO_DRAFT` (a structured 404, and the overwhelmingly common answer for + * any artifact nobody is editing) print 45 stack traces in one browsing + * session — while the data family open-coded four different spellings of + * "expected" at 12 sites. `isExpectedQueryRejection`'s own docblock records the + * previous lap of exactly this drift: the filter and sort codes shipped without + * joining the list, so every rejection they produced was logged as an unhandled + * error too. One predicate, one door, so there is no third lap. + */ +function isExpectedRouteError(status: number, body: Record | undefined): boolean { + return isExpectedDataStatus(status) + || isExpectedQueryRejection(body) + || body?.code === 'VALIDATION_FAILED'; +} + +/** + * Log "[REST] Unhandled error" only when `resolved` is a genuine fault. For + * catch blocks that must emit their own response shape (the CRUD handlers that + * respond straight from a `mapDataError` envelope, one of which rewrites 400 → + * 404 on the wire) — they keep their responder and share only the verdict. + */ +function logUnexpectedRouteError(error: any, resolved: { status: number; body: Record }): void { + if (!isExpectedRouteError(resolved.status, resolved.body)) { + logError('[REST] Unhandled error:', error); + } +} + +/** + * The single door a route catch block should use: resolve the response once, + * log it only if it is a real fault, then send it. Wire behaviour is identical + * to a bare `sendError(res, error, object)` — this only decides whether the log + * line is printed. + */ +function handleRouteError(res: any, error: any, object?: string): void { + const resolved = resolveErrorResponse(error, object); + logUnexpectedRouteError(error, resolved); + res.status(resolved.status).json(resolved.body); +} + /** * [#3431] `X-ObjectStack-Dropped-Fields` — surface the engine's LEGAL write * strips (static `readonly` #2948 / TRUE `readonlyWhen` #3042 / #3043 create @@ -2661,8 +2729,7 @@ export class RestServer { res.json(discovery); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }; @@ -2997,8 +3064,7 @@ export class RestServer { res.header('Vary', 'Accept-Language'); res.json(translated); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3040,8 +3106,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3080,8 +3145,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3164,8 +3228,7 @@ export class RestServer { }); res.json(report); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3392,8 +3455,7 @@ export class RestServer { res.header('Vary', 'Accept-Language'); res.json(translated); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3426,8 +3488,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3525,8 +3586,7 @@ export class RestServer { } res.json(tree); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3760,8 +3820,7 @@ export class RestServer { res.json(await this.translateMetaItem(req, req.params.type, environmentId, visible)); } } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3840,8 +3899,7 @@ export class RestServer { } as any); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3901,8 +3959,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3945,8 +4002,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -3984,8 +4040,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -4023,8 +4078,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -4072,8 +4126,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -4113,8 +4166,7 @@ export class RestServer { }); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -4145,8 +4197,7 @@ export class RestServer { res.header('Vary', 'Accept-Language'); res.json(await this.translateMetaItem(req, req.params.type, environmentId, item)); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -4194,8 +4245,7 @@ export class RestServer { } as any); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error); + handleRouteError(res, error); } }, metadata: { @@ -4231,8 +4281,7 @@ export class RestServer { res.status(501).json({ error: 'UI View resolution not supported by protocol implementation', code: 'NOT_IMPLEMENTED' }); } } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error, req.params?.object); + handleRouteError(res, error, req.params?.object); } }, metadata: { @@ -4273,12 +4322,8 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (isExpectedDataStatus(mapped.status) || isExpectedQueryRejection(mapped.body)) { - res.status(mapped.status).json(mapped.body); - } else { - logError("[REST] Unhandled error:", error); - res.status(mapped.status).json(mapped.body); - } + logUnexpectedRouteError(error, mapped); + res.status(mapped.status).json(mapped.body); } }, metadata: { @@ -4312,7 +4357,7 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (!isExpectedDataStatus(mapped.status) && mapped.body?.code !== "VALIDATION_FAILED") logError("[REST] Unhandled error:", error); + logUnexpectedRouteError(error, mapped); res.status(mapped.status === 400 ? 404 : mapped.status).json(mapped.body); } }, @@ -4367,7 +4412,7 @@ export class RestServer { res.status(201).json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (!isExpectedDataStatus(mapped.status) && mapped.body?.code !== "VALIDATION_FAILED") logError("[REST] Unhandled error:", error); + logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } }, @@ -4429,9 +4474,7 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (!isExpectedDataStatus(mapped.status) && !isExpectedQueryRejection(mapped.body)) { - logError("[REST] Unhandled error:", error); - } + logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } }, @@ -4509,7 +4552,7 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (!isExpectedDataStatus(mapped.status) && mapped.body?.code !== "VALIDATION_FAILED") logError("[REST] Unhandled error:", error); + logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } }, @@ -4551,7 +4594,7 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (!isExpectedDataStatus(mapped.status) && mapped.body?.code !== "VALIDATION_FAILED") logError("[REST] Unhandled error:", error); + logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } }, @@ -4615,13 +4658,9 @@ export class RestServer { res.status(201).json(result); } catch (error: any) { // Clone's domain errors (CLONE_DISABLED/RECORD_NOT_FOUND) - // carry an explicit `.status`; fall back to mapDataError for - // driver/validation faults. Only log genuine server faults. - const status = typeof error?.status === 'number' - ? error.status - : mapDataError(error, req.params?.object).status; - if (!isExpectedDataStatus(status) && error?.code !== 'VALIDATION_FAILED') logError('[REST] Unhandled error:', error); - sendError(res, error, req.params?.object); + // carry an explicit `.status`; `handleRouteError` resolves + // that passthrough itself and logs only genuine faults. + handleRouteError(res, error, req.params?.object); } }, metadata: { @@ -4700,8 +4739,7 @@ export class RestServer { results: summary.results, }); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, String(req.params?.object || '')); + handleRouteError(res, error, String(req.params?.object || '')); } }, metadata: { @@ -4893,8 +4931,7 @@ export class RestServer { } })(); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, String(req.params?.object || '')); + handleRouteError(res, error, String(req.params?.object || '')); } }, metadata: { @@ -4929,8 +4966,7 @@ export class RestServer { } res.json({ success: true }); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, ''); + handleRouteError(res, error, ''); } }, metadata: { summary: 'Cancel an in-flight import job', tags: ['data', 'import'] }, @@ -5008,8 +5044,7 @@ export class RestServer { }); res.json({ success: true, jobId, object: objectName, deleted, restored, failed }); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, ''); + handleRouteError(res, error, ''); } }, metadata: { summary: 'Undo (logically roll back) a finished import job', tags: ['data', 'import'] }, @@ -5035,8 +5070,7 @@ export class RestServer { const items = Array.isArray(stored?.items) ? stored.items : Array.isArray(stored) ? stored : []; res.json({ ...importJobToProgress(row), results: items, resultsTruncated: !!stored?.truncated }); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, ''); + handleRouteError(res, error, ''); } }, metadata: { summary: 'Import job results (capped per-row report)', tags: ['data', 'import'] }, @@ -5060,8 +5094,7 @@ export class RestServer { } res.json(importJobToProgress(row)); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, ''); + handleRouteError(res, error, ''); } }, metadata: { summary: 'Import job progress', tags: ['data', 'import'] }, @@ -5095,8 +5128,7 @@ export class RestServer { : Array.isArray(r) ? r : []; res.json({ jobs: rows.map(importJobToSummary) }); } catch (error: any) { - logError('[REST] Unhandled error:', error); - sendError(res, error, ''); + handleRouteError(res, error, ''); } }, metadata: { summary: 'List import jobs (history)', tags: ['data', 'import'] }, @@ -5458,10 +5490,9 @@ export class RestServer { res.end(); } } catch (error: any) { - logError('[REST] Unhandled error:', error); // Best-effort error envelope; if headers already sent the // client receives a truncated stream which signals failure. - try { sendError(res, error, String(req.params?.object || '')); } + try { handleRouteError(res, error, String(req.params?.object || '')); } catch { try { res.end(); } catch { /* swallow */ } } } }, @@ -5536,9 +5567,7 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error); - if (!isExpectedDataStatus(mapped.status) && mapped.body?.code !== 'VALIDATION_FAILED') { - logError('[REST] Unhandled error:', error); - } + logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } }, @@ -5951,7 +5980,9 @@ export class RestServer { res.status(201).json(result); } catch (error: any) { const mapped = mapDataError(error); - if (!isExpectedDataStatus(mapped.status) && mapped.body?.code !== 'VALIDATION_FAILED') { + // Distinct message (this is not the "unhandled" channel), + // same shared verdict — see `isExpectedRouteError`. + if (!isExpectedRouteError(mapped.status, mapped.body)) { logError('[REST] Public form submit error:', error); } res.status(mapped.status).json(mapped.body); @@ -6099,7 +6130,9 @@ export class RestServer { }); } catch (error: any) { const mapped = mapDataError(error); - if (!isExpectedDataStatus(mapped.status)) { + // Distinct message (this is not the "unhandled" channel), + // same shared verdict — see `isExpectedRouteError`. + if (!isExpectedRouteError(mapped.status, mapped.body)) { logError('[REST] Public form lookup error:', error); } res.status(mapped.status).json(mapped.body); @@ -7651,9 +7684,12 @@ export class RestServer { } catch (error: any) { // Log only genuine server faults; client 4xx (validation, // unresolved ref, atomic rollback of a bad op) are expected. - const status = typeof error?.status === 'number' ? error.status : mapDataError(error).status; - if (status >= 500) logError('[REST] Unhandled error:', error); - sendError(res, error); + // This site used to judge on `status >= 500` alone, which + // also swallowed the un-coded 400 `mapDataError` degrades an + // UNRECOGNISED error to — a handler `TypeError` inside a + // batch transaction vanished here. The shared predicate + // keeps that one loud while staying quiet on the coded 4xx. + handleRouteError(res, error); } }, metadata: { @@ -7707,8 +7743,7 @@ export class RestServer { } as any); res.json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error, req.params?.object); + handleRouteError(res, error, req.params?.object); } }, metadata: { @@ -7760,8 +7795,7 @@ export class RestServer { } as any); res.status(201).json(result); } catch (error: any) { - logError("[REST] Unhandled error:", error); - sendError(res, error, req.params?.object); + handleRouteError(res, error, req.params?.object); } }, metadata: { @@ -7816,8 +7850,7 @@ export class RestServer { } as any); res.json(result); } catch (error: any) { - if (error?.code !== 'VALIDATION_FAILED') logError("[REST] Unhandled error:", error); - sendError(res, error, req.params?.object); + handleRouteError(res, error, req.params?.object); } }, metadata: { @@ -7879,8 +7912,7 @@ export class RestServer { } as any); res.json(result); } catch (error: any) { - if (error?.code !== 'VALIDATION_FAILED') logError("[REST] Unhandled error:", error); - sendError(res, error, req.params?.object); + handleRouteError(res, error, req.params?.object); } }, metadata: {