Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .changeset/meta-validation-issues-passthrough.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 5 additions & 1 deletion packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 } : {}),
});
}
}
Expand Down
25 changes: 24 additions & 1 deletion packages/runtime/src/http-dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} });

Expand Down
33 changes: 31 additions & 2 deletions packages/runtime/src/http-dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) };
}
}

Expand Down Expand Up @@ -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) };
Expand Down