diff --git a/.changeset/rest-union-branch-field-errors.md b/.changeset/rest-union-branch-field-errors.md new file mode 100644 index 0000000000..2cd26c2476 --- /dev/null +++ b/.changeset/rest-union-branch-field-errors.md @@ -0,0 +1,34 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): 联合类型分支里的拒绝理由现在能到达调用方,不再只剩 `Invalid input` (#5014) + +zod 会把一个失配的 `z.union([...])` 折叠成**一条**顶层 `invalid_union` issue,它自己的 +`message` 是裸的 `"Invalid input"`;每个分支真正的抱怨——包括 #4001 那批 `strictObject` +写下的处方文案——躺在 `issue.errors` 里(每分支一个数组)。`zodIssuesToFields` 过去只映射 +顶层 issue,于是 `POST /api/v1/data/:object/query` 对着 +`{"search": {"fields": ["name"]}}` 只回一条 + +``` +{ "field": "query.search", "code": "invalid_shape", "message": "Invalid input" } +``` + +——说清「缺的是 `query` 这个键」的那句话被生产出来,然后被丢掉。同一个坑在 +`QuerySchema.groupBy` 的联合分支上一样:`dateGranularity` 写错值,作者拿不到那份 +「可选 day/week/month/quarter/year」的清单。 + +现在 `fields[]` 会在联合条目**之后**追加解释它的分支条目,`field` 用分支路径拼上联合自身 +的路径(`query.search.query`),`code` 照常走 ADR-0114 D3 的目录映射——所以缺键报 +`required` 而不是 `invalid_type`(这一判定要走绝对路径去读入参,分支路径是相对的)。 + +分支选择策略直接沿用 #4971 给 CLI/spec 侧 `formatZodError` 落的那一套:只报根部 +KIND 不匹配的分支整支丢弃(全部如此则不展开,输出和以前逐字一致);剩下的**报得最少的 +分支胜出**——这条是防止「一个拼错的键被 N 个分支各报一遍」的机制本身;`unrecognized_keys` +破平局;声明顺序破剩下的;真正并列的分支全部输出(上限 3 条);跨分支重复的相同结论只 +出现一次;嵌套联合按绝对路径递归,深度上限 3。两侧必须给出**同一个判定**,否则同一个错误 +从终端发布和从 API 提交会得到两套说法。 + +对 wire 而言这是**纯追加**:原有的每一条 `fields[]` 条目——包括联合自身那条——`field` / +`code` / `message` 和相对次序都不变,新条目插在它解释的那条之后。信封形状仍与 +`mapDataError` 同形(ADR-0114),数组长度从来不是契约的一部分。 diff --git a/content/docs/api/error-handling-server.mdx b/content/docs/api/error-handling-server.mdx index 08c906d03b..622b78a5dc 100644 --- a/content/docs/api/error-handling-server.mdx +++ b/content/docs/api/error-handling-server.mdx @@ -176,6 +176,12 @@ import { zodIssuesToFields } from '@objectstack/rest'; // `too_small` becomes `min_length` / `min_value` / `min_items` depending on what // was too small, and a MISSING property becomes `required` instead of the // `invalid_type` Zod reports for it. +// +// One entry per issue is the common case, NOT a guarantee: Zod folds a failed +// `z.union` into a single issue whose message is the bare "Invalid input", so a +// union failure produces that entry PLUS the entries of the branch that explains +// it, addressed with the union's own path (#5014). Read `fields.length` as the +// number of field errors, never as the number of Zod issues. function createValidationError(zodError: z.ZodError, input?: unknown): AppError { const fields = zodIssuesToFields(zodError.issues, input); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index b64d0899ae..618bf97b66 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -114,7 +114,7 @@ async function isTranslatableMetaType(type: string): Promise { * string, received undefined), so passing it through marked a missing input as a * type error. */ -function zodIssueToFieldCode(issue: any, input?: unknown, inputProvided = false): FieldErrorCode { +function zodIssueToFieldCode(issue: any, path: unknown, input?: unknown, inputProvided = false): FieldErrorCode { const origin = issue?.origin; switch (issue?.code) { case 'too_small': @@ -138,8 +138,13 @@ function zodIssueToFieldCode(issue: any, input?: unknown, inputProvided = false) // keep `invalid_type`: reading "received undefined" out of the message // would make the wire contract depend on Zod's phrasing, which is the // leak this mapping exists to stop. + // + // `path` is passed in rather than read off the issue because a union + // BRANCH issue carries a path relative to the union (#5014): walking + // the relative one would read the wrong slot of the input — usually + // `undefined` — and report every branch mismatch as `required`. if (!inputProvided) return 'invalid_type'; - return valueAtPath(input, issue?.path) === undefined ? 'required' : 'invalid_type'; + return valueAtPath(input, path) === undefined ? 'required' : 'invalid_type'; } case 'invalid_value': // A closed set (`z.enum`, `z.literal`) the value is not a member of. @@ -170,6 +175,163 @@ function valueAtPath(input: unknown, path: unknown): unknown { return cur; } +/** + * How many levels of nested `invalid_union` are expanded below a top-level + * issue, and how many equally-informative branches are emitted at one level. + * + * Both bounds — and the whole selection policy below — are the ones + * `formatZodError` landed for the CLI/spec side of this defect (#4971, + * `spec/src/shared/error-map.zod.ts`). They are duplicated rather than imported + * because spec exports only the STRING renderer (`formatZodIssue`), and the wire + * needs structured `{field, code, message}` entries; the *verdict* must match all + * the same, or one mistake gets two different prescriptions depending on whether + * the author published from the terminal or POSTed to the API (#5014). + */ +const UNION_EXPANSION_DEPTH_LIMIT = 3; +const UNION_BRANCH_EMIT_LIMIT = 3; + +/** A Zod issue path, normalised to the array Zod always produces. */ +function issuePathOf(issue: any): Array { + return Array.isArray(issue?.path) ? issue.path : []; +} + +/** + * True when a branch only complains that the value is the wrong *kind* at the + * branch root — `expected string, received object` for the string member of + * `z.union([z.string(), SomeObject])`. + * + * Such a branch carries no prescription: the author never intended it, and + * emitting it is the "N branches, N times the noise" failure. An empty branch + * (zod's "matched multiple" variant carries `errors: []`) counts as + * uninformative too — `every` on an empty list is `true`. + */ +function isKindMismatchOnly(issues: readonly any[]): boolean { + return issues.every( + (issue) => + issuePathOf(issue).length === 0 + && (issue?.code === 'invalid_type' || issue?.code === 'invalid_value'), + ); +} + +/** True when a branch carries the #4001 campaign's unknown-key prescription. */ +function carriesUnknownKey(issues: readonly any[]): boolean { + return issues.some((issue) => issue?.code === 'unrecognized_keys'); +} + +/** + * Pick the branch(es) of a failed union whose issues actually explain the + * failure. Ranking, in order (identical to `selectUnionBranches` in + * `spec/src/shared/error-map.zod.ts`): + * + * 1. **Kind-mismatch-only branches are dropped entirely.** If *every* branch is + * one — a plain `z.union([z.string(), z.number()])` handed an object — + * nothing is selected and the union reports exactly what it always has. + * 2. **Fewest issues wins.** The branch the author was closest to hitting + * complains least, so "fewest" is what keeps ONE unknown key from arriving as + * N `fields[]` entries, one per branch. + * 3. **A branch carrying `unrecognized_keys` breaks a tie**, because that is + * where the curated prose lives. + * 4. Declaration order breaks what remains, so the wire is deterministic. + * + * Branches that tie at the top are all emitted (capped): when two shapes explain + * the failure equally well, privileging the first by accident of declaration + * order would be a lie about which shape was expected. + */ +function selectUnionBranches(branches: readonly (readonly any[])[]): readonly (readonly any[])[] { + const informative = branches + .map((issues, index) => ({ issues, index })) + .filter((branch) => !isKindMismatchOnly(branch.issues)); + if (informative.length === 0) return []; + + const rank = (branch: { issues: readonly any[] }): [number, number] => [ + branch.issues.length, + carriesUnknownKey(branch.issues) ? 0 : 1, + ]; + + const sorted = [...informative].sort((a, b) => { + const [aCount, aKeys] = rank(a); + const [bCount, bKeys] = rank(b); + return aCount - bCount || aKeys - bKeys || a.index - b.index; + }); + + const [bestCount, bestKeys] = rank(sorted[0]!); + return sorted + .filter((branch) => { + const [count, keys] = rank(branch); + return count === bestCount && keys === bestKeys; + }) + .slice(0, UNION_BRANCH_EMIT_LIMIT) + .map((branch) => branch.issues); +} + +/** + * One issue → its `fields[]` entries, appended to `out`. + * + * An ordinary issue is one entry. An `invalid_union` is its own entry (zod's + * bare `"Invalid input"`, mapped to `invalid_shape`) FOLLOWED by the entries of + * the branches that explain it, with `field` resolved against the union's own + * path — branch paths are relative to it. + * + * The union's entry is kept rather than replaced: it is the only entry naming + * the slot the client sent, existing clients already read it, and when every + * branch is uninformative it is still the whole answer. So the expansion is + * strictly ADDITIVE — no entry that shipped before this changed is gone or + * renumbered, only newly accompanied (ADR-0114: same `{field, code, message}` + * shape as {@link mapDataError}, which has never bounded the array's length). + * + * `seen` de-duplicates entries *within one top-level issue*: two branches that + * reject the same key with the same words say it once. Union entries themselves + * are exempt, since two same-path `"Invalid input"` entries can head genuinely + * different sub-trees. + * + * Deliberate divergence from the spec-side renderer: where it prints a trailing + * "… and N more branches rejected this value", this emits nothing. That line is + * a rendering affordance; a `fields[]` entry must name a real field and carry a + * catalog code, and the omission note has neither. + */ +function collectIssueFields( + issue: any, + parentPath: Array, + depth: number, + seen: Set, + input: unknown, + inputProvided: boolean, + out: Array<{ field: string; code: FieldErrorCode; message: string }>, +): void { + const ownPathIsArray = Array.isArray(issue?.path); + const path = ownPathIsArray ? [...parentPath, ...issue.path] : parentPath; + const field = ownPathIsArray + ? path.join('.') + : [...parentPath, String(issue?.path ?? '')].join('.'); + + const branches: readonly (readonly any[])[] = issue?.code === 'invalid_union' && Array.isArray(issue?.errors) + ? issue.errors.filter((branch: unknown): branch is any[] => Array.isArray(branch)) + : []; + const expandable = branches.length > 0 && depth < UNION_EXPANSION_DEPTH_LIMIT; + + const entry = { + field, + // A non-array path keeps the pre-#5014 reading (`valueAtPath` bails and + // the mapper stays conservative) instead of being coerced into one. + code: zodIssueToFieldCode(issue, ownPathIsArray ? path : issue?.path, input, inputProvided), + message: String(issue?.message ?? 'Invalid value'), + }; + + if (!expandable) { + const key = JSON.stringify([entry.field, entry.code, entry.message]); + if (seen.has(key)) return; + seen.add(key); + } + out.push(entry); + if (!expandable) return; + + for (const branch of selectUnionBranches(branches)) { + for (const nested of branch) { + collectIssueFields(nested, path, depth + 1, seen, input, inputProvided, out); + } + } +} + /** * Zod issues → the data surface's `fields[]` validation envelope * (`{ field, code, message }`, docs/api/wire-format §7). @@ -180,6 +342,13 @@ function valueAtPath(input: unknown, path: unknown): unknown { * per route, and `code: 'VALIDATION_FAILED'` stops meaning one thing on the * wire. Since ADR-0114 that sameness covers the `code` VALUE too, not just the * shape: see {@link zodIssueToFieldCode}. + * + * A rejection behind a `z.union` is expanded (#5014): zod folds every branch of + * a failed union into ONE top-level issue whose message is the literal + * `"Invalid input"`, so mapping only top-level issues put `{field: 'query.search', + * code: 'invalid_shape', message: 'Invalid input'}` on the wire while the branch + * that says WHICH key is wrong — required-property and unknown-key prescriptions + * alike — was produced and dropped. See {@link collectIssueFields}. */ export function zodIssuesToFields( issues: unknown, @@ -187,11 +356,13 @@ export function zodIssuesToFields( ): Array<{ field: string; code: FieldErrorCode; message: string }> { if (!Array.isArray(issues)) return []; const inputProvided = input.length > 0; - return issues.map((i: any) => ({ - field: Array.isArray(i?.path) ? i.path.join('.') : String(i?.path ?? ''), - code: zodIssueToFieldCode(i, input[0], inputProvided), - message: String(i?.message ?? 'Invalid value'), - })); + const out: Array<{ field: string; code: FieldErrorCode; message: string }> = []; + for (const issue of issues) { + // A fresh `seen` per top-level issue: de-duplication is about one + // union's branches agreeing, never about two independent issues. + collectIssueFields(issue, [], 0, new Set(), input[0], inputProvided, out); + } + return out; } export function mapDataError(error: any, object?: string): { status: number; body: Record } { diff --git a/packages/rest/src/zod-union-fields.test.ts b/packages/rest/src/zod-union-fields.test.ts new file mode 100644 index 0000000000..54afe399c4 --- /dev/null +++ b/packages/rest/src/zod-union-fields.test.ts @@ -0,0 +1,240 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { z } from 'zod'; +import { FieldErrorCode } from '@objectstack/spec/api'; +// `.js` extension deliberately: under `moduleResolution: NodeNext` an +// extensionless relative import does not resolve, every symbol it names becomes +// `any`, and each callback over those symbols then reports TS7006 (AGENTS.md +// §Build & Test). This is the same spelling `rest-server.ts` itself uses. +import { zodIssuesToFields } from './rest-server.js'; + +/** + * `invalid_union` expansion on the wire (#5014). + * + * Zod folds a failed `z.union([...])` into ONE top-level issue whose message is + * the literal `"Invalid input"`; every branch's real complaint — including the + * curated `strictObject` prescriptions the #4001 campaign wrote — lives in + * `issue.errors`, one array per branch. Mapping only top-level issues therefore + * put `{field: 'query.search', code: 'invalid_shape', message: 'Invalid input'}` + * on the wire and dropped the sentence that says what to fix. + * + * Like `zod-field-codes.test.ts`, every case drives a REAL `safeParse`: the + * expansion reads `issue.errors`, which is Zod's internal shape, so a test built + * on a hand-written issue would keep passing after a Zod upgrade moved it while + * the wire silently went back to reporting nothing but `Invalid input`. + * + * The selection policy is the one `formatZodError` landed for the terminal in + * #4971 (`spec/src/shared/error-map.zod.ts`). The two must agree in VERDICT — + * an author who publishes from the CLI and an author who POSTs the same mistake + * must be told the same thing — so the cases below pin the parts of the policy + * that decide *which* branches speak, not just that some branch does. + */ +describe('zodIssuesToFields — invalid_union expansion (#5014)', () => { + const fieldsFor = (schema: z.ZodType, value: unknown) => { + const r = schema.safeParse(value); + expect(r.success, 'the fixture must actually fail to parse').toBe(false); + return zodIssuesToFields((r as { error: { issues: unknown[] } }).error.issues, value); + }; + + describe('the live REST ingress path', () => { + /** + * `POST /api/v1/data/:object/query` parses the body against + * `FindDataRequestSchema` (#3899) and reports through `zodIssuesToFields`. + * `QuerySchema.search` is `z.union([z.string(), FullTextSearchSchema])`, so + * a structured search missing its required `query` text is exactly the + * defect: the union is all the author used to be told. + */ + it('delivers a branch prescription instead of a bare Invalid input', async () => { + const { FindDataRequestSchema } = await import('@objectstack/spec/api'); + const body = { object: 'account', query: { object: 'account', search: { fields: ['name'] } } }; + const fields = fieldsFor(FindDataRequestSchema as unknown as z.ZodType, body); + + // The union's own entry is still there, unchanged — clients read it today. + expect(fields).toContainEqual({ + field: 'query.search', + code: 'invalid_shape', + message: 'Invalid input', + }); + + // …and now the branch that explains it reaches the author. `required`, + // not `invalid_type`: the branch path is RELATIVE to the union, so the + // input walk that tells "missing" from "wrong type" only lands on the + // right slot once the path is resolved against the union's own. + const prescription = fields.find((f) => f.field === 'query.search.query'); + expect(prescription, JSON.stringify(fields)).toBeDefined(); + expect(prescription!.code).toBe('required'); + expect(prescription!.message).not.toBe('Invalid input'); + }); + + it('delivers the enum prescription from inside groupBy union arm', async () => { + const { FindDataRequestSchema } = await import('@objectstack/spec/api'); + // `GroupByNodeSchema` = z.union([z.string(), {field, dateGranularity?, alias?}]). + const body = { + object: 'account', + query: { object: 'account', groupBy: [{ field: 'closed_at', dateGranularity: 'decade' }] }, + }; + const fields = fieldsFor(FindDataRequestSchema as unknown as z.ZodType, body); + + const prescription = fields.find((f) => f.field === 'query.groupBy.0.dateGranularity'); + expect(prescription, JSON.stringify(fields)).toBeDefined(); + expect(prescription!.code).toBe('invalid_option'); + // The list of what IS accepted is the whole point of carrying it over. + expect(prescription!.message).toContain('quarter'); + }); + }); + + describe('branch selection', () => { + /** + * The regression the fewest-issues rule exists to prevent (the #4001 批 6c + * shape): expanding every branch reports ONE unknown key once per branch, + * so a three-member union turns a single typo into three `fields[]` + * entries — noisier than the `Invalid input` it replaced. + */ + it('reports a single unknown key once, not once per branch', () => { + const member = (kind: string) => + z.object({ kind: z.literal(kind), [`${kind}Value`]: z.string() }).strict(); + const schema = z.object({ + widget: z.union([member('metric'), member('chart'), member('table')]), + }); + + const fields = fieldsFor(schema, { + widget: { kind: 'metric', metricValue: 'x', typo: 1 }, + }); + + const unknown = fields.filter((f) => f.code === 'unknown_field'); + expect(unknown, JSON.stringify(fields)).toHaveLength(1); + expect(unknown[0].field).toBe('widget'); + expect(unknown[0].message).toContain('typo'); + }); + + it('expands nothing when every branch is only a kind mismatch', () => { + // `z.union([z.string(), z.number()])` handed an object: no branch has + // anything to say beyond "not my type", and naming both is noise. + const fields = fieldsFor(z.object({ u: z.union([z.string(), z.number()]) }), { u: {} }); + expect(fields).toEqual([{ field: 'u', code: 'invalid_shape', message: 'Invalid input' }]); + }); + + it('prefers the branch carrying unrecognized_keys when issue counts tie', () => { + const schema = z.object({ + u: z.union([ + // 1 issue: `a` is the wrong type. + z.object({ a: z.string() }), + // 1 issue too — but it is the unknown-key prescription, which is + // where the curated prose lives, so declaration order must not + // decide this one. + z.object({ b: z.string().optional() }).strict(), + ]), + }); + const fields = fieldsFor(schema, { u: { a: 1 } }); + const unknown = fields.find((f) => f.field === 'u' && f.code === 'unknown_field'); + expect(unknown, JSON.stringify(fields)).toBeDefined(); + expect(unknown!.message).toContain('a'); + // The loser branch's own complaint stays behind: one union, one story. + expect(fields.some((f) => f.field === 'u.a')).toBe(false); + }); + + it('emits identical cross-branch verdicts once', () => { + // Two members that reject the same key with the same words: the author + // has one mistake, so the wire carries one entry. + const same = () => z.object({ shared: z.string() }); + const fields = fieldsFor(z.object({ u: z.union([same(), same()]) }), { u: { shared: 1 } }); + expect(fields.filter((f) => f.field === 'u.shared')).toHaveLength(1); + }); + + it('caps how many tied branches are emitted', () => { + const member = (n: number) => z.object({ [`k${n}`]: z.string() }); + const schema = z.object({ u: z.union([member(1), member(2), member(3), member(4), member(5)]) }); + const fields = fieldsFor(schema, { u: {} }); + // 1 union anchor + exactly 3 of the 5 tied branches: an author fixing + // one shape does not need the other four spelled out, and an unbounded + // expansion is how a union with many members becomes unreadable. + expect(fields).toHaveLength(4); + expect(fields[0]).toEqual({ field: 'u', code: 'invalid_shape', message: 'Invalid input' }); + }); + }); + + describe('paths and codes of expanded entries', () => { + it('resolves branch paths against the union path, through nesting', () => { + const inner = z.union([z.string(), z.object({ kind: z.enum(['a', 'b']) }).strict()]); + const outer = z.object({ w: z.union([z.number(), z.object({ nest: inner }).strict()]) }); + const fields = fieldsFor(outer, { w: { nest: { kind: 'zzz' } } }); + + expect(fields.map((f) => f.field)).toContain('w.nest.kind'); + const leaf = fields.find((f) => f.field === 'w.nest.kind')!; + expect(leaf.code).toBe('invalid_option'); + }); + + it('tells missing from wrong-typed inside a branch, using the absolute path', () => { + // The relative-path trap, pinned from both sides: `a` is PRESENT with + // the wrong type, `b` is absent. Walking the branch-relative path would + // miss both values and report each as `required`. + const schema = z.object({ u: z.union([z.string(), z.object({ a: z.string(), b: z.string() })]) }); + const fields = fieldsFor(schema, { u: { a: 1 } }); + expect(fields.find((f) => f.field === 'u.a')!.code).toBe('invalid_type'); + expect(fields.find((f) => f.field === 'u.b')!.code).toBe('required'); + }); + + it('keeps every expanded code inside the ADR-0114 catalog', () => { + const schema = z.object({ + u: z.union([ + z.string(), + z.object({ + n: z.number().min(3), + s: z.string().email(), + arr: z.array(z.string()).max(1), + }).strict(), + ]), + }); + const fields = fieldsFor(schema, { u: { n: 1, s: 'nope', arr: ['a', 'b'], extra: true } }); + expect(fields.length).toBeGreaterThan(1); + for (const f of fields) { + expect(() => FieldErrorCode.parse(f.code), `'${f.code}' is not a catalog member`).not.toThrow(); + } + }); + }); + + describe('wire compatibility (ADR-0114)', () => { + /** + * The expansion is ADDITIVE. Every entry the route emitted before still + * appears, in the same order, with the same `field`/`code`/`message`; + * branch entries are inserted after the union entry they explain. Nothing + * about `{field, code, message}` changes, which is what ADR-0114 requires + * of anything sharing `mapDataError`'s envelope — the array's LENGTH was + * never part of that contract. + */ + it('leaves non-union issues mapped exactly one-to-one', () => { + const schema = z.object({ a: z.string(), b: z.number() }); + const fields = fieldsFor(schema, { a: 1 }); + expect(fields).toEqual([ + { field: 'a', code: 'invalid_type', message: expect.any(String) }, + { field: 'b', code: 'required', message: expect.any(String) }, + ]); + }); + + it('keeps the union entry first and unchanged among its expansion', () => { + const schema = z.object({ + a: z.string(), + u: z.union([z.string(), z.object({ deep: z.string() })]), + z: z.number(), + }); + const fields = fieldsFor(schema, { a: 'ok', u: {}, z: 1 }); + const idx = fields.findIndex((f) => f.field === 'u'); + expect(fields[idx]).toEqual({ field: 'u', code: 'invalid_shape', message: 'Invalid input' }); + // The independent `z`/`a` issues keep their own relative order. + expect(fields.filter((f) => f.field === 'u.deep')).toHaveLength(1); + expect(fields.findIndex((f) => f.field === 'u.deep')).toBe(idx + 1); + }); + + it('still tolerates junk in place of an issue list', () => { + for (const junk of [null, undefined, {}, 'issues', 0]) { + expect(zodIssuesToFields(junk)).toEqual([]); + } + // …and an `invalid_union` whose `errors` is absent or malformed. + expect(zodIssuesToFields([{ code: 'invalid_union', path: ['x'], message: 'Invalid input' }])) + .toEqual([{ field: 'x', code: 'invalid_shape', message: 'Invalid input' }]); + expect(zodIssuesToFields([{ code: 'invalid_union', path: ['x'], message: 'Invalid input', errors: 'nope' }])) + .toEqual([{ field: 'x', code: 'invalid_shape', message: 'Invalid input' }]); + }); + }); +});