Confusing error messages being produced by lack of null checking in builders/.../Assertions.ts

[LeoDog896]

Which package is this bug report for?

builders

Issue description

In Assertions.ts for the builders package, there is a lack of null checking which causes confusing errors to occur. For example, omitting TextInputBuilder#setLabel gives you the following error message:

ValidationError > s.string
  Expected a string primitive

  Received:
  | undefined

This can be fixed by providing more context when the necessary parameters are passed as null:

export function validateRequiredParameters(customId?: string, style?: TextInputStyle, label?: string) {
        // I am unfamiliar with any code styling guides for discord.js, so this is just an example.
        if (customId === null || customId === undefined) throw Error("CustomID not provided in builder.")
	customIdValidator.parse(customId);
        // and so on ...
}

Package version

14

Node.js version

18

Operating system

Ubuntu Linux

Priority this issue should have

Low (slightly annoying)

Which partials do you have configured?

Not applicable (subpackage bug), No Partials

Which gateway intents are you subscribing to?

Not applicable (subpackage bug)

I have tested this issue on a development release

No response

[jaw0r3k]

but it needs label 🤔

[LeoDog896]

Yes, and so a descriptive error message should be given whenever label is missing, not the one I sent above: (even the stacktrace is confusing)

[kyranet]

A label system for shapeshift is tracked at https://github.com/sapphiredev/shapeshift/issues/201

[LeoDog896]

Interesting! I'll see if I can work on the feature there.

[willuhmjs]

It would probably be easier to move from @sapphire/shapeshift to zod to support validation error messages.

For instance, you could change:

export const buttonLabelValidator = s.string
	.lengthGreaterThanOrEqual(1)
	.lengthLessThanOrEqual(80)
	.setValidationEnabled(isValidationEnabled);

to

const buttonLabelValidator = z.string()
	.min(1, { message: "Must be 1 or more characters long" } )
	.max(80, { message: "Must be 80 or fewer characters long" } )

[monbrey]

We moved off zod some time ago due to it's terrible default error messages and comparatively poor performance. Very unlikely we'll go back.

[imranbarbhuiya]

check https://github.com/sapphiredev/shapeshift/pull/231 for error message support in shapeshift. Zod has custom error messages but it doesn't have proper error stack

[f3nai]

Got this error in my command handler. No idea how I did it, but I got it randomly.