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

feat: check payload data length and throw exception when it exceeds limit #46

Open
ubertao opened this issue Jan 7, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ubertao
Copy link

ubertao commented Jan 7, 2024

I got a 400 Error BUTTON_DATA_INVALID when using menu plugin. The error message is very confusing and it took me a while to debug.

Turns out it's because I used long menu id and and long payload data so the callback query data exceeded Telegram's limit. (I didn't know menu id was also passed to api as part of callback query data. And even if I knew, it would be tricky for developer to keep track of bytes used for callback query data because menu plugin adds its own stuff to it).

Would be helpful if menu plugin could check payload length before making api request, and output some meaningful error message, e.g. telling me how many bytes are available for payload data.

@KnorpelSenf KnorpelSenf added enhancement New feature or request good first issue Good for newcomers labels Jan 7, 2024
@KnorpelSenf
Copy link
Member

This should ideally just be a warning. I do not want to perform client-side validation because the limits may change and then the plugin would be more restrictive than necessary.

@DonVietnam
Copy link

This should ideally just be a warning. I do not want to perform client-side validation because the limits may change and then the plugin would be more restrictive than necessary.

Just yesterday, I spent more than 5 hours searching for the reason why I was getting this error, and I got closer to the answer only after reading this page, so it seems that this is a really significant problem, the error should be more informative, and at the moment the message leads away from the real problem.

@KnorpelSenf
Copy link
Member

That is because the error is created by Telegram not grammY. We also should not validate/error out in grammY because that might break forward compatibility. Hence, adding such a warning is what needs to be implemented. @DonVietnam do you feel like contributing this to the plugin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants