fix(runtime): route every domain catch through errorFromThrown (#3918 follow-up)

.changeset/domain-error-passthrough.md

@@ -0,0 +1,40 @@
+---
+"@objectstack/runtime": patch
+---
+
+fix(runtime): route every domain `catch` through `errorFromThrown` so status and `fields[]` survive (#3918 follow-up)
+
+#3867 taught `dispatcher-plugin`'s `errorResponseBase` to read an error's
+`status` (not just `statusCode`), and #3918 taught
+`HttpDispatcher.errorFromThrown` the `VALIDATION_FAILED` shape. Both fixes were
+invisible to a whole tier of handlers underneath them: the domain modules each
+caught their own errors and called `deps.error(e.message, e.statusCode || 500)`
+directly, bypassing `errorFromThrown` entirely — 13 call sites, 9 of them in
+`/packages` alone.
+
+The consequence on `/packages`, `/meta/_drafts`, `/ui`, `/security` and the
+`/mcp` transport:
+
+- **A deliberate status was downgraded to 500.** Every protocol-layer domain
+  error in this codebase carries its HTTP status as `status`, not `statusCode`
+  (`OBJECT_NOT_FOUND`, `RECORD_NOT_FOUND`, `CLONE_DISABLED`, plugin-sharing's
+  `FORBIDDEN`, …) — the exact read #3867 fixed one tier up. So a 404 these
+  routes meant to return arrived as a 500, and the message was dragged through
+  the 5xx leak sanitiser on the way out.
+- **A `ValidationError` still lost its `fields[]`** and its 400, re-opening
+  #3918 on precisely the routes it was filed against.
+
+Every one of those catches now calls `deps.errorFromThrown(e, …)`, so both
+fixes finally reach the routes that need them. Deliberate per-route fallbacks
+are preserved rather than flattened to 500: the `/meta` save fallback keeps
+**501** (that branch is reached only when the protocol has no `saveMetaItem`,
+so "unsupported" is the honest default) and the `/meta` two-part lookup keeps
+**404** — but a validation failure on either now answers 400 with its fields
+instead of being swallowed by the fallback.
+
+`domains/keys.ts` is deliberately **not** converted: it discards the underlying
+error on purpose, because the message could echo row contents. Its literal
+`'Failed to create API key'` is the correct answer there and stays.
+
+No behaviour change for errors that already carried `statusCode` — that read is
+preserved, only widened.

packages/runtime/src/domains/error-passthrough.test.ts

@@ -0,0 +1,160 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #3918 follow-up — the THIRD tier of dispatcher error handling.
+ *
+ * #3867 taught `dispatcher-plugin`'s `errorResponseBase` to read `status` (not
+ * just `statusCode`) and #3918 taught `HttpDispatcher.errorFromThrown` the
+ * `VALIDATION_FAILED` shape. Both fixes were invisible to a whole tier of
+ * handlers underneath them: the domain modules each caught their own errors and
+ * called `deps.error(e.message, e.statusCode || 500)` directly, bypassing
+ * `errorFromThrown` entirely. So on `/packages`, `/meta/_drafts`, `/ui`,
+ * `/security` and the `/mcp` transport:
+ *
+ *  - a domain error carrying `status` (the convention every protocol-layer
+ *    error uses — `OBJECT_NOT_FOUND`, `RECORD_NOT_FOUND`, `CLONE_DISABLED`,
+ *    plugin-sharing's `FORBIDDEN`, …) still rendered as a **500**, and
+ *  - a record `ValidationError` still lost its `fields[]` and its 400.
+ *
+ * Every one of those catches now routes through `deps.errorFromThrown(e, …)`,
+ * so the two fixes above finally reach the routes that need them. The
+ * per-route fallback status is preserved where it was deliberate (`/meta`
+ * save → 501, the `/meta` two-part lookup → 404).
+ *
+ * `keys.ts` is deliberately NOT converted and is guarded below: it discards the
+ * underlying error on purpose, because the message could echo row contents.
+ */
+
+import { describe, it, expect } from 'vitest';
+
+import { HttpDispatcher } from './../http-dispatcher.js';
+
+/** A `ValidationError` as it looks on the wire side of a throw. */
+const FIELDS = [{ field: 'name', code: 'required', message: 'name is required' }];
+function validationError() {
+    const err = new Error('name is required') as Error & { code: string; fields: unknown[] };
+    err.name = 'ValidationError';
+    err.code = 'VALIDATION_FAILED';
+    err.fields = FIELDS;
+    return err;
+}
+
+/** A protocol-layer domain error: status on `status`, never `statusCode`. */
+function domainNotFound() {
+    return Object.assign(new Error("Object 'ghost' is not registered"), {
+        code: 'OBJECT_NOT_FOUND',
+        status: 404,
+    });
+}
+
+/**
+ * Dispatch against a kernel whose `protocol` service throws `err` from every
+ * method these routes call. `objectql.registry` satisfies the `/packages`
+ * pre-check.
+ */
+async function dispatchWith(method: string, path: string, err: unknown, body: any = {}) {
+    const thrower = async () => { throw err; };
+    const protocol = {
+        listCommits: thrower,
+        listDrafts: thrower,
+        getUiView: thrower,
+        updatePackage: thrower,
+    };
+    const objectql = { registry: {} };
+    const resolve = (name: string) =>
+        name === 'protocol' ? protocol : name === 'objectql' ? objectql : undefined;
+    const kernel: any = {
+        getService: resolve,
+        getServiceAsync: async (name: string) => resolve(name),
+    };
+    const result: any = await new HttpDispatcher(kernel).dispatch(method, path, body, {}, {} as any);
+    return result.response;
+}
+
+describe('#3918 follow-up — domain catches honour `status` instead of forcing 500', () => {
+    // One case per converted module, driven through the real route.
+    const routes: Array<[string, string, string]> = [
+        ['/packages', 'GET', '/packages/demo/commits'],
+        ['/meta/_drafts', 'GET', '/meta/_drafts'],
+        ['/ui', 'GET', '/ui/view/account'],
+    ];
+
+    for (const [label, method, path] of routes) {
+        it(`${label}: a domain error's own 404 survives (was 500)`, async () => {
+            const res = await dispatchWith(method, path, domainNotFound());
+
+            expect(res.status).toBe(404);
+            // A 4xx message is a deliberate answer and must reach the caller.
+            expect(res.body.error.message).toContain("Object 'ghost' is not registered");
+            expect(res.body.error.details?.code).toBe('OBJECT_NOT_FOUND');
+        });
+
+        it(`${label}: a ValidationError keeps its 400 and its fields[]`, async () => {
+            const res = await dispatchWith(method, path, validationError());
+
+            expect(res.status).toBe(400);
+            expect(res.body.error.details).toEqual({
+                code: 'VALIDATION_FAILED',
+                fields: FIELDS,
+            });
+        });
+
+        it(`${label}: an ordinary error still falls back to 500`, async () => {
+            const res = await dispatchWith(method, path, new Error('backend unavailable'));
+
+            expect(res.status).toBe(500);
+            expect(res.body.error.message).toBe('backend unavailable');
+        });
+    }
+
+    it('/packages: `statusCode` still works for callers that use it', async () => {
+        // The property the old hand-rolled read honoured — converting must not
+        // drop it, only widen what else is understood.
+        const err = Object.assign(new Error('conflict'), { statusCode: 409 });
+        const res = await dispatchWith('GET', '/packages/demo/commits', err);
+
+        expect(res.status).toBe(409);
+        expect(res.body.error.message).toBe('conflict');
+    });
+
+    it('/packages PATCH: the same holds on the write path', async () => {
+        // A real patch body — the route rejects an empty one with its own 400
+        // before it ever reaches `updatePackage`.
+        const res = await dispatchWith('PATCH', '/packages/demo', domainNotFound(), { name: 'renamed' });
+
+        expect(res.status).toBe(404);
+    });
+});
+
+describe('#3918 follow-up — deliberate per-route fallbacks are preserved', () => {
+    /** `/meta` PUT with a metadata service (no `saveMetaItem`) → the 501 branch. */
+    async function putMetaViaMetadataService(err: unknown) {
+        const metadata = { saveItem: async () => { throw err; } };
+        const resolve = (name: string) => (name === 'metadata' ? metadata : undefined);
+        const kernel: any = {
+            getService: resolve,
+            getServiceAsync: async (name: string) => resolve(name),
+        };
+        const result: any = await new HttpDispatcher(kernel).dispatch(
+            'PUT', '/meta/object/widget', { name: 'widget' }, {}, {} as any,
+        );
+        return result.response;
+    }
+
+    it('/meta save keeps 501 as the fallback for an ordinary failure', async () => {
+        // This branch is only reached when the protocol has no `saveMetaItem`,
+        // so "not supported" remains the honest default.
+        const res = await putMetaViaMetadataService(new Error('no backend'));
+
+        expect(res.status).toBe(501);
+    });
+
+    it('/meta save still answers 400 when the failure is validation', async () => {
+        // …but a save rejected by validation is a fixable 400, not a capability
+        // gap. This is the case the 501 fallback used to swallow.
+        const res = await putMetaViaMetadataService(validationError());
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.details.fields).toEqual(FIELDS);
+    });
+});

packages/runtime/src/domains/mcp.ts

@@ -122,7 +122,7 @@ export async function handleMcpRequest(deps: DomainHandlerDeps, body: any, conte
             ...(grantedScopes ? { toolOptions: { grantedScopes } } : {}),
         });
     } catch (err: any) {
-        return { handled: true, response: deps.error(err?.message ?? 'MCP request failed', 500) };
+        return { handled: true, response: deps.errorFromThrown(err, 500) };
     }
 
     // Convert the transport's buffered Web Response into the dispatcher's

packages/runtime/src/domains/meta.ts

@@ -174,7 +174,11 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin
                     const data = await (metaSvc as any).saveItem(type, name, body);
                     return { handled: true, response: deps.success(data) };
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message || 'Save not supported', 501) };
+                    // 501 stays the FALLBACK (this branch is reached only when
+                    // the protocol has no `saveMetaItem`, so "unsupported" is
+                    // the honest default) — but a save that fails validation is
+                    // a 400 the caller can fix, not a capability gap.
+                    return { handled: true, response: deps.errorFromThrown(e, 501) };
                 }
             }
             return { handled: true, response: deps.error('Save not supported', 501) };
@@ -261,8 +265,9 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin
             return { handled: true, response: deps.error('Not found', 404) };
         } catch (e: any) {
             // Fallback: treat first part as object name if only 1 part (handled below)
-            // But here we are deep in 2 parts. Must be an error.
-            return { handled: true, response: deps.error(e.message, 404) };
+            // But here we are deep in 2 parts. Must be an error — 404 remains the
+            // default, but an error carrying its own status keeps it.
+            return { handled: true, response: deps.errorFromThrown(e, 404) };
         }
     }
 
@@ -283,7 +288,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin
                 });
                 return { handled: true, response: deps.success(data) };
             } catch (e: any) {
-                return { handled: true, response: deps.error(e.message, 500) };
+                return { handled: true, response: deps.errorFromThrown(e, 500) };
             }
         }
         return { handled: true, response: deps.error('Draft listing not supported', 501) };

packages/runtime/src/domains/packages.ts

@@ -37,6 +37,17 @@ export function createPackagesDomain(deps: DomainHandlerDeps): DomainRoute {
  * - POST   /packages/:id/revert  → revert a package to last published state
  * 
  * Uses ObjectQL SchemaRegistry directly (via the 'objectql' service).
+ *
+ * **Error handling.** Every `catch` here goes through `deps.errorFromThrown(e,
+ * 500)` — never `deps.error(e.message, …)`. Each of these routes calls into the
+ * protocol service, whose errors carry their own HTTP status as `status` (the
+ * codebase convention: `OBJECT_NOT_FOUND`, `RECORD_NOT_FOUND`, `CLONE_DISABLED`,
+ * …), and can be a record `ValidationError` carrying `fields[]`. Hand-rolling
+ * `e.statusCode || 500` saw neither — #3867 fixed exactly that read in
+ * `errorResponseBase` and #3918 taught `errorFromThrown` the validation shape,
+ * but these call sites bypassed both, so a deliberate 404 still rendered as a
+ * 500 and `fields[]` was still dropped. Route new handlers through the shared
+ * helper rather than re-deriving the status here.
  */
 export async function handlePackagesRequest(deps: DomainHandlerDeps, path: string, method: string, body: any, query: any, _context: HttpProtocolContext): Promise<HttpDispatcherResult> {
     const m = method.toUpperCase();
@@ -275,7 +286,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                     });
                     return { handled: true, response: deps.success(result) };
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                    return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
             return { handled: true, response: deps.error('Draft discarding not supported', 501) };
@@ -296,7 +307,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                     });
                     return { handled: true, response: deps.success({ commits }) };
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                    return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
             return { handled: true, response: deps.error('Commit history not supported', 501) };
@@ -318,7 +329,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                     });
                     return { handled: true, response: deps.success(result) };
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                    return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
             return { handled: true, response: deps.error('Commit revert not supported', 501) };
@@ -341,7 +352,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                     });
                     return { handled: true, response: deps.success(result) };
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                    return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
             return { handled: true, response: deps.error('Commit rollback not supported', 501) };
@@ -387,7 +398,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                 });
                 return { handled: true, response: deps.success(result) };
             } catch (e: any) {
-                return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                return { handled: true, response: deps.errorFromThrown(e, 500) };
             }
         }
 
@@ -416,7 +427,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                 });
                 return { handled: true, response: deps.success(result) };
             } catch (e: any) {
-                return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                return { handled: true, response: deps.errorFromThrown(e, 500) };
             }
         }
 
@@ -457,7 +468,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                     const updated = await (protocol as any).updatePackage({ packageId: id, patch });
                     return { handled: true, response: deps.success((updated as any)?.package ?? updated) };
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                    return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
             // Fallback: no protocol service — in-memory registry only.
@@ -490,7 +501,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                         ...(keepData ? { keepData: true } : {}),
                     });
                 } catch (e: any) {
-                    return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+                    return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
 
@@ -501,7 +512,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
             return { handled: true, response: deps.success({ success: true, registryRemoved, persisted }) };
         }
     } catch (e: any) {
-        return { handled: true, response: deps.error(e.message, e.statusCode || 500) };
+        return { handled: true, response: deps.errorFromThrown(e, 500) };
     }
 
     return { handled: false };

packages/runtime/src/domains/security.ts

@@ -94,8 +94,9 @@ export async function handleSecurityRequest(
     } catch (err: any) {
         // The service throws typed errors carrying their HTTP status:
         // PermissionDeniedError → 403, SuggestionNotFoundError → 404,
-        // SuggestionStateError → 409.
-        const status = typeof err?.statusCode === 'number' ? err.statusCode : 500;
-        return { handled: true, response: deps.error(err?.message ?? 'Security operation failed', status) };
+        // SuggestionStateError → 409. Read via `errorFromThrown` so `status`
+        // counts too, not just `statusCode` — the rest of the codebase's domain
+        // errors use the former (#3867).
+        return { handled: true, response: deps.errorFromThrown(err, 500) };
     }
 }

packages/runtime/src/domains/ui.ts

@@ -41,7 +41,7 @@ export async function handleUiRequest(
                 const result = await protocol.getUiView({ object: objectName, type });
                 return { handled: true, response: deps.success(result) };
             } catch (e: any) {
-                return { handled: true, response: deps.error(e.message, 500) };
+                return { handled: true, response: deps.errorFromThrown(e, 500) };
             }
         } else {
             return { handled: true, response: deps.error('Protocol service not available', 503) };