Skip to content
Closed
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
2 changes: 1 addition & 1 deletion apps/zbugs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@headlessui/react": "^2.1.8",
"@octokit/core": "^6.1.2",
"@radix-ui/react-tooltip": "^1.1.4",
"@rocicorp/zero": "0.6.2024111800+725d79",
"@rocicorp/zero": "0.6.2024111900+a6527c",
"@schickling/fps-meter": "^0.1.2",
"classnames": "^2.5.1",
"dotenv": "^16.4.5",
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 55 additions & 33 deletions packages/shared/src/valita.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,38 @@ import {assert} from './asserts.js';
import * as v from './valita.js';
import {parse} from './valita.js';

test('basic', () => {
const t = <T>(s: v.Type<T>, val: unknown, message?: string) => {
const r1 = v.test(val, s);
const r2 = v.testOptional(val, s);
let ex;
try {
const parsed = parse(val, s);
expect(parsed).toBe(val);

expect(r1.ok).toBe(true);
expect(r1.ok && r1.value).toBe(val);

expect(r2.ok).toBe(true);
expect(r2.ok && r2.value).toBe(val);
} catch (err) {
ex = err;
}

if (message !== undefined) {
assert(ex instanceof TypeError);
expect(ex.message).toBe(message);

expect(r1.ok).toBe(false);
expect(!r1.ok && r1.error).toBe(message);

expect(r2.ok).toBe(false);
expect(!r2.ok && r2.error).toBe(message);
} else {
expect(ex).toBe(undefined);
}
};
const t = <T>(s: v.Type<T>, val: unknown, message?: string) => {
const r1 = v.test(val, s);
const r2 = v.testOptional(val, s);
let ex;
try {
const parsed = parse(val, s);
expect(parsed).toBe(val);

expect(r1.ok).toBe(true);
expect(r1.ok && r1.value).toBe(val);

expect(r2.ok).toBe(true);
expect(r2.ok && r2.value).toBe(val);
} catch (err) {
ex = err;
}

if (message !== undefined) {
assert(ex instanceof TypeError);
expect(ex.message).toBe(message);

expect(r1.ok).toBe(false);
expect(!r1.ok && r1.error).toBe(message);

expect(r2.ok).toBe(false);
expect(!r2.ok && r2.error).toBe(message);
} else {
expect(ex).toBe(undefined);
}
};

test('basic', () => {
{
const s = v.string();
t(s, 'ok');
Expand Down Expand Up @@ -225,8 +225,8 @@ test('basic', () => {
t(s, [1, 1]);
t(s, ['b', 'b']);
t(s, [true, true]);
t(s, ['d', true], 'Invalid union value');
t(s, [1, '1'], 'Invalid union value');
t(s, ['d', true], 'Invalid union value: ["d",true]');
t(s, [1, '1'], 'Invalid union value: [1,"1"]');
t(s, {}, 'Expected array. Got object');
t(s, 'a', 'Expected array. Got "a"');

Expand All @@ -237,6 +237,28 @@ test('basic', () => {
}
});

test('union error message', () => {
const type = v.union(
v.tuple([v.literal('a'), v.object({a: v.string()})]),
v.tuple([v.literal('b'), v.object({b: v.number()})]),
v.tuple([v.literal('c'), v.object({c: v.boolean()})]),
);
// Test once with the union itself, then with union(union) and union(union(union))
// to verify recursion.
for (const s of [type, v.union(type), v.union(v.union(type))]) {
t(s, ['a', {a: 'payload'}]);
t(s, ['b', {b: 123}]);
t(s, ['c', {c: true}]);
t(s, ['a', {b: 'not the right field'}], 'Missing property a at 1');
t(
s,
['b', {b: 'not a number'}],
'Expected number at 1.b. Got "not a number"',
);
t(s, ['c', {c: 1}], 'Expected boolean at 1.c. Got 1');
}
});

test('testOptional', () => {
const s = v.number().optional();

Expand Down
69 changes: 65 additions & 4 deletions packages/shared/src/valita.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ function displayList<T>(
return `${expected.slice(0, -2).map(toDisplay).join(', ')}, ${suffix}`;
}

function getMessage(err: v.Err | v.ValitaError, v: unknown): string {
function getMessage(
err: v.Err | v.ValitaError,
v: unknown,
schema: v.Type | v.Optional,
mode: ParseOptionsMode | undefined,
): string {
const firstIssue = err.issues[0];
const {path} = firstIssue;
const atPath = path?.length ? ` at ${path.join('.')}` : '';
Expand Down Expand Up @@ -102,7 +107,9 @@ function getMessage(err: v.Err | v.ValitaError, v: unknown): string {
)}${atPath}`;

case 'invalid_union':
return `Invalid union value${atPath}`;
return schema.name === 'union'
? getDeepestUnionParseError(v, schema as v.UnionType, mode ?? 'strict')
: `Invalid union value${atPath}`;

case 'custom_error': {
const {error} = firstIssue;
Expand All @@ -116,6 +123,57 @@ function getMessage(err: v.Err | v.ValitaError, v: unknown): string {
}
}

type FailedType = {type: v.Type; err: v.Err};

function getDeepestUnionParseError(
value: unknown,
schema: v.UnionType,
mode: ParseOptionsMode,
): string {
const failures: FailedType[] = [];
for (const type of schema.options) {
const r = type.try(value, {mode});
if (!r.ok) {
failures.push({type, err: r});
}
}
if (failures.length) {
// compare the first and second longest-path errors
failures.sort(pathCmp);
if (failures.length === 1 || pathCmp(failures[0], failures[1]) < 0) {
return getMessage(failures[0].err, value, failures[0].type, mode);
}
}
// paths are equivalent
try {
const str = JSON.stringify(value);
return `Invalid union value: ${str}`;
} catch (e) {
// fallback if the value could not be stringified
return `Invalid union value`;
}
}

// Descending-order comparison of Issue paths.
// * [1, 'a'] sorts before [1]
// * [1] sorts before [0] (i.e. errors later in the tuple sort before earlier errors)
function pathCmp(a: FailedType, b: FailedType) {
const aPath = a.err.issues[0].path;
const bPath = b.err.issues[0].path;
if (aPath.length !== bPath.length) {
return bPath.length - aPath.length;
}
for (let i = 0; i < aPath.length; i++) {
if (bPath[i] > aPath[i]) {
return -1;
}
if (bPath[i] < aPath[i]) {
return 1;
}
}
return 0;
}

/**
* 'strip' allows unknown properties and removes unknown properties.
* 'strict' errors if there are unknown properties.
Expand Down Expand Up @@ -160,7 +218,10 @@ export function test<T>(
): Result<T> {
const res = schema.try(value, mode ? {mode} : undefined);
if (!res.ok) {
return {ok: false, error: getMessage(res, value)};
return {
ok: false,
error: getMessage(res, value, schema, mode),
};
}
return res;
}
Expand Down Expand Up @@ -188,7 +249,7 @@ export function testOptional<T>(
return res;
}
const err = new v.ValitaError(res);
return {ok: false, error: getMessage(err, value)};
return {ok: false, error: getMessage(err, value, schema, mode)};
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/zero-cache/src/config/zero-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ test('zero-cache --help', () => {
Automatically wipe and resync the replica when replication is halted.
This situation can occur for configurations in which the upstream database
provider prohibits event trigger creation, preventing the zero-cache from
being able to correctly replicating schema changes. For such configurations,
being able to correctly replicate schema changes. For such configurations,
an upstream schema change will instead result in halting replication with an
error indicating that the replica needs to be reset.

Expand Down
15 changes: 11 additions & 4 deletions packages/zero-cache/src/config/zero-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import * as v from '../../../shared/src/valita.js';
import {parseOptions, type Config} from './config.js';
import {ExitAfterUsage, parseOptions, type Config} from './config.js';

/**
* Configures the view of the upstream database replicated to this zero-cache.
Expand Down Expand Up @@ -242,7 +242,7 @@ export const zeroOptions = {
`Automatically wipe and resync the replica when replication is halted.`,
`This situation can occur for configurations in which the upstream database`,
`provider prohibits event trigger creation, preventing the zero-cache from`,
`being able to correctly replicating schema changes. For such configurations,`,
`being able to correctly replicate schema changes. For such configurations,`,
`an upstream schema change will instead result in halting replication with an`,
`error indicating that the replica needs to be reset.`,
``,
Expand Down Expand Up @@ -291,8 +291,15 @@ let loadedConfig: ZeroConfig | undefined;

export function getZeroConfig(argv = process.argv.slice(2)): ZeroConfig {
if (!loadedConfig) {
const config = parseOptions(zeroOptions, argv, ENV_VAR_PREFIX);
loadedConfig = config;
try {
const config = parseOptions(zeroOptions, argv, ENV_VAR_PREFIX);
loadedConfig = config;
} catch (e) {
if (e instanceof ExitAfterUsage) {
process.exit(0);
}
throw e;
}
}

return loadedConfig;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ describe('change-source/pg', () => {
"Must be superuser to create an event trigger."

Proceeding in degraded mode: schema changes will halt replication,
after which the operator is responsible for resyncing the replica.",
requiring the replica to be reset (manually or with --auto-reset).",
],
]
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export async function setupTablesAndReplication(
`Unable to create event triggers for schema change detection:\n\n` +
`"${e.hint ?? e.message}"\n\n` +
`Proceeding in degraded mode: schema changes will halt replication,\n` +
`after which the operator is responsible for resyncing the replica.`,
`requiring the replica to be reset (manually or with --auto-reset).`,
);
}
}
Expand Down
Loading