-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Notifier] [Discord] Fix value limits #60269
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
Conversation
Hmm, what about removing the client side validation completely and let the API return a proper error? Can you check what is happening if we would do so? |
I'm pretty sure that limits were not changed, just wrong calculation was used :) I'm fine either way. But at least for me it's easier when i can click on the method like |
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.
Ok then, lets do it
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.
I'm going to merge it as a bug fix, but Oskar is right, we avoid doing the validation on our side as it forces us to make sure we update the code whenever upstream changes as well.
Thank you @norkunas. |
It was not properly calculating values length with non latin characters for embed data. This makes it consistent with calculating chat message length in
DiscordTransport
.