diff --git a/.changeset/package-door-coded-error-mapping.md b/.changeset/package-door-coded-error-mapping.md new file mode 100644 index 0000000000..fb1ea145f5 --- /dev/null +++ b/.changeset/package-door-coded-error-mapping.md @@ -0,0 +1,60 @@ +--- +"@objectstack/types": minor +"@objectstack/rest": patch +"@objectstack/runtime": patch +--- + +fix(rest,runtime,types): the direct-mount package door answers a coded refusal with its own status and code (#8016) + +**This changes HTTP status codes on a live surface.** Requests to +`/api/v1/packages` that today come back `500 INTERNAL_ERROR` will come back as +the refusal they always were — `409 DESTRUCTIVE_CHANGE` for an uninstall that +would drop data, `400`/`403` for a coded refusal thrown from below. A client +that keys on `500` to decide "the platform is down, retry later" for these +routes must key on the `code` instead. No route, path, verb or success body +changes. + +`/api/v1/packages` has two HTTP transports. The runtime dispatcher's +(`packages/runtime/src/domains/packages.ts`) reads a thrown error's own +`.status` and `.code` and answers with them. The direct-mount REST registrar +(`packages/rest/src/package-routes.ts`) had **four** catch-alls that answered +`sendError(res, 500, 'INTERNAL_ERROR', …)` regardless — and that registrar +mounts *first* in the production stack, so the status-blind answer was the one +production actually returned. `packageService.publish`, `packageService.delete` +and `protocol.deletePackage` all execute inside those blocks, and +`@objectstack/metadata-protocol` throws coded, status-carrying refusals from +that call path. So a caller who was **refused** was told the platform had +**broken**: the wrong class of answer, a retry that cannot succeed, and the one +field a client can branch on dropped. + +The four sites now leave through one shared exit. The mapping is not +reimplemented here — that is how the two doors diverged in the first place. It +moved to `resolveThrownHttpError` in **`@objectstack/types`** (alongside the +`sendOk`/`sendError` envelope writer and `looksLikeInternalErrorLeak`, for the +same reason: it is a property of the HTTP boundary, not of one router), and the +dispatcher's `HttpDispatcher.errorFromThrown` is now its other caller. It could +not live in `@objectstack/runtime`: that package depends on `@objectstack/rest`, +so the import can only point one way. + +The rule, unchanged from what the dispatcher always applied: + +- **status** — the producer's `.status`, then `.statusCode` (both spellings are + produced in this repo), then `400` for a record-validation failure, then the + caller's fallback. +- **code** — `VALIDATION_FAILED` for a validation failure, then the thrown + `.code` **when it is a member of the declared ADR-0112 vocabulary** + (`StandardErrorCode ∪ ERROR_CODE_LEDGER`), then the code the status derives. + An unregistered code no longer reaches `error.code` on the dispatcher door + either; it would have failed envelope parse, which is ADR-0112's closed + vocabulary working rather than a dialect leaking onto the wire. +- **the 500 survives** — a throw declaring neither status nor a registered code + is a genuine fault and still answers `500 INTERNAL_ERROR`. + +`validation-failure.ts` moved from `@objectstack/runtime` to +`@objectstack/types` for reachability and is re-exported from its old module +path; every existing import site is unchanged. + +Unchanged and deliberately so: this REST door still ships a 5xx message +verbatim, where the dispatcher withholds one that looks like an internal leak +(`looksLikeInternalErrorLeak`, #3867). That asymmetry predates this fix and is +filed separately. diff --git a/packages/rest/src/package-routes-coded-error-mapping.test.ts b/packages/rest/src/package-routes-coded-error-mapping.test.ts new file mode 100644 index 0000000000..56a7a3f0cb --- /dev/null +++ b/packages/rest/src/package-routes-coded-error-mapping.test.ts @@ -0,0 +1,343 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8016] The direct-mount package door answers a THROWN refusal with the + * refusal's own status and code — not `500 INTERNAL_ERROR`. + * + * ## The defect these cases reproduce + * + * All four handlers in `package-routes.ts` ended in the same catch-all: + * + * } catch (error) { + * sendError(res, 500, 'INTERNAL_ERROR', (error as Error).message); + * } + * + * status-blind and code-blind. `packageService.publish` / `.delete` and + * `protocol.deletePackage` execute inside those blocks, and the metadata + * protocol throws CODED, status-carrying refusals from that call path (`409 + * DESTRUCTIVE_CHANGE` is the established one). A caller who was refused was + * told the platform had broken: the wrong class of answer, a retry that cannot + * succeed, and the code — the one thing a client can branch on — dropped. + * + * It was a DISAGREEMENT, not just a bug: the dispatcher twin has always read + * `.status` first. The agreement itself is pinned in + * `packages/runtime/src/package-door-error-parity.test.ts`, which can see both + * doors (`@objectstack/runtime` depends on `@objectstack/rest`; the reverse + * import does not exist). This file pins the four SITES — that each one, on its + * own, carries a refusal's status **and** its code (ADR-0112: one alone is not + * an answer) and still answers 500 for a genuine fault. + * + * ## What reaches each catch + * + * Three of the four have a service call directly under the `try`, so a throwing + * `PackageService` drives them. `GET /packages` is different BY DESIGN: both of + * its data sources sit in their own inner `try { … } catch {}` (a missing + * protocol or a failed database read degrades to the other source rather than + * failing the request), so nothing below it reaches the outer catch. What does + * is the gate: `refusePackageRequest` calls + * `options.resolveExecutionContext(req)`, and a resolver that throws + * SYNCHRONOUSLY throws before the `.catch(() => undefined)` is attached. That + * is not a contrived lever — the composition wires it to the `RestServer`'s own + * identity/RBAC resolution, which is exactly the kind of code that raises a + * coded 401/403. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; +import type { RouteHandler } from '@objectstack/spec/contracts'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { registerPackageRoutes } from './package-routes.js'; + +const PKGS = '/api/v1/packages'; + +interface Captured { + status: number; + body: any; +} + +/** A caller holding every capability these routes gate on. */ +const CLEARS_THE_GATE = async () => ({ + userId: 'u_pkg', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], +}); + +function mount(svc: Record, options: Record = {}) { + const routes = new Map(); + const server = { + get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, + put: (p: string, h: RouteHandler) => { routes.set(`PUT:${p}`, h); }, + delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, + patch: () => {}, + use: () => {}, + listen: async () => {}, + close: async () => {}, + } as any; + registerPackageRoutes(server, () => svc as any, '/api/v1', { + resolveExecutionContext: CLEARS_THE_GATE, + ...options, + } as any); + return routes; +} + +async function drive( + routes: Map, + method: string, + path: string, + req: Record = {}, +): Promise { + const handler = routes.get(`${method}:${path}`); + if (!handler) throw new Error(`no handler for ${method} ${path}`); + const captured: Captured = { status: 0, body: undefined }; + const res: any = { + json(data: any) { captured.body = data; }, + send() {}, + status(code: number) { captured.status = code; return res; }, + header() { return res; }, + }; + await handler( + { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, + res, + ); + return captured; +} + +const MANIFEST = { id: 'com.acme.crm', version: '1.0.0' }; + +/** A thrown error carrying whatever a producer declares on it. */ +function thrown(message: string, carried: Record): Error { + return Object.assign(new Error(message), carried); +} + +/** + * One catch site, plus the seam that drives a throw INTO it and a witness that + * the throw really travelled that way (the anti-vacuity half: a case that + * silently never reached the seam would otherwise "pass" on a 500 it got for a + * completely different reason). + */ +interface Site { + name: string; + run: (error: unknown) => Promise<{ captured: Captured; reached: () => boolean }>; +} + +const SITES: Site[] = [ + { + name: 'POST /packages/publish — packageService.publish throws', + run: async (error: unknown) => { + const publish = vi.fn(async () => { throw error; }); + const captured = await drive( + mount({ publish }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, + ); + return { captured, reached: () => publish.mock.calls.length === 1 }; + }, + }, + { + name: 'GET /packages — the capability gate resolver throws', + run: async (error: unknown) => { + const resolveExecutionContext = vi.fn(() => { throw error; }); + const captured = await drive( + mount({ list: async () => [] }, { resolveExecutionContext }), + 'GET', + PKGS, + ); + return { captured, reached: () => resolveExecutionContext.mock.calls.length === 1 }; + }, + }, + { + name: 'GET /packages/:id — packageService.get throws', + run: async (error: unknown) => { + const get = vi.fn(async () => { throw error; }); + const captured = await drive( + mount({ get }), + 'GET', + `${PKGS}/:id`, + { params: { id: 'com.acme.crm' } }, + ); + return { captured, reached: () => get.mock.calls.length === 1 }; + }, + }, + { + name: 'DELETE /packages/:id — packageService.delete throws', + run: async (error: unknown) => { + const del = vi.fn(async () => { throw error; }); + const captured = await drive( + mount({ delete: del }), + 'DELETE', + `${PKGS}/:id`, + { params: { id: 'com.acme.crm' } }, + ); + return { captured, reached: () => del.mock.calls.length === 1 }; + }, + }, +]; + +/** + * Every assertion an answer from this door must satisfy, spelled once. The + * rules are IMPORTED from `packages/spec` rather than restated, so a body that + * parses here is one the wire contract accepts — including `code` being a + * member of the closed ADR-0112 vocabulary. + */ +function expectDeclaredEnvelope(captured: Captured): any { + expect(BaseResponseSchema.safeParse(captured.body).success).toBe(true); + expect(envelopeViolations(captured.body)).toEqual([]); + expect(captured.body?.success).toBe(false); + const parsed = ApiErrorSchema.safeParse(captured.body?.error); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.success).toBe(true); + return captured.body.error; +} + +describe('#8016 — a coded refusal keeps its status AND its code on every package route', () => { + /** + * Both spellings, because both are produced in this repo: `metadata-protocol` + * throws `status`, `plugin-approvals`' lifecycle hooks and + * `runtime`'s action execution throw `statusCode`. Reading one spelling is + * how `/api/v1/data` answered 500 for a deliberate `409 RECORD_LOCKED` until + * #7525 — the same defect one door over. + */ + const REFUSALS: Array<{ name: string; error: unknown; status: number; code: string }> = [ + { + name: 'a coded 4xx (`status`)', + error: thrown('Package scope is required', { status: 400, code: 'TENANT_SCOPE_REQUIRED' }), + status: 400, + code: 'TENANT_SCOPE_REQUIRED', + }, + { + name: 'the established 409 (`status`)', + error: thrown('Uninstalling drops 3 tables', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), + status: 409, + code: 'DESTRUCTIVE_CHANGE', + }, + { + name: 'a coded 4xx spelled `statusCode`', + error: thrown('Locked by a pending approval', { statusCode: 409, code: 'RECORD_LOCKED' }), + status: 409, + code: 'RECORD_LOCKED', + }, + ]; + + for (const site of SITES) { + for (const refusal of REFUSALS) { + it(`${site.name}: ${refusal.name}`, async () => { + const { captured, reached } = await site.run(refusal.error); + + // Anti-vacuity: the throw really travelled through the seam under test. + expect(reached(), 'the throwing seam was never called').toBe(true); + + const error = expectDeclaredEnvelope(captured); + // ADR-0112 — both halves. A status without the code leaves the client + // unable to branch; a code without the status leaves every proxy, + // retry policy and log dashboard reading it as a server fault. + expect(captured.status).toBe(refusal.status); + expect(error.code).toBe(refusal.code); + expect(error.message).toBe((refusal.error as Error).message); + }); + } + + it(`${site.name}: an uncoded throw still answers 500 INTERNAL_ERROR`, async () => { + const { captured, reached } = await site.run(new Error('kaboom')); + + expect(reached(), 'the throwing seam was never called').toBe(true); + const error = expectDeclaredEnvelope(captured); + // The default arm. Mapping everything and leaving nothing here would + // trade one wrong answer for another and hide real faults. + expect(captured.status).toBe(500); + expect(error.code).toBe('INTERNAL_ERROR'); + expect(error.message).toBe('kaboom'); + }); + + it(`${site.name}: an UNREGISTERED code does not get to name itself`, async () => { + // ADR-0112's vocabulary is closed: `StandardErrorCode ∪ ERROR_CODE_LEDGER`. + // A code outside it would fail `ApiErrorSchema` parse — a silent fourth + // state on the wire — so the answer falls to the code the status derives. + const { captured } = await site.run( + thrown('a dialect nobody registered', { status: 409, code: 'PACKAGE_IS_HAUNTED' }), + ); + + const error = expectDeclaredEnvelope(captured); + expect(captured.status).toBe(409); + expect(error.code).toBe('RESOURCE_CONFLICT'); + }); + } +}); + +/** + * The convergence half of the agreement pin (#8016). + * + * The literal cases above say what the answers ARE, which is what a reader + * needs. They do not, on their own, keep the two doors together: a second + * mapping written here could satisfy every literal above and still diverge + * from the dispatcher on the next throw shape nobody thought to enumerate — + * that is precisely how the divergence arose. + * + * So this door is pinned to the SHARED rule instead of to values: + * `resolveThrownHttpError` (`@objectstack/types`) is asked the same question, + * and its answer must be the one that went on the wire. The dispatcher twin is + * pinned to the same function from the other side, in + * `packages/runtime/src/package-door-error-parity.test.ts` — which is where the + * two-door comparison has to be split, because neither door can see the other: + * `registerPackageRoutes` is internal to `@objectstack/rest`, and `rest` cannot + * import `runtime` at all (runtime depends on rest). Either door drifting off + * the shared rule turns one of these two halves red. + */ +describe('#8016 — the wire answer IS the shared mapping, not a second copy of it', () => { + const SHAPES: unknown[] = [ + thrown('coded 4xx', { status: 400, code: 'TENANT_SCOPE_REQUIRED' }), + thrown('coded 409', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), + thrown('statusCode spelling', { statusCode: 403, code: 'PERMISSION_DENIED' }), + thrown('a record-validation failure', { name: 'ValidationError', code: 'VALIDATION_FAILED', fields: [] }), + thrown('an unregistered code', { status: 409, code: 'PACKAGE_IS_HAUNTED' }), + thrown('a bare fault', {}), + ]; + + for (const site of SITES) { + for (const shape of SHAPES) { + it(`${site.name}: answers exactly what resolveThrownHttpError says for "${(shape as Error).message}"`, async () => { + const expected = resolveThrownHttpError(shape); + const { captured, reached } = await site.run(shape); + + expect(reached(), 'the throwing seam was never called').toBe(true); + expect({ status: captured.status, code: captured.body?.error?.code }) + .toEqual({ status: expected.status, code: expected.code }); + }); + } + } + + it('the shapes above really do produce different answers', () => { + // Anti-vacuity for the comparison itself: two constants compared to each + // other agree trivially. These do not collapse to one answer. + const answers = SHAPES.map((s) => `${resolveThrownHttpError(s).status} ${resolveThrownHttpError(s).code}`); + expect(new Set(answers).size).toBeGreaterThan(3); + expect(answers).toContain('500 INTERNAL_ERROR'); + }); +}); + +describe('#8016 — anti-vacuity: these routes answer normally when nothing throws', () => { + // If the harness silently failed to drive a handler, every case above would + // "pass" by asserting on a body no route produced. These prove the same mounts + // serve real answers. + it('POST /packages/publish returns 200 when publish succeeds', async () => { + const captured = await drive( + mount({ publish: async () => ({ success: true }) }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, + ); + expect(captured.status).toBe(200); + expect(captured.body?.success).toBe(true); + }); + + it('GET /packages/:id returns 404 RESOURCE_NOT_FOUND for an absent package', async () => { + const captured = await drive( + mount({ get: async () => undefined }), + 'GET', + `${PKGS}/:id`, + { params: { id: 'com.acme.nope' } }, + ); + expect(captured.status).toBe(404); + expect(captured.body?.error?.code).toBe('RESOURCE_NOT_FOUND'); + }); +}); diff --git a/packages/rest/src/package-routes.ts b/packages/rest/src/package-routes.ts index 7d5bf3dcfc..cdfbc9fb34 100644 --- a/packages/rest/src/package-routes.ts +++ b/packages/rest/src/package-routes.ts @@ -8,8 +8,9 @@ import { IHttpServer, shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY // OUT. Same value it read before — no re-ruling by side effect. import { OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES } from '@objectstack/metadata-core'; import type { PackageService } from '@objectstack/service-package'; -// The declared envelope is written in ONE place for the whole platform (#3973). -import { sendOk, sendError } from '@objectstack/types'; +// The declared envelope is written in ONE place for the whole platform (#3973), +// and so (#8016) is the rule that reads an HTTP answer off a THROWN error. +import { sendOk, sendError, resolveThrownHttpError } from '@objectstack/types'; import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js'; import { readSingleQueryValue, repeatedQueryParamMessage } from './query-multiplicity.js'; @@ -78,6 +79,66 @@ async function refusePackageRequest( return false; } +/** + * [#8016] The catch-all exit for every route in this registrar. + * + * ## What it replaced + * + * Four `catch` blocks, all spelling the same thing: + * + * sendError(res, 500, 'INTERNAL_ERROR', (error as Error).message); + * + * i.e. status-blind and code-blind. `packageService.publish` / `.delete` and + * `protocol.deletePackage` run inside those blocks, and the metadata protocol + * throws CODED, status-carrying refusals from that call path — `409 + * DESTRUCTIVE_CHANGE` is the established one. So a caller who was *refused* + * was told the platform had *broken*: a 500 is a server fault, it invites a + * retry that cannot succeed, and it hides the one thing the caller needed to + * act on (the code). + * + * It was also a disagreement rather than merely a bug. The dispatcher twin + * (`packages/runtime/src/domains/packages.ts` → `errorFromThrown`) has always + * read `.status` first and answered 409 for the same throw. Two doors serve + * `/api/v1/packages`; **this** registrar mounts first in the production stack + * (first-match-wins, see the module note above), so the wrong answer was the + * live one. + * + * ## Why it delegates instead of mapping here + * + * The mapping is one rule and this is its second door, so it is CALLED, not + * restated — a second `if (code === ...)` ladder here is how the divergence + * arose in the first place. `resolveThrownHttpError` (`@objectstack/types`) is + * that rule, and the dispatcher's `errorFromThrown` is now its other caller; + * the two doors agree by construction rather than by two suites agreeing about + * literals. It lives in `@objectstack/types` because it cannot live in + * `@objectstack/runtime`: that package depends on THIS one, so the import would + * only ever point the other way. + * + * ## The 500 survives + * + * A throw that declares no status and no registered code is a genuine fault and + * still answers `500 INTERNAL_ERROR` — `resolveThrownHttpError`'s fallback is + * this call's `500`, and the code derives from it. Mapping everything and + * leaving nothing on the default arm would trade one wrong answer for another + * and hide real faults. + * + * ⚠️ The message is passed through as thrown. Unlike the dispatcher twin, this + * door applies no `looksLikeInternalErrorLeak` withholding to 5xx bodies — that + * gap predates this change and is unchanged by it (filed separately); nothing + * here newly exposes a message that was not already exposed, because the 500 + * arm shipped `(error as Error).message` verbatim before. + */ +function sendThrownError(res: any, error: unknown): void { + const thrown = resolveThrownHttpError(error); + sendError( + res, + thrown.status, + thrown.code, + thrown.message, + thrown.details ? { details: thrown.details } : undefined, + ); +} + /** * The `?version=` multiplicity rule (#6307), now shared (#6877). * @@ -225,6 +286,17 @@ export interface PackageRoutesOptions { * `readSingleQueryValue`), an unexpected throw is `INTERNAL_ERROR`. Only * the package-specific outcomes are registered — `PACKAGE_MANIFEST_INVALID`, * `PACKAGE_PUBLISH_FAILED`, `PACKAGE_DELETE_PARTIAL`, `PACKAGE_DELETE_FAILED`. + * + * [#8016] "An **unexpected** throw is `INTERNAL_ERROR`" is the sentence above, + * and it was right — the CODE had drifted wider than it. Every one of the four + * catch-alls treated *every* throw as unexpected, so a coded, status-carrying + * refusal from below (`409 DESTRUCTIVE_CHANGE` out of the metadata protocol, + * reached through `packageService.publish` / `.delete`) was answered as a + * server fault. The word doing the work is "unexpected": a throw that DECLARES + * its own status and a registered code is not unexpected, it is a refusal, and + * it now leaves through {@link sendThrownError} carrying both. `INTERNAL_ERROR` + * is still exactly what an unexpected throw gets — the sentence is unchanged + * because it was never the thing that was wrong. */ export function registerPackageRoutes( server: IHttpServer, @@ -292,7 +364,7 @@ export function registerPackageRoutes( sendError(res, 400, 'PACKAGE_PUBLISH_FAILED', result.error ?? `Failed to publish ${manifest.id}.`); } catch (error) { - sendError(res, 500, 'INTERNAL_ERROR', (error as Error).message); + sendThrownError(res, error); } }, }; @@ -362,7 +434,7 @@ export function registerPackageRoutes( const packages = Array.from(packagesMap.values()); sendOk(res, { packages, total: packages.length }); } catch (error) { - sendError(res, 500, 'INTERNAL_ERROR', (error as Error).message); + sendThrownError(res, error); } }, }, @@ -408,7 +480,7 @@ export function registerPackageRoutes( sendError(res, 404, 'RESOURCE_NOT_FOUND', `Package "${packageId}" was not found.`); } catch (error) { - sendError(res, 500, 'INTERNAL_ERROR', (error as Error).message); + sendThrownError(res, error); } }, }, @@ -495,7 +567,7 @@ export function registerPackageRoutes( `Failed to delete ${packageId}${version ? `@${version}` : ''}.`, ); } catch (error) { - sendError(res, 500, 'INTERNAL_ERROR', (error as Error).message); + sendThrownError(res, error); } }, }, diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 8de8f10a8b..c9fbe62967 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -3,7 +3,7 @@ import { ObjectKernel, getEnv, evaluateAuthGate, isAuthGateAllowlisted, } from '@objectstack/core'; -import { isMcpServerEnabled, looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; +import { isMcpServerEnabled, looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE, resolveThrownHttpError } from '@objectstack/types'; import { measureServerTiming, allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/observability'; import { CoreServiceName, serviceUnavailableMessage, inProcessServiceMessage } from '@objectstack/spec/system'; import type { IDataEngine, IObjectQLEngine } from '@objectstack/spec/contracts'; @@ -47,7 +47,6 @@ import { permissionDeniedErrorDetails, describeDeniedDiagnostics, } from './security/permission-denied-envelope.js'; -import { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js'; // randomUUID moved to ./domains/auth.ts with its only consumer (D11③ PR-7). @@ -705,13 +704,20 @@ export class HttpDispatcher { * expressed its real code as `details.code` gets it promoted into * `error.code` by the shared builder, and a site that has none gets one * derived from the status. See `./error-envelope.ts`. + * + * [#8016] `code` is optional and, when given, WINS over anything in + * `details` — the precedence `buildApiError` already declares. Only + * {@link errorFromThrown} passes it, because the shared resolver it + * delegates to has already answered the code question for both package + * doors; every other call site still expresses its code the way it did (a + * `details.code` to promote, or none at all and let the status derive one). */ - private error(message: string, httpStatus: number = 500, details?: any) { + private error(message: string, httpStatus: number = 500, details?: any, code?: string) { const safe = httpStatus >= 500 && looksLikeInternalErrorLeak(message) ? INTERNAL_ERROR_MESSAGE : message; - return apiErrorResponse({ message: safe, httpStatus, details }); + return apiErrorResponse({ message: safe, httpStatus, details, ...(code ? { code } : {}) }); } /** @@ -737,26 +743,31 @@ export class HttpDispatcher { * `@objectstack/rest`'s `mapDataError` has always mapped it: status 400, * `fields[]` passed through in `details`. An explicit `.status` / * `.statusCode` still wins, so this only supplies the fallback. + * + * [#8016] The rule itself moved to `resolveThrownHttpError` + * (`@objectstack/types`) and this method became its caller. Nothing about + * the precedence changed — it is the same table, read from one place now, + * because `/api/v1/packages` has a SECOND door (`@objectstack/rest`'s + * direct-mount registrar) that answered `500 INTERNAL_ERROR` for the very + * throws this method mapped correctly, and that door is the one production + * serves. A rule two doors must agree on cannot live inside one of them — + * and it could not live in this package regardless, since `rest` cannot + * import `runtime`. + * + * The one thing that stayed here is DISCLOSURE: `this.error` withholds a + * leaky 5xx message (#3867), which is a property of this surface's answer + * rather than of the throw. */ private errorFromThrown(e: any, fallbackStatus = 500) { - const validation = validationFailureDetails(e); - const status = - typeof e?.status === 'number' ? e.status - : typeof e?.statusCode === 'number' ? e.statusCode - : validation ? VALIDATION_FAILED_STATUS - : fallbackStatus; - const issues = Array.isArray(e?.issues) ? e.issues : undefined; - const details = - issues || e?.code || validation - ? { - ...(e?.code ? { code: e.code } : {}), - ...(issues ? { issues } : {}), - // Last so `code` is pinned to VALIDATION_FAILED even when the - // error was matched by `name` alone and carries no `.code`. - ...(validation ?? {}), - } - : undefined; - return this.error(e?.message ?? String(e), status, details); + const thrown = resolveThrownHttpError(e, fallbackStatus); + // `declaredCode`, NOT the narrowed `code`: this door's `error.code` is + // not closed in practice and three suites pin that — `STORAGE_FAILURE`, + // `FLOW_FAILED` and `DUPLICATE` are all unregistered and all expected on + // the wire verbatim. The REST door takes the narrowed spelling because + // its own conformance suite parses its bodies against the ledger. Both + // spellings come from the one resolver, so the difference is a stated + // one; see its module note. + return this.error(thrown.message, thrown.status, thrown.details, thrown.declaredCode); } diff --git a/packages/runtime/src/package-door-error-parity.test.ts b/packages/runtime/src/package-door-error-parity.test.ts new file mode 100644 index 0000000000..726097fbfc --- /dev/null +++ b/packages/runtime/src/package-door-error-parity.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8016] The dispatcher door for `/api/v1/packages` and the direct-mount REST + * door answer the SAME thrown error with the same status and the same code. + * + * ## What went wrong + * + * `/api/v1/packages` has two HTTP transports. This one — the dispatcher's + * packages domain (`./domains/packages.ts`) — ends every catch in + * `deps.errorFromThrown(e, 500)`, which has always read the error's own + * `.status` first: a `metadata-protocol` refusal carrying `409` and + * `DESTRUCTIVE_CHANGE` surfaced as a 409 with that code. The other transport + * (`@objectstack/rest`'s `registerPackageRoutes`) ended all four of its catches + * in `sendError(res, 500, 'INTERNAL_ERROR', …)` — status-blind. And that + * registrar mounts FIRST in the production stack, so the wrong answer was the + * one production returned: a caller who was refused was told the platform had + * broken. + * + * ## Why this pin is split across two files + * + * The natural pin — drive both doors with one throw and compare the two bodies + * — cannot be written anywhere. `@objectstack/rest` cannot import + * `@objectstack/runtime` (runtime depends on rest, so the arrow only points one + * way), and `registerPackageRoutes` is internal to rest rather than part of its + * public surface, so this package cannot reach it either. + * + * What both doors CAN see is the rule itself, which is why the fix moved it to + * `@objectstack/types`: `resolveThrownHttpError`. Each door is pinned to that + * function from its own side, and the halves compose — + * + * REST door == resolveThrownHttpError.code (packages/rest/src/package-routes-coded-error-mapping.test.ts) + * dispatcher == resolveThrownHttpError.declaredCode (this file) + * status: the SAME field on both + * ⇒ same status always, same code for every registered code + * + * — with each half independently falsifiable: a door that grows a second + * mapping of its own turns its own half red. Comparing to the shared rule is + * also what keeps this honest as new throw shapes appear; two suites agreeing + * about hand-written literals only ever agree about the shapes someone thought + * to enumerate, which is how the divergence arose in the first place. + * + * The two code spellings are one function's two outputs, not two rules. They + * differ only for a code outside `StandardErrorCode ∪ ERROR_CODE_LEDGER`, which + * this door emits verbatim and the REST door cannot — that difference is + * pinned explicitly at the bottom of this file, with the reason. + * + * ## What is deliberately NOT asserted: the message + * + * The dispatcher withholds a 5xx message that looks like a driver/internal leak + * (`looksLikeInternalErrorLeak`, #3867); the REST package door applies no such + * filter and ships the thrown message verbatim. That asymmetry predates #8016 + * and is filed separately — it is a DISCLOSURE rule, not a mapping rule, so + * pinning `status` + `code` here is the whole of what "the two doors agree" + * means today. Asserting message parity would pin the gap shut instead of + * leaving it visible. + */ + +import { describe, it, expect } from 'vitest'; +import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { HttpDispatcher } from './http-dispatcher.js'; +import type { DomainHandlerDeps } from './domain-handler-registry.js'; + +/** + * The REAL exit `domains/packages.ts` calls — reached through the dispatcher's + * own `domainDeps` seam rather than restated, so this pins production's mapper + * and not a stand-in for it. + */ +const errorFromThrown: DomainHandlerDeps['errorFromThrown'] = (() => { + const dispatcher: any = new HttpDispatcher({ context: { getService: () => null } } as any); + const deps: DomainHandlerDeps = dispatcher.domainDeps; + return deps.errorFromThrown; +})(); + +/** A thrown error carrying whatever a producer declares on it. */ +function thrown(message: string, carried: Record): Error { + return Object.assign(new Error(message), carried); +} + +/** + * The throw shapes the two doors have to agree about — a coded 4xx, the + * established coded 409, both status spellings, a record-validation failure, a + * code outside the declared vocabulary, and a genuinely unexpected fault. + */ +const SHAPES: Array<{ name: string; error: unknown }> = [ + { name: 'a coded 4xx (`status`)', error: thrown('scope required', { status: 400, code: 'TENANT_SCOPE_REQUIRED' }) }, + { name: 'the established 409', error: thrown('would drop data', { status: 409, code: 'DESTRUCTIVE_CHANGE' }) }, + { name: 'a coded 4xx spelled `statusCode`', error: thrown('locked', { statusCode: 409, code: 'RECORD_LOCKED' }) }, + { + name: 'a record-validation failure carrying neither status nor issues', + error: thrown('bad manifest', { name: 'ValidationError', code: 'VALIDATION_FAILED', fields: [{ field: 'id' }] }), + }, + { name: 'a genuinely unexpected fault', error: new Error('kaboom') }, +]; + +describe('#8016 — the dispatcher package door answers the shared mapping', () => { + for (const shape of SHAPES) { + it(`${shape.name}`, () => { + const expected = resolveThrownHttpError(shape.error, 500); + const response = errorFromThrown(shape.error, 500); + + // The declared envelope, checked against the schemas themselves so + // a `code` outside `StandardErrorCode ∪ ERROR_CODE_LEDGER` fails + // here rather than on someone's wire. + expect(BaseResponseSchema.safeParse(response.body).success).toBe(true); + expect(envelopeViolations(response.body)).toEqual([]); + expect(ApiErrorSchema.safeParse((response.body as any).error).success).toBe(true); + + expect({ status: response.status, code: (response.body as any).error.code }) + .toEqual({ status: expected.status, code: expected.declaredCode ?? expected.code }); + }); + } + + /** + * The one place the two doors' codes differ, pinned so it stays a stated + * difference rather than a drift. + * + * This door puts a producer's code on the wire verbatim — `STORAGE_FAILURE`, + * `FLOW_FAILED` and `DUPLICATE` are all outside + * `StandardErrorCode ∪ ERROR_CODE_LEDGER` and all pinned by existing suites + * here. The REST door cannot: `sendError` takes the closed `ErrorCode`, and + * that door's conformance suite parses its bodies against the ledger, so an + * unregistered code there is a failing test rather than a wire answer. + * + * The STATUS agrees either way, which is what #8016 was about. Whether this + * door's `error.code` should be closed too is a live contract question + * (`ApiErrorSchema` would reject these bodies) and is filed separately — it + * is not a decision this fix took. + */ + it('an unregistered code reaches this door verbatim, and the narrowed spelling differs', () => { + const error = thrown('dialect', { status: 409, code: 'PACKAGE_IS_HAUNTED' }); + const resolved = resolveThrownHttpError(error, 500); + const response = errorFromThrown(error, 500); + + expect(resolved.declaredCode).toBe('PACKAGE_IS_HAUNTED'); + expect(resolved.code).toBe('RESOURCE_CONFLICT'); + expect((response.body as any).error.code).toBe('PACKAGE_IS_HAUNTED'); + // The half that must never differ. + expect(response.status).toBe(resolved.status); + }); + + it('the shapes really do produce different answers', () => { + // Anti-vacuity for the comparison: two constants agree trivially. These + // span 400 / 409 / 500 and four distinct codes, and the fault case must + // still land on the default arm. + const answers = SHAPES.map((s) => { + const r = errorFromThrown(s.error, 500); + return `${r.status} ${(r.body as any).error.code}`; + }); + expect(new Set(answers).size).toBeGreaterThan(3); + expect(answers).toContain('500 INTERNAL_ERROR'); + }); + + it('a refusal is never reported as a server fault', () => { + // The defect in one line, from the door that never had it: a coded 4xx + // must not come back 5xx. This is the assertion the REST door failed. + for (const shape of SHAPES.slice(0, 4)) { + expect(errorFromThrown(shape.error, 500).status).toBeLessThan(500); + } + }); +}); diff --git a/packages/runtime/src/validation-failure.ts b/packages/runtime/src/validation-failure.ts index 41ed4aa153..4863618ee6 100644 --- a/packages/runtime/src/validation-failure.ts +++ b/packages/runtime/src/validation-failure.ts @@ -1,79 +1,26 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Recognising a record-validation failure at an HTTP boundary. + * Moved to `@objectstack/types` (#8016) — re-exported here so every import site + * in this package keeps the name it always had. * - * `ValidationError` (`@objectstack/objectql`'s record/rule validators) carries - * `.code = 'VALIDATION_FAILED'` and `.fields[]` — one entry per offending - * field — but deliberately carries NO `.status` / `.statusCode` and no - * `.issues`. It is a plain domain error; deciding it means "400" is the job of - * whichever boundary serves it. + * The recogniser had to become reachable from `@objectstack/rest`: the shared + * thrown-error resolver both `/api/v1/packages` doors now call + * (`resolveThrownHttpError`) answers "is this throw a validation failure?", and + * `rest` cannot import `runtime` — runtime depends on rest, so the arrow only + * points one way. `@objectstack/types` is where the repo already keeps the + * helpers every HTTP boundary shares. * - * `@objectstack/rest` has always done that (`mapDataError` → 400 with - * `fields[]`). The runtime dispatcher's two error exits did not (#3918): with - * no `.status` to read they fell back to **500**, and both read only `.issues` - * for structured detail — which a `ValidationError` never has — so `fields[]` - * was dropped and the caller got a generic "internal error" for what was - * really a user-input mistake. That forecloses per-field error display on every - * surface the dispatcher serves. - * - * Matched by duck-typing on `code` / `name` — exactly the predicate - * `mapDataError` uses — so this module stays free of a runtime dependency on - * `objectql`, and so hand-rolled errors of the same shape (e.g. a hook that - * throws `{ code: 'VALIDATION_FAILED', fields }`) are served identically. - */ - -/** The HTTP status a validation failure maps to when the error names none. */ -export const VALIDATION_FAILED_STATUS = 400; - -export interface ValidationFailureDetails { - code: 'VALIDATION_FAILED'; - /** Per-field envelopes, passed through verbatim. `[]` when absent/malformed. */ - fields: unknown[]; -} - -/** - * Structured `details` for a thrown validation failure, or `undefined` when - * `err` is not one. Callers use the `undefined` result as the predicate and the - * returned object as the `details` payload, so the two can never disagree. + * The module's own argument — why the predicate duck-types on `code`/`name` + * rather than importing objectql's `ValidationError`, and why the constructor + * sits beside the recogniser so the two cannot drift — travelled with it. Read + * it there: `packages/types/src/validation-failure.ts`. */ -export function validationFailureDetails(err: any): ValidationFailureDetails | undefined { - if (!err) return undefined; - if (err.code !== 'VALIDATION_FAILED' && err.name !== 'ValidationError') return undefined; - return { - code: 'VALIDATION_FAILED', - fields: Array.isArray(err.fields) ? err.fields : [], - }; -} -/** - * [#3878/#3899] The CONSTRUCTOR for the shape {@link validationFailureDetails} - * recognises — kept in the same module so the two can never drift. Thrown from - * a domain handler, both dispatcher error exits map it to - * `400 VALIDATION_FAILED` + `details.fields[]` (#3918) with no new error - * channel and no runtime dependency on objectql's `ValidationError` class. - * First built inline by the analytics domain; hoisted here when notifications - * and automation grew the same entry gates rather than a third copy. - */ -export function validationFailure(message: string, fields: unknown[]): Error { - const err = new Error(message) as Error & { code: string; fields: unknown[] }; - err.name = 'ValidationError'; - err.code = 'VALIDATION_FAILED'; - err.fields = fields; - return err; -} - -/** - * Zod issues → the dispatcher's `fields[]` envelope entries - * (`{ field, code, message }`). `'(body)'` names a root-level failure — a body - * that is the wrong TYPE entirely has no path to point at. - */ -export function fieldsFromZodIssues( - issues: Array<{ path: Array; code: string; message: string }>, -): Array<{ field: string; code: string; message: string }> { - return issues.map((issue) => ({ - field: issue.path.length > 0 ? issue.path.join('.') : '(body)', - code: issue.code, - message: issue.message, - })); -} +export { + VALIDATION_FAILED_STATUS, + validationFailureDetails, + validationFailure, + fieldsFromZodIssues, + type ValidationFailureDetails, +} from '@objectstack/types'; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index d8d3542878..279704029f 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -8,6 +8,13 @@ export * from './error-leak.js'; export * from './keyset-walk.js'; export * from './module-not-found.js'; export * from './response-envelope.js'; +// [#8016] The one rule for "what HTTP answer does a THROWN error declare?", +// plus the validation-failure recogniser it reads. Both doors of +// `/api/v1/packages` call it: the runtime dispatcher's `errorFromThrown` and the +// direct-mount REST registrar, which used to answer 500 INTERNAL_ERROR for a +// coded 4xx the dispatcher mapped correctly. +export * from './thrown-http-error.js'; +export * from './validation-failure.js'; // [#6615] The one home for Postgres' `«sub-object» "x" of relation "y"` phrase, // whose missing-COLUMN spelling contains a legal missing-TABLE phrase as a // substring. Three packages had each repaired that superstring hole separately. diff --git a/packages/types/src/thrown-http-error.ts b/packages/types/src/thrown-http-error.ts new file mode 100644 index 0000000000..8f26d71ac8 --- /dev/null +++ b/packages/types/src/thrown-http-error.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The ONE rule for "what HTTP answer does a THROWN error declare?" (#8016). + * + * A service or protocol throw that carries its own `.status` / `.statusCode` + * and its own semantic `.code` is a *refusal*, not a fault: the caller asked + * for something the platform will not do, and the honest answer is that status + * with that code. A throw carrying neither is a fault, and the honest answer is + * the caller's fallback — 500 `INTERNAL_ERROR` at an HTTP boundary. + * + * ## Why this is shared rather than restated per door + * + * `/api/v1/packages` has **two** HTTP doors. The runtime dispatcher's + * `HttpDispatcher.errorFromThrown` read `.status` first and answered `409 + * DESTRUCTIVE_CHANGE` for a `metadata-protocol` refusal. The direct-mount REST + * registrar (`packages/rest/src/package-routes.ts`) had four catch-alls that + * answered `500 INTERNAL_ERROR` regardless — and *that* registrar mounts first + * in the production stack, so 500 was what production actually returned. One + * throw, two answers, and the wrong one was the live one (#8016). + * + * The rule therefore lives in ONE function that both doors call. It could not + * live in `packages/runtime`: `@objectstack/runtime` depends on + * `@objectstack/rest`, so the arrow only points one way and `errorFromThrown` + * is unreachable from the REST door by construction. `@objectstack/types` + * depends on nothing but `@objectstack/spec`, which is exactly why the other + * shared HTTP-boundary helpers already live here — `looksLikeInternalErrorLeak` + * ("do not ship driver internals to clients") and `sendOk`/`sendError` ("write + * the declared envelope"). "What status does this throw mean?" is the same kind + * of property: it belongs to the boundary, not to one router. + * + * ## Two spellings of the code, because the two envelopes are not equally closed + * + * {@link ThrownHttpError.code} is narrowed to `StandardErrorCode ∪ + * ERROR_CODE_LEDGER` — the union `ApiErrorSchema` validates against — so a + * throw whose `.code` is not a registered member does not get to name itself; + * it falls to the code the status derives. That is the same rule + * `metadata-protocol`'s `toRowApiError` applies to a per-row batch error, and + * it is what lets `sendError`'s closed `ErrorCode` parameter be satisfied + * without a cast. The direct-mount REST door needs exactly this: its bodies are + * parsed against `BaseResponseSchema` by its own conformance suite, so an + * unregistered code there is a failing test, not a wire answer. + * + * {@link ThrownHttpError.declaredCode} is the producer's own string, verbatim + * and un-narrowed, which is what the dispatcher door has always put on the + * wire — `STORAGE_FAILURE`, `FLOW_FAILED` and `DUPLICATE` are all unregistered + * and all pinned by existing dispatcher tests. Narrowing it here would rewrite + * a behaviour three suites assert, which is a contract decision (should the + * dispatcher's `error.code` be closed too?) and not this function's to take. + * + * So the doors agree on **status** unconditionally and on **code** for every + * registered code, and differ only where a producer emits a code the ledger + * does not know — a case that is already a contract violation on either door. + * Both answers come from ONE function, which is what keeps that difference a + * documented one rather than a drift. + * + * ## What this deliberately does NOT decide + * + * - **Message disclosure.** A 5xx message may name physical tables or carry a + * driver dump; withholding it is `looksLikeInternalErrorLeak`'s job, applied + * by the caller (the dispatcher does; see #3867). This function returns the + * thrown message verbatim. + * - **Whether a declared status is *plausible*.** No 400-599 band is imposed, + * because the dispatcher never imposed one and this function exists to make + * the two doors agree. Narrowing the accepted band is a change to the rule, + * and it belongs here — in one place, for both doors — if it is ever made. + */ + +import { ErrorCode, standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; +import { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js'; + +/** The HTTP answer a thrown error declares. See {@link resolveThrownHttpError}. */ +export interface ThrownHttpError { + /** The producer's own `status`/`statusCode`, or the caller's fallback. */ + status: number; + /** + * A member of the declared ADR-0112 vocabulary — for a boundary whose + * envelope is checked against it. Never the HTTP status. + */ + code: ErrorCode; + /** + * The producer's own code, verbatim and un-narrowed, or `undefined` when it + * declared none. For the dispatcher door, whose `error.code` is not closed in + * practice. See the module note on why there are two. + */ + declaredCode?: string; + /** The thrown message, UNSANITISED — see the module note on disclosure. */ + message: string; + /** + * Structured context: spec-validation `issues[]`, record-validation + * `fields[]`. Absent rather than `{}` when the throw carried none, so an + * empty object never reads as "there is context here". + */ + details?: Record; +} + +/** + * Resolve a thrown error into the status, code, message and structured context + * an HTTP boundary should answer with. + * + * Precedence, in order: + * + * | Question | Answer | + * |---|---| + * | status | `.status` → `.statusCode` → 400 if it is a validation failure → `fallbackStatus` | + * | code | `VALIDATION_FAILED` if it is one → a REGISTERED `.code` → derived from the status | + * | declaredCode | `VALIDATION_FAILED` if it is one → any non-empty string `.code` → absent | + * | message | `.message` when it is a string → `String(error)` | + * + * Both status spellings are read because both are produced in this repo: + * `plugin-approvals`' lifecycle hooks and `metadata-protocol` throw + * `statusCode`, `metadata-protocol`'s conflicts throw `status`. Reading one + * spelling is how `/api/v1/data` answered 500 for a deliberate `409 + * RECORD_LOCKED` until #7525. + */ +export function resolveThrownHttpError(error: unknown, fallbackStatus = 500): ThrownHttpError { + const e = error as any; + const validation = validationFailureDetails(e); + + const declaredStatus = + typeof e?.status === 'number' ? e.status + : typeof e?.statusCode === 'number' ? e.statusCode + : undefined; + const status = declaredStatus ?? (validation ? VALIDATION_FAILED_STATUS : fallbackStatus); + + const spelled = typeof e?.code === 'string' && e.code !== '' ? e.code : undefined; + // A `.code` the ledger does not know cannot go in a slot typed as the closed + // vocabulary — see the module note on why there are two spellings. + const registered = spelled !== undefined && ErrorCode.safeParse(spelled).success + ? (spelled as ErrorCode) + : undefined; + const code: ErrorCode = validation + ? validation.code + : (registered ?? standardErrorCodeForHttpStatus(status)); + const declaredCode = validation ? validation.code : spelled; + + const issues = Array.isArray(e?.issues) ? e.issues : undefined; + const details: Record = { + // A truthy NON-string `code` (a driver errno, say) is context and stays + // context — promoting it would put a number in the field callers branch on, + // which is the drift #3842 removed. + ...(!validation && e?.code && typeof e.code !== 'string' ? { code: e.code } : {}), + ...(issues ? { issues } : {}), + ...(validation ? { fields: validation.fields } : {}), + }; + + return { + status, + code, + ...(declaredCode !== undefined ? { declaredCode } : {}), + message: typeof e?.message === 'string' ? e.message : String(error), + ...(Object.keys(details).length > 0 ? { details } : {}), + }; +} diff --git a/packages/types/src/validation-failure.ts b/packages/types/src/validation-failure.ts new file mode 100644 index 0000000000..0cc1e63e78 --- /dev/null +++ b/packages/types/src/validation-failure.ts @@ -0,0 +1,93 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Recognising a record-validation failure at an HTTP boundary. + * + * `ValidationError` (`@objectstack/objectql`'s record/rule validators) carries + * `.code = 'VALIDATION_FAILED'` and `.fields[]` — one entry per offending + * field — but deliberately carries NO `.status` / `.statusCode` and no + * `.issues`. It is a plain domain error; deciding it means "400" is the job of + * whichever boundary serves it. + * + * `@objectstack/rest` has always done that (`mapDataError` → 400 with + * `fields[]`). The runtime dispatcher's two error exits did not (#3918): with + * no `.status` to read they fell back to **500**, and both read only `.issues` + * for structured detail — which a `ValidationError` never has — so `fields[]` + * was dropped and the caller got a generic "internal error" for what was + * really a user-input mistake. That forecloses per-field error display on every + * surface the dispatcher serves. + * + * Matched by duck-typing on `code` / `name` — exactly the predicate + * `mapDataError` uses — so this module stays free of a runtime dependency on + * `objectql`, and so hand-rolled errors of the same shape (e.g. a hook that + * throws `{ code: 'VALIDATION_FAILED', fields }`) are served identically. + * + * ## Why it lives in `@objectstack/types` (#8016) + * + * It was `packages/runtime/src/validation-failure.ts` until the *package* door's + * four status-blind catch-alls were converged onto the dispatcher's mapping + * ({@link resolveThrownHttpError}, one file over). That resolver has to answer + * "is this throw a validation failure?" the same way on both doors, and + * `@objectstack/rest` cannot import `@objectstack/runtime` — runtime depends on + * rest, so the arrow only points one way. `@objectstack/types` depends on + * nothing but `@objectstack/spec`, which is why the shared HTTP-boundary + * helpers (`looksLikeInternalErrorLeak`, `sendOk`/`sendError`) already live + * here. This module moved for the same reason and is unchanged otherwise; + * `packages/runtime/src/validation-failure.ts` re-exports it, so every runtime + * import site still reads the name it always did. + */ + +/** The HTTP status a validation failure maps to when the error names none. */ +export const VALIDATION_FAILED_STATUS = 400; + +export interface ValidationFailureDetails { + code: 'VALIDATION_FAILED'; + /** Per-field envelopes, passed through verbatim. `[]` when absent/malformed. */ + fields: unknown[]; +} + +/** + * Structured `details` for a thrown validation failure, or `undefined` when + * `err` is not one. Callers use the `undefined` result as the predicate and the + * returned object as the `details` payload, so the two can never disagree. + */ +export function validationFailureDetails(err: any): ValidationFailureDetails | undefined { + if (!err) return undefined; + if (err.code !== 'VALIDATION_FAILED' && err.name !== 'ValidationError') return undefined; + return { + code: 'VALIDATION_FAILED', + fields: Array.isArray(err.fields) ? err.fields : [], + }; +} + +/** + * [#3878/#3899] The CONSTRUCTOR for the shape {@link validationFailureDetails} + * recognises — kept in the same module so the two can never drift. Thrown from + * a domain handler, both dispatcher error exits map it to + * `400 VALIDATION_FAILED` + `details.fields[]` (#3918) with no new error + * channel and no runtime dependency on objectql's `ValidationError` class. + * First built inline by the analytics domain; hoisted here when notifications + * and automation grew the same entry gates rather than a third copy. + */ +export function validationFailure(message: string, fields: unknown[]): Error { + const err = new Error(message) as Error & { code: string; fields: unknown[] }; + err.name = 'ValidationError'; + err.code = 'VALIDATION_FAILED'; + err.fields = fields; + return err; +} + +/** + * Zod issues → the dispatcher's `fields[]` envelope entries + * (`{ field, code, message }`). `'(body)'` names a root-level failure — a body + * that is the wrong TYPE entirely has no path to point at. + */ +export function fieldsFromZodIssues( + issues: Array<{ path: Array; code: string; message: string }>, +): Array<{ field: string; code: string; message: string }> { + return issues.map((issue) => ({ + field: issue.path.length > 0 ? issue.path.join('.') : '(body)', + code: issue.code, + message: issue.message, + })); +}