Skip to content

Commit e3498fb

Browse files
os-zhuangclaude
andauthored
fix(runtime): carry spec-validation issues + 422 status through metadata save/publish errors (#2589)
protocol.saveMetaItem throws a field-anchored spec-validation error (status 422, code invalid_metadata, issues:[{path,message,code}]), but the HTTP dispatcher collapsed it to a single message (save even hardcoded 400) and publish dropped issues from each failed[] entry. The Studio could therefore only show a generic banner, never point at the offending field. - Add errorFromThrown(e, fallback) dispatcher helper: preserves the error's own status (validation now surfaces as 422, not a downgraded 400) and attaches {code, issues} under error.details when present; unchanged for plain errors. Wired into the meta save (PUT /meta/:type/:name) and publish (POST /packages/:id/publish-drafts) catch sites. - publishPackageDrafts carries issues into each failed[] entry. Server half of "surface validation at the save/publish moment, on the field". Additive to the error payload; only behavior change is the more-correct 422. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f7606a1 commit e3498fb

4 files changed

Lines changed: 92 additions & 4 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/runtime": patch
4+
---
5+
6+
fix(runtime): carry spec-validation issues (and the 422 status) through metadata save/publish errors
7+
8+
`protocol.saveMetaItem` already validates a metadata draft against its spec Zod
9+
schema and, on failure, throws a rich error: HTTP `status: 422`, `code:
10+
'invalid_metadata'`, and a structured `issues: [{ path, message, code }]` array
11+
(field-anchored, `superRefine` issues included). But the HTTP dispatcher's catch
12+
blocks collapsed all of that to a single message — the save path even hardcoded
13+
`400` — so a client could only show a generic "failed validation" banner with no
14+
way to point at the offending field. The publish path was worse: the per-draft
15+
catch in `publishPackageDrafts` flattened each failure into `{ type, name, error
16+
}` and **dropped `issues` entirely**.
17+
18+
Now:
19+
- A new `errorFromThrown(e, fallbackStatus)` dispatcher helper preserves the
20+
error's own `status` (so validation surfaces as **422**, not a downgraded 400)
21+
and attaches `{ code, issues }` under `error.details` when present. Errors that
22+
carry neither behave exactly as before. Used by the metadata **save** (`PUT
23+
/meta/:type/:name`) and **publish** (`POST /packages/:id/publish-drafts`)
24+
catch sites.
25+
- `publishPackageDrafts` now carries `issues` into each `failed[]` entry, so a
26+
validation failure during publish is field-anchored too (it previously kept
27+
only the message).
28+
29+
This is the server half of "surface validation at the save/publish moment, on
30+
the field" — the Studio can now map each issue back to its input instead of
31+
showing a wall-of-text banner. Purely additive to the error payload; the only
32+
behavior change is the more-correct 422 (was 400) for a failed metadata save.

packages/metadata-protocol/src/protocol.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4355,7 +4355,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
43554355
const drafts = await repo.listDrafts({ packageId: request.packageId });
43564356

43574357
const published: Array<{ type: string; name: string; version: string }> = [];
4358-
const failed: Array<{ type: string; name: string; error: string; code?: string }> = [];
4358+
const failed: Array<{ type: string; name: string; error: string; code?: string; issues?: Array<{ path: string; message: string; code?: string }> }> = [];
43594359

43604360
// Structure first, seeds LAST — a seed's rows can only land after its
43614361
// object's table exists (publishMetaItem creates it). Within the seeds we
@@ -4431,6 +4431,10 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
44314431
name: d.name,
44324432
error: e?.message ?? 'publish failed',
44334433
...(e?.code ? { code: e.code } : {}),
4434+
// Carry structured spec-validation issues so the publish
4435+
// surface can point at the offending field, not just report
4436+
// "N failed" (this catch used to flatten them to a message).
4437+
...(Array.isArray(e?.issues) ? { issues: e.issues } : {}),
44344438
});
44354439
}
44364440
}

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,30 @@ describe('HttpDispatcher', () => {
119119
expect(result.response?.status).toBe(400);
120120
expect(result.response?.body?.error?.message).toBe('Save failed');
121121
});
122-
122+
123+
it('preserves the 422 status + structured spec-validation issues on save', async () => {
124+
// protocol.saveMetaItem throws a spec-validation error carrying the
125+
// field-anchored issues; the dispatcher must pass them through (not
126+
// flatten to a single 400 message) so the Studio can point at fields.
127+
const err: any = new Error('[invalid_metadata] object/bad failed spec validation: fields.amount.type: Required');
128+
err.code = 'invalid_metadata';
129+
err.status = 422;
130+
err.issues = [
131+
{ path: 'fields.amount.type', message: 'Required', code: 'invalid_type' },
132+
{ path: 'label', message: 'Required', code: 'invalid_type' },
133+
];
134+
mockProtocol.saveMetaItem.mockRejectedValue(err);
135+
136+
const result = await dispatcher.handleMetadata('/objects/bad', { request: {} }, 'PUT', {});
137+
138+
expect(result.handled).toBe(true);
139+
expect(result.response?.status).toBe(422); // NOT the old hardcoded 400
140+
const error = result.response?.body?.error;
141+
expect(error?.details?.code).toBe('invalid_metadata');
142+
expect(error?.details?.issues).toEqual(err.issues);
143+
expect(error?.details?.issues[0].path).toBe('fields.amount.type');
144+
});
145+
123146
it('should handle READ operations via ObjectQL registry', async () => {
124147
mockObjectQL.registry.getObject.mockReturnValue({ name: 'my_obj', fields: {} });
125148

packages/runtime/src/http-dispatcher.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,29 @@ export class HttpDispatcher {
216216
};
217217
}
218218

219+
/**
220+
* Build an error response from a THROWN service/protocol error, preserving
221+
* the error's own HTTP `status` and — critically — any structured `issues`
222+
* array (e.g. spec-validation `{ path, message, code }[]` from
223+
* `protocol.saveMetaItem`). The plain `error(msg, code)` path collapses a
224+
* validation failure to a single message, so the UI can only show a generic
225+
* banner; carrying `issues` (and the semantic `code`) in `details` lets it
226+
* map each error back to the offending field. Falls back to `fallbackStatus`
227+
* and behaves exactly like `error()` for errors that carry neither.
228+
*/
229+
private errorFromThrown(e: any, fallbackStatus = 500) {
230+
const status =
231+
typeof e?.status === 'number' ? e.status
232+
: typeof e?.statusCode === 'number' ? e.statusCode
233+
: fallbackStatus;
234+
const issues = Array.isArray(e?.issues) ? e.issues : undefined;
235+
const details =
236+
issues || e?.code
237+
? { ...(e?.code ? { code: e.code } : {}), ...(issues ? { issues } : {}) }
238+
: undefined;
239+
return this.error(e?.message ?? String(e), status, details);
240+
}
241+
219242
/**
220243
* ADR-0046: `doc` list responses omit `content` by default — manuals
221244
* are the one metadata payload that grows unbounded, and the list
@@ -1489,7 +1512,10 @@ export class HttpDispatcher {
14891512
const result = await protocol.saveMetaItem({ type, name, item: body, organizationId, ...(packageId ? { packageId } : {}) });
14901513
return { handled: true, response: this.success(result) };
14911514
} catch (e: any) {
1492-
return { handled: true, response: this.error(e.message, 400) };
1515+
// Preserve the 422 + structured spec-validation `issues` so
1516+
// the Studio can point at the offending field, not just a
1517+
// generic banner (the old path hardcoded 400 + dropped them).
1518+
return { handled: true, response: this.errorFromThrown(e, 400) };
14931519
}
14941520
}
14951521

@@ -2200,7 +2226,10 @@ export class HttpDispatcher {
22002226
}
22012227
return { handled: true, response: this.success(result) };
22022228
} catch (e: any) {
2203-
return { handled: true, response: this.error(e.message, e.statusCode || 500) };
2229+
// Carry spec-validation `issues` (and the real 422 status —
2230+
// the protocol sets `.status`, not `.statusCode`) through to
2231+
// the publish surface so failures are field-anchored.
2232+
return { handled: true, response: this.errorFromThrown(e, 500) };
22042233
}
22052234
}
22062235
return { handled: true, response: this.error('Draft publishing not supported', 501) };

0 commit comments

Comments
 (0)