Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul metadata parsing #68

Merged
merged 4 commits into from
Aug 12, 2023

Conversation

Protonull
Copy link
Contributor

No description provided.

This defers the error and json utilities to their own files. Zod is used to easily parse the config files.
@Protonull
Copy link
Contributor Author

Protonull commented Aug 12, 2023

This PR refactors metadata.ts to rely on zod for config validation. The config files themselves are still manually read and parsed into JSON, but then that JSON is fed into a premade zod schema, eg:

// This is an actual snipped of code from the PR
const CONFIG_SCHEMA = z.object({
	host: z.string().default('0.0.0.0'),
	port: z.coerce.number().positive().max(65535).default(12312),
	gameAddress: z.string(),
	whitelist: z.boolean().default(true),
})

The only drawback, to exaggerate a little, is that zod's errors aren't particularly helpful for laymen. Even if someone isn't a programmer, the error that's thrown for a missing file is still fairly legible to many inexperienced users... whereas zod errors aren't. Even with the addition of zod-validation-error, a library specifically about pretty-printing zod errors to users, can be unhelpfully phrased. For example, using the example above, not including the required 'gameAddress' value produces the following error: Validation error: Required at "gameAddress"

@Huskydog9988
Copy link
Contributor

Yea I'm mostly fine with that, I just want to know if there is a relatively easy way to add context that this error happened in the config file for example? I feel like we could reasonably diagnose the issue if someone approached us with this as is, but additional context would make it easier.

@Protonull
Copy link
Contributor Author

I just want to know if there is a relatively easy way to add context that this error happened in the config file for example?

Yeah, the error gets prefixed with the file path here.

@Huskydog9988 Huskydog9988 merged commit cd9cb26 into CivPlatform:stable Aug 12, 2023
2 checks passed
@Protonull Protonull deleted the update-metadata branch August 13, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants