-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore(SlashCommandBuilder): improve message when missing required params #9468
chore(SlashCommandBuilder): improve message when missing required params #9468
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
7fbef78
to
f8fdd34
Compare
@@ -35,14 +40,22 @@ export function validateMaxOptionsLength(options: unknown): asserts options is T | |||
} | |||
|
|||
export function validateRequiredParameters( | |||
name: string, | |||
description: string, | |||
name: string | undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name
and description
are casted as non-undefined on SlashCommandBuilder, but I figure they should be treated as potentially undefined wherever they can be, since they can be undefined.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9468 +/- ##
==========================================
+ Coverage 59.29% 59.31% +0.02%
==========================================
Files 223 223
Lines 14633 14646 +13
Branches 1258 1263 +5
==========================================
+ Hits 8676 8688 +12
- Misses 5917 5918 +1
Partials 40 40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
export class MissingRequiredParameterError extends Error { | ||
public constructor(missingParameter: string) { | ||
super(`Required parameter "${missingParameter}" is missing`); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to create this (and make the function return non-shapeshift errors), you can use the MissingPropertyError
error class from @sapphire/shapeshift
, which is exported.
Using the aforementioned class:
import { MissingPropertyError } from '@sapphire/shapeshift';
throw new MissingPropertyError('name');
Closing in favour of #10448. |
When you try to call
toJSON
on aSlashCommandBuilder
that has not had a name or description set, or has any options that are missing a name or description, you get the following error:In my opinion, this error message is a bit cryptic and makes it difficult to figure out what the issue is. This PR adds explicit checks for missing command/options names/descriptions, and throws a more useful error message.
Status and versioning classification: