From 79cce0d48c84c61dfc7722fe777d775a8bfa75ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ad=C3=A1mek?= Date: Fri, 4 Sep 2026 14:56:10 +0200 Subject: [PATCH] fix(validations): adopt the richer error formatting from apify-client --- packages/validations/src/validation.ts | 99 ++++++++++-- test/validations.test.ts | 211 +++++++++++++++++++++++++ 2 files changed, 299 insertions(+), 11 deletions(-) create mode 100644 test/validations.test.ts diff --git a/packages/validations/src/validation.ts b/packages/validations/src/validation.ts index fc7ce0b81..063c20bc9 100644 --- a/packages/validations/src/validation.ts +++ b/packages/validations/src/validation.ts @@ -1,5 +1,8 @@ import { z } from 'zod'; +// Messages are formatted (and asserted on by consumers) in English, regardless of any global locale. +const { localeError } = z.locales.en(); + /** Formats a zod issue path like `groups[0]` or `countryCode`. */ function formatIssuePath(path: readonly PropertyKey[]): string { let out = ''; @@ -31,7 +34,10 @@ function describeType(value: unknown): string { const BARE_EXPECTED_TYPE_MESSAGE = /^Invalid input: expected (an array of .+|a typed array|an object|object|array|function|number|string|boolean)$/; -/** Longest received string rendered in an error; the rest is elided. */ +/** + * How much of a received string the message renders. A rejected argument can be arbitrarily large - a + * whole JSON payload passed where an object was expected - and its full text would swamp the message. + */ const MAX_RENDERED_STRING_LENGTH = 200; /** Renders a primitive received value for an error; skips objects/Dates (noisy). */ @@ -45,8 +51,10 @@ function describeReceived(value: unknown): string | undefined { : value; case 'number': case 'boolean': - case 'bigint': return String(value); + case 'bigint': + // Keep the `n` suffix, so a rejected bigint is not mistaken for a number. + return `${value}n`; default: return undefined; } @@ -62,15 +70,78 @@ function describeReceivedClause(value: unknown): string { : `received the ${describeType(value)} \`${rendered}\``; } -/** Renders one issue as a line each; a union expands into a line per failed arm. */ -function formatIssue(issue: z.ZodError['issues'][number], root: unknown, basePath: readonly PropertyKey[]): string[] { +/** + * Renders the issue's own sentence, except where zod's contradicts itself: a value of the expected type + * that fails that type's implicit constraint is still reported as the wrong *type*, giving "expected + * number, received number" for `Infinity` / `NaN` and "expected date, received Date" for an invalid + * `Date`. Name the constraint that actually failed instead. + */ +function describeIssue(issue: z.ZodError['issues'][number], value: unknown): string { + if (issue.code === 'invalid_type') { + if (issue.expected === 'number' && typeof value === 'number') { + return 'Invalid input: expected a finite number'; + } + // A tag check, not `instanceof`, so a `Date` from another realm is named too. + if (issue.expected === 'date' && Object.prototype.toString.call(value) === '[object Date]') { + return 'Invalid input: expected a valid date'; + } + } + return issue.message; +} + +/** + * How many issue lines the message renders, before a closing "... and N more" line. Validating a + * large array - a dataset push, a request batch - can fail on every element, and rendering all of + * them would make the message megabytes long. The full set stays on `issues` either way. + */ +const MAX_RENDERED_LINES = 10; + +/** + * How deep into the value the lines for `issue` would sit, as a path length. Computed without + * rendering anything, so a union can weigh its arms before any string is built. + */ +function deepestIssueDepth(issue: z.ZodError['issues'][number], baseDepth: number): number { + const depth = baseDepth + issue.path.length; + if (issue.code === 'invalid_union') { + let deepest = -1; + for (const arm of issue.errors) { + for (const nested of arm) deepest = Math.max(deepest, deepestIssueDepth(nested, depth)); + } + return deepest; + } + return depth; +} + +/** Collects one line per issue into `lines`; a union expands into a line per deepest-failing arm. */ +function collectIssueLines( + issue: z.ZodError['issues'][number], + root: unknown, + basePath: readonly PropertyKey[], + lines: string[], + counter: { total: number }, +): void { const path = [...basePath, ...issue.path]; - // A union's own message is a bare "Invalid input" — the useful part is in `errors`, + // A union's own message is a bare "Invalid input" - the useful part is in `errors`, // whose paths are relative to the union, hence passing `path` down as the base. if (issue.code === 'invalid_union') { - return issue.errors.flatMap((arm) => arm.flatMap((nested) => formatIssue(nested, root, path))); + // Only the arms that reached deepest are reported. An arm that failed nearer the root rejected a + // shape the value never had - for `[{ ok: 1 }, 2]` against `object | string | array`, the object + // and string arms fail on the whole array, and only the array arm can point at `[1]`. When every + // arm fails at the same depth, as for an argument of an outright wrong type, they are all kept. + const armDepths = issue.errors.map((arm) => + arm.reduce((deepest, nested) => Math.max(deepest, deepestIssueDepth(nested, path.length)), -1), + ); + const deepest = Math.max(...armDepths); + for (const [index, arm] of issue.errors.entries()) { + if (armDepths[index] !== deepest) continue; + for (const nested of arm) collectIssueLines(nested, root, path, lines, counter); + } + return; } + counter.total += 1; + if (lines.length >= MAX_RENDERED_LINES) return; + const location = path.length ? ` at \`${formatIssuePath(path)}\`` : ''; const value = valueAtPath(root, path); const rendered = describeReceived(value); @@ -79,7 +150,7 @@ function formatIssue(issue: z.ZodError['issues'][number], root: unknown, basePat // folded into that clause (``received the string `3` ``) rather than dangling after the location: our // custom schemas stop at the expected type, so the clause is appended; zod's built-in messages already // end with `, received `, so that tail is replaced with the enriched one. - let { message } = issue; + let message = describeIssue(issue, value); let got = ''; const bareExpected = BARE_EXPECTED_TYPE_MESSAGE.exec(message); const zodReceived = /, received (\S+)$/.exec(message); @@ -94,7 +165,7 @@ function formatIssue(issue: z.ZodError['issues'][number], root: unknown, basePat got = `, got \`${rendered}\``; } - return [`${message}${location}${got}`]; + lines.push(`${message}${location}${got}`); } /** @@ -104,9 +175,15 @@ function formatIssue(issue: z.ZodError['issues'][number], root: unknown, basePat * than zod's default, which omits the received value. */ function formatZodError(error: z.ZodError, root: unknown, label?: string): string { - const lines = error.issues.flatMap((issue) => formatIssue(issue, root, [])); + const lines: string[] = []; + const counter = { total: 0 }; + for (const issue of error.issues) collectIssueLines(issue, root, [], lines, counter); + // The label names the validated interface, the way ow's errors ended with "in object `X`". - return (label ? lines.map((line) => `${line} in \`${label}\``) : lines).join('\n'); + const rendered = label ? lines.map((line) => `${line} in \`${label}\``) : [...lines]; + const hidden = counter.total - lines.length; + if (hidden > 0) rendered.push(`... and ${hidden} more problem${hidden === 1 ? '' : 's'}`); + return rendered.join('\n'); } /** @@ -145,7 +222,7 @@ export function parseArgument( schema: TSchema, label?: string, ): TValue & z.output { - const result = schema.safeParse(value); + const result = schema.safeParse(value, { error: localeError }); if (!result.success) throw new ArgumentValidationError(result.error, value, label); return result.data as TValue & z.output; } diff --git a/test/validations.test.ts b/test/validations.test.ts new file mode 100644 index 000000000..bbab2cf9f --- /dev/null +++ b/test/validations.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, test } from 'vitest'; +import { z } from 'zod'; + +import { ArgumentValidationError, objectSchema, parseArgument } from '@apify/validations'; + +const anyNumber = z.custom((value) => typeof value === 'number' && !Number.isNaN(value), { + error: 'Invalid input: expected number', +}); + +describe('objectSchema', () => { + test.each([ + ['plain object', {}], + ['array', [1, 2, 3]], + ['class instance', new Date()], + ])('accepts %s', (_, value) => { + expect(objectSchema.safeParse(value).success).toBe(true); + }); + + test.each([ + ['null', null], + ['undefined', undefined], + ['string', 'foo'], + ['number', 123], + ['function', () => {}], + ])('rejects %s', (_, value) => { + expect(objectSchema.safeParse(value).success).toBe(false); + }); + + test('parsing returns the value itself instead of a copy', () => { + const obj = { foo: 'bar' }; + expect(parseArgument(obj, objectSchema)).toBe(obj); + }); +}); + +describe('parseArgument', () => { + test('throws ArgumentValidationError naming the received value', () => { + let error!: ArgumentValidationError; + try { + parseArgument('not an object', objectSchema); + } catch (err) { + error = err as ArgumentValidationError; + } + + expect(error).toBeInstanceOf(ArgumentValidationError); + expect(error).toBeInstanceOf(Error); + expect(error.name).toBe('ArgumentValidationError'); + expect(error.message).toBe('Invalid input: expected object, received the string `not an object`'); + expect(error.cause).toBeInstanceOf(z.ZodError); + expect(error.issues).toBe(error.cause.issues); + expect(error.issues.length).toBeGreaterThan(0); + }); + + test('message names the offending field and the value it received', () => { + const schema = z + .object({ + countryCode: z.string().regex(/^[A-Z]{2}$/), + retries: z.number().optional(), + }) + .strict(); + + expect(() => parseArgument({ countryCode: 'CZE' }, schema)).toThrow( + 'Invalid string: must match pattern /^[A-Z]{2}$/ at `countryCode`, got `CZE`', + ); + }); + + test('message folds the received type and value into one clause', () => { + expect(() => parseArgument({ retries: 'three' }, z.strictObject({ retries: anyNumber }))).toThrow( + 'Invalid input: expected number, received the string `three` at `retries`', + ); + + // `NaN` is named as itself, not as the self-contradictory "received number". + expect(() => parseArgument(Number.NaN, anyNumber)).toThrow('Invalid input: expected number, received NaN'); + + // An empty string would render as bare backticks — made visible instead. + expect(() => parseArgument('', anyNumber)).toThrow('Invalid input: expected number, received an empty string'); + + // Long strings are elided rather than dumped whole into the message. + expect(() => parseArgument('a'.repeat(250), anyNumber)).toThrow( + `Invalid input: expected number, received the string \`${'a'.repeat(200)}… (50 more characters)\``, + ); + }); + + test('message names the finiteness constraint where zod contradicts itself', () => { + const schema = z.strictObject({ timeoutSecs: z.number() }); + + // Zod's own sentence for these is the self-contradictory "expected number, received number". + expect(() => parseArgument({ timeoutSecs: Infinity }, schema)).toThrow( + 'Invalid input: expected a finite number at `timeoutSecs`, got `Infinity`', + ); + expect(() => parseArgument({ timeoutSecs: Number.NaN }, schema)).toThrow( + 'Invalid input: expected a finite number at `timeoutSecs`, got `NaN`', + ); + }); + + test('message names the validity constraint for an invalid Date', () => { + const schema = z.strictObject({ startedBefore: z.date() }); + + // Zod's own sentence for this one is the self-contradictory "expected date, received Date". + expect(() => parseArgument({ startedBefore: new Date('nonsense') }, schema)).toThrow( + 'Invalid input: expected a valid date at `startedBefore`', + ); + }); + + test('message renders a bigint with its suffix', () => { + const schema = z.strictObject({ retries: z.number() }); + + expect(() => parseArgument({ retries: 1n }, schema)).toThrow( + 'Invalid input: expected number, received the bigint `1n` at `retries`', + ); + }); + + test('message points at the offending array element', () => { + const schema = z.object({ groups: z.array(z.object({ name: z.string() })) }); + + expect(() => parseArgument({ groups: [{ name: 'ok' }, { name: 7 }] }, schema)).toThrow( + 'Invalid input: expected string, received the number `7` at `groups[1].name`', + ); + }); + + test('label names the validated interface on every line', () => { + const schema = z.strictObject({ retries: anyNumber, name: z.string() }); + + expect(() => parseArgument({ retries: 'three', name: 7 }, schema, 'ExampleOptions')).toThrow( + 'Invalid input: expected number, received the string `three` at `retries` in `ExampleOptions`\n' + + 'Invalid input: expected string, received the number `7` at `name` in `ExampleOptions`', + ); + }); + + test('applies schema defaults in the returned value', () => { + const schema = z.strictObject({ limit: z.number().default(42) }); + expect(parseArgument({}, schema)).toEqual({ limit: 42 }); + }); +}); + +// Shaped like a dataset push argument: a value, or an array of those values. +const nestedUnionSchema = z.union([z.looseObject({}), z.string(), z.array(z.union([z.looseObject({}), z.string()]))]); + +describe('union formatting', () => { + test('message lists every failed arm of a union', () => { + const schema = z.array(z.union([z.looseObject({}), z.string()])); + + expect(() => parseArgument([1], schema)).toThrow( + 'Invalid input: expected object, received the number `1` at `[0]`\n' + + 'Invalid input: expected string, received the number `1` at `[0]`', + ); + }); + + test('message drops the union arms that failed above the located problem', () => { + let error!: ArgumentValidationError; + try { + parseArgument([{ foo: 'bar' }, [1, 2, 3]], nestedUnionSchema); + } catch (err) { + error = err as ArgumentValidationError; + } + const lines = error.message.split('\n'); + + // The object and string arms fail on the whole array, so their lines carry no location. + expect(lines).toHaveLength(2); + expect(lines.every((line) => line.includes('at `[1]`'))).toBe(true); + }); + + test('message keeps every union arm when they all fail at the same depth', () => { + let error!: ArgumentValidationError; + try { + parseArgument(42, nestedUnionSchema); + } catch (err) { + error = err as ArgumentValidationError; + } + const lines = error.message.split('\n'); + + // Nothing located the problem more precisely than the argument itself, so no arm is redundant. + expect(lines).toHaveLength(3); + expect(lines.every((line) => line.includes('`42`'))).toBe(true); + }); +}); + +describe('message size limits', () => { + test('message caps the rendered lines when a whole array is invalid', () => { + const schema = z.array(z.union([z.looseObject({}), z.string()])); + const value = Array.from({ length: 5000 }, () => 1); + + let error!: ArgumentValidationError; + try { + parseArgument(value, schema); + } catch (err) { + error = err as ArgumentValidationError; + } + const lines = error.message.split('\n'); + + expect(lines).toHaveLength(11); + expect(lines.at(-1)).toBe('... and 9990 more problems'); + // Nothing is lost - the full set stays on `issues`. + expect(error.issues).toHaveLength(5000); + }); + + test('message does not count the dropped union arms in the hidden tally', () => { + const value = Array.from({ length: 5000 }, () => [1]); + + let error!: ArgumentValidationError; + try { + parseArgument(value, nestedUnionSchema); + } catch (err) { + error = err as ArgumentValidationError; + } + const lines = error.message.split('\n'); + + // Two arms per element, and the two top-level arms are dropped rather than merely hidden. + expect(lines).toHaveLength(11); + expect(lines.at(-1)).toBe('... and 9990 more problems'); + }); +});