discordjs / builders

A collection of builders that you can use when creating your bot.
Apache License 2.0
97 stars 37 forks source link

Validation errors reported by zod are not useful #64

Open Fleny113 opened 2 years ago

Fleny113 commented 2 years ago

Issue description

Calling toJSON() from an instance of a SlashCommandBuilder without setting required parameters (name and description) throw an error that seem from zod that isn't useful at all to understand the problem because of the size of the error and don't say what parameter is missing.

Original message: When creating a SlashCommandBuilder and doing toJSON() zod thrown about an recived undefined and expect a string.

Step to reproduce:

  1. Import SlashCommandBuilder from @discordjs/builders
  2. Create a new Builder and call toJSON()
  3. zod should throw an error that isn't useful at all

zodError.log

Code sample

import { SlashCommandBuilder } from "@discordjs/builders";
new SlashCommandBuilder().setName("test").toJSON();

@discordjs/builders version

0.9.0

Node.js version

node.js v17.2.0 | Typescript 4.5.4

Operating system

Windows 11

Priority this issue should have

High (immediate attention needed)

vladfrangu commented 2 years ago

Is your code..minified/bundled at all? Because it definitely throws an error, as seen at the end, but a really, REALLY not helpful one

Khasms commented 2 years ago

The issue is you didn't provide a description but the description is required.

Fleny113 commented 2 years ago

Is your code..minified/bundled at all? Because it definitely throws an error, as seen at the end, but a really, REALLY not helpful one

No, i having this issue from typescript (and tsc) but i tried directly from nodejs and still throw error

The issue is you didn't provide a description but the description is required.

Sorry, i added it.

vladfrangu commented 2 years ago

That's not what @Khasms meant haha. You need to call .setDescription on the builder you created! For further questions, feel free to ask in our Discord server

Fleny113 commented 2 years ago

That's not what @Khasms meant haha. You need to call .setDescription on the builder you created! For further questions, feel free to ask in our Discord server

I known that is not he mean but really, i don't known what to write, the errore is useless. Anyway, thanks for help.

vladfrangu commented 2 years ago

Hrmm, true true, the errors could be better.. I'll be reopening this but with a different title 👍

Fleny113 commented 2 years ago

Hrmm, true true, the errors could be better.. I'll be reopening this but with a different title 👍

I updated the description to made that more consistent with title

hampuskraft commented 2 years ago

Hi there, one possible alternative is to wrap parsing with a custom error that unsets the stacktrace, resulting in the below:

image

I'm willing to PR this if it seems like a good tradeoff. ValidationError can also be exported so it can be caught by consumers.

Khasms commented 2 years ago

I can't reproduce the code dump at the beginning of the error, it outputs as it should on my end. But, as far as the error not being helpful, I opened #61 a week ago to improve them, so give some feedback on that if you see any other messages that need to be improved.

kyranet commented 2 years ago

@hampuskraft Not a good tradeoff since that makes parsing +2x slower, which adds up to how slow Zod's validation already is, see https://github.com/discordjs/discord.js/pull/7067#issuecomment-989502034