diff --git a/.changeset/meta-validation-issues-passthrough.md b/.changeset/meta-validation-issues-passthrough.md new file mode 100644 index 0000000000..27024f1612 --- /dev/null +++ b/.changeset/meta-validation-issues-passthrough.md @@ -0,0 +1,32 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/runtime": patch +--- + +fix(runtime): carry spec-validation issues (and the 422 status) through metadata save/publish errors + +`protocol.saveMetaItem` already validates a metadata draft against its spec Zod +schema and, on failure, throws a rich error: HTTP `status: 422`, `code: +'invalid_metadata'`, and a structured `issues: [{ path, message, code }]` array +(field-anchored, `superRefine` issues included). But the HTTP dispatcher's catch +blocks collapsed all of that to a single message — the save path even hardcoded +`400` — so a client could only show a generic "failed validation" banner with no +way to point at the offending field. The publish path was worse: the per-draft +catch in `publishPackageDrafts` flattened each failure into `{ type, name, error +}` and **dropped `issues` entirely**. + +Now: +- A new `errorFromThrown(e, fallbackStatus)` dispatcher helper preserves the + error's own `status` (so validation surfaces as **422**, not a downgraded 400) + and attaches `{ code, issues }` under `error.details` when present. Errors that + carry neither behave exactly as before. Used by the metadata **save** (`PUT + /meta/:type/:name`) and **publish** (`POST /packages/:id/publish-drafts`) + catch sites. +- `publishPackageDrafts` now carries `issues` into each `failed[]` entry, so a + validation failure during publish is field-anchored too (it previously kept + only the message). + +This is the server half of "surface validation at the save/publish moment, on +the field" — the Studio can now map each issue back to its input instead of +showing a wall-of-text banner. Purely additive to the error payload; the only +behavior change is the more-correct 422 (was 400) for a failed metadata save. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 417cabe10d..f1b0e50570 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4355,7 +4355,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { const drafts = await repo.listDrafts({ packageId: request.packageId }); const published: Array<{ type: string; name: string; version: string }> = []; - const failed: Array<{ type: string; name: string; error: string; code?: string }> = []; + const failed: Array<{ type: string; name: string; error: string; code?: string; issues?: Array<{ path: string; message: string; code?: string }> }> = []; // Structure first, seeds LAST — a seed's rows can only land after its // object's table exists (publishMetaItem creates it). Within the seeds we @@ -4431,6 +4431,10 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { name: d.name, error: e?.message ?? 'publish failed', ...(e?.code ? { code: e.code } : {}), + // Carry structured spec-validation issues so the publish + // surface can point at the offending field, not just report + // "N failed" (this catch used to flatten them to a message). + ...(Array.isArray(e?.issues) ? { issues: e.issues } : {}), }); } } diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 27984ee528..c5d01f6396 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -119,7 +119,30 @@ describe('HttpDispatcher', () => { expect(result.response?.status).toBe(400); expect(result.response?.body?.error?.message).toBe('Save failed'); }); - + + it('preserves the 422 status + structured spec-validation issues on save', async () => { + // protocol.saveMetaItem throws a spec-validation error carrying the + // field-anchored issues; the dispatcher must pass them through (not + // flatten to a single 400 message) so the Studio can point at fields. + const err: any = new Error('[invalid_metadata] object/bad failed spec validation: fields.amount.type: Required'); + err.code = 'invalid_metadata'; + err.status = 422; + err.issues = [ + { path: 'fields.amount.type', message: 'Required', code: 'invalid_type' }, + { path: 'label', message: 'Required', code: 'invalid_type' }, + ]; + mockProtocol.saveMetaItem.mockRejectedValue(err); + + const result = await dispatcher.handleMetadata('/objects/bad', { request: {} }, 'PUT', {}); + + expect(result.handled).toBe(true); + expect(result.response?.status).toBe(422); // NOT the old hardcoded 400 + const error = result.response?.body?.error; + expect(error?.details?.code).toBe('invalid_metadata'); + expect(error?.details?.issues).toEqual(err.issues); + expect(error?.details?.issues[0].path).toBe('fields.amount.type'); + }); + it('should handle READ operations via ObjectQL registry', async () => { mockObjectQL.registry.getObject.mockReturnValue({ name: 'my_obj', fields: {} }); diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 2b4c0d41ba..845956e94b 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -216,6 +216,29 @@ export class HttpDispatcher { }; } + /** + * Build an error response from a THROWN service/protocol error, preserving + * the error's own HTTP `status` and — critically — any structured `issues` + * array (e.g. spec-validation `{ path, message, code }[]` from + * `protocol.saveMetaItem`). The plain `error(msg, code)` path collapses a + * validation failure to a single message, so the UI can only show a generic + * banner; carrying `issues` (and the semantic `code`) in `details` lets it + * map each error back to the offending field. Falls back to `fallbackStatus` + * and behaves exactly like `error()` for errors that carry neither. + */ + private errorFromThrown(e: any, fallbackStatus = 500) { + const status = + typeof e?.status === 'number' ? e.status + : typeof e?.statusCode === 'number' ? e.statusCode + : fallbackStatus; + const issues = Array.isArray(e?.issues) ? e.issues : undefined; + const details = + issues || e?.code + ? { ...(e?.code ? { code: e.code } : {}), ...(issues ? { issues } : {}) } + : undefined; + return this.error(e?.message ?? String(e), status, details); + } + /** * ADR-0046: `doc` list responses omit `content` by default — manuals * are the one metadata payload that grows unbounded, and the list @@ -1489,7 +1512,10 @@ export class HttpDispatcher { const result = await protocol.saveMetaItem({ type, name, item: body, organizationId, ...(packageId ? { packageId } : {}) }); return { handled: true, response: this.success(result) }; } catch (e: any) { - return { handled: true, response: this.error(e.message, 400) }; + // Preserve the 422 + structured spec-validation `issues` so + // the Studio can point at the offending field, not just a + // generic banner (the old path hardcoded 400 + dropped them). + return { handled: true, response: this.errorFromThrown(e, 400) }; } } @@ -2200,7 +2226,10 @@ export class HttpDispatcher { } return { handled: true, response: this.success(result) }; } catch (e: any) { - return { handled: true, response: this.error(e.message, e.statusCode || 500) }; + // Carry spec-validation `issues` (and the real 422 status — + // the protocol sets `.status`, not `.statusCode`) through to + // the publish surface so failures are field-anchored. + return { handled: true, response: this.errorFromThrown(e, 500) }; } } return { handled: true, response: this.error('Draft publishing not supported', 501) };