From 71e45a3d8cfae7f1c4f7950c62c87d9d540947dd Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Wed, 9 Nov 2022 21:36:23 +0100 Subject: [PATCH] More user-friendly (and developer-friendly) zod error reporting (#12) Show each error message from zod and include an excerpt of the data around the failing path if possible --- src/adapters/discovery/hap_node_js_client.ts | 6 +-- src/boundaries/checker.ts | 52 ++++++++++++++++++++ src/boundaries/index.ts | 1 + src/platform.ts | 4 +- tests/boundaries/checker.test.ts | 45 +++++++++++++++++ 5 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 src/boundaries/checker.ts create mode 100644 tests/boundaries/checker.test.ts diff --git a/src/adapters/discovery/hap_node_js_client.ts b/src/adapters/discovery/hap_node_js_client.ts index e28df64..4cea6b7 100644 --- a/src/adapters/discovery/hap_node_js_client.ts +++ b/src/adapters/discovery/hap_node_js_client.ts @@ -1,6 +1,6 @@ import type { HapDiscover } from './api' import { HAPNodeJSClient } from 'hap-node-client' -import { Device, DeviceBoundary } from '../../boundaries/hap' +import { Device, DeviceBoundary, checkBoundary } from '../../boundaries' import { Logger } from 'homebridge' import z from 'zod' @@ -29,9 +29,9 @@ function startDiscovery(logger: Logger, config: HapConfig, resolve: ResolveFunc, try { const devices: Device[] = [] - for (const device of MaybeDevices.parse(deviceData)) { + for (const device of checkBoundary(MaybeDevices, deviceData)) { try { - devices.push(DeviceBoundary.parse(device)) + devices.push(checkBoundary(DeviceBoundary, device)) } catch (e) { logger.error('Boundary check for device data failed %o %s', e, JSON.stringify(device, null, 4)) } diff --git a/src/boundaries/checker.ts b/src/boundaries/checker.ts new file mode 100644 index 0000000..003baf7 --- /dev/null +++ b/src/boundaries/checker.ts @@ -0,0 +1,52 @@ +import z from 'zod' + +type Path = (string | number)[] + +function resolvePath(data: unknown, path: Path): { resolvedValue: string; resolvedPath: Path } { + const resolvedPath: Path = [] + for (const element of path) { + try { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + if (data[element] != null) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + data = data[element] + resolvedPath.push(element) + } else { + break + } + } catch (e) { + break + } + } + + return { resolvedValue: JSON.stringify(data), resolvedPath } +} + +function formatPath(path: Path): string { + return path.map((element) => (typeof element === 'number' ? `[${element}]` : element)).join('.') +} + +export function checkBoundary>(type: T, data: unknown): z.infer { + const result = type.safeParse(data) + if (result.success) { + return result.data + } + + const message = + 'Error checking type. Details: ' + + result.error.issues + .map((issue) => ({ ...issue, ...resolvePath(data, issue.path) })) + .map( + (issue) => + `[${issue.code}] ${issue.message}${ + issue.path.length > 0 ? ` at path "${formatPath(issue.path)}"` : '' + } (data${ + issue.resolvedPath.length > 0 ? ` at resolved path "${formatPath(issue.resolvedPath)}"` : '' + } is "${issue.resolvedValue}")`, + ) + .join(' | ') + + throw new Error(message) +} diff --git a/src/boundaries/index.ts b/src/boundaries/index.ts index 8d41092..298c893 100644 --- a/src/boundaries/index.ts +++ b/src/boundaries/index.ts @@ -1,6 +1,7 @@ import z from 'zod' import { ConfigBoundary as BaseConfigBoundary } from './config' +export * from './checker' export * from './hap' export const ConfigBoundary = z.intersection(BaseConfigBoundary, z.object({ platform: z.string() })) export type Config = z.infer diff --git a/src/platform.ts b/src/platform.ts index 7a4f8b3..dd737e9 100644 --- a/src/platform.ts +++ b/src/platform.ts @@ -5,7 +5,7 @@ import { discover } from './adapters/discovery/hap_node_js_client' import { serve } from './adapters/http/fastify' import { HttpServerController } from './adapters/http/api' import { PrometheusServer } from './prometheus' -import { Config, ConfigBoundary } from './boundaries' +import { Config, ConfigBoundary, checkBoundary } from './boundaries' export class PrometheusExporterPlatform implements IndependentPlatformPlugin { private readonly httpServer: PrometheusServer @@ -15,7 +15,7 @@ export class PrometheusExporterPlatform implements IndependentPlatformPlugin { constructor(public readonly log: Logger, config: PlatformConfig, public readonly api: API) { this.log.debug('Initializing platform %s', config.platform) - this.config = ConfigBoundary.parse(config) + this.config = checkBoundary(ConfigBoundary, config) this.log.debug('Configuration parsed', this.config) diff --git a/tests/boundaries/checker.test.ts b/tests/boundaries/checker.test.ts new file mode 100644 index 0000000..50a07d3 --- /dev/null +++ b/tests/boundaries/checker.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from '@jest/globals' +import z from 'zod' +import { checkBoundary } from '../../src/boundaries' + +const TestBoundary = z.object({ + member: z.literal('something'), + anotherMember: z.optional(z.literal('something else')), + yetAnotherMember: z.optional( + z.array( + z.object({ + member: z.literal('member'), + }), + ), + ), +}) + +describe('Test boundary checker', () => { + test('Returns checked data after successful check', () => { + const result = checkBoundary(TestBoundary, { member: 'something' }) + + expect(result).toEqual({ member: 'something' }) + }) + + test('Returns error and insightful error message on failing check for simple string', () => { + expect(() => checkBoundary(z.string(), 123)).toThrow( + '[invalid_type] Expected string, received number (data is "123")', + ) + }) + + test('Returns error and insightful error message on failing check for nested object', () => { + expect(() => + checkBoundary(TestBoundary, { + member: 'something else', + anotherMember: 'unexpected', + yetAnotherMember: [{ foo: 123 }], + }), + ).toThrow( + [ + '[invalid_literal] Invalid literal value, expected "something" at path "member" (data at resolved path "member" is ""something else"") | ', + '[invalid_literal] Invalid literal value, expected "something else" at path "anotherMember" (data at resolved path "anotherMember" is ""unexpected"") | ', + '[invalid_literal] Invalid literal value, expected "member" at path "yetAnotherMember.[0].member" (data at resolved path "yetAnotherMember.[0]" is "{"foo":123}")', + ].join(''), + ) + }) +})