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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃挕 Better handling of thread rename rate limit #72

Closed
MarcusOtter opened this issue Feb 5, 2022 · 3 comments 路 Fixed by #250
Closed

馃挕 Better handling of thread rename rate limit #72

MarcusOtter opened this issue Feb 5, 2022 · 3 comments 路 Fixed by #250
Labels
improvement 馃挕 Improvement suggestion

Comments

@MarcusOtter
Copy link
Owner

Is your request for improvement related to a problem? Please describe.
Discord has an exceptionally low rate limit for renaming threads. You may only rename the same thread 2-3 times within 10 minutes. The bot doesn't handle this very well at the moment if that rate limit is hit. From ! - Ki馃寑Zashikix#0001 on Discord:

image

Describe the solution you'd like
We need to determine if we're about to get rate limited and show a message to the user somehow. While we could hard-code the current rate-limit, the rate limit may change in the future. Discord.js doesn't make it very easy to detect this ahead of time so I'm not sure what the implementation would look like. We should reply with a message saying something like "Too many commands have been invoked in this thread recently, try again in 10 minutes".

Describe alternatives you've considered
Manually adding the rate-limit may be the way to go right now anyways, to keep the complexity down.

@MarcusOtter MarcusOtter added the improvement 馃挕 Improvement suggestion label Feb 5, 2022
@c43721
Copy link
Contributor

c43721 commented Feb 5, 2022

@MarcusOtter

Tip: never hardcode ratelimits. They're fluid and can change at anytime, per both Discord and Discord docs.

We should catch this error and pass that down to the user, but since this is a ratelimit and not an actual error, the request will still be caught by DJS and thrown into a bucket to be retried after the bucket's timer is exhausted.

Solution: Use discord/rest OR client API directly. This way, we get direct raw access to the API. That would help but makes this a bit messy, sorry.

Also, this can become really spammy. If someone decides to bot this for 10 minutes sending in excess of 10 req/sec, you risk being cloudflare banned. This should have some sort of rate limit. That can be addressed with @sapphire/ratelimits and keeping a global bucket somewhere. That would be my solution, introducing a small package to prevent rate limits (and it's done in memory)

@MarcusOtter
Copy link
Owner Author

I really don't want to do that, lol.

@n1ckoates
Copy link
Contributor

This command should be on a cooldown anyway to prevent abuse. Once #29 is fixed, this will be less of an issue, as people won't create a thread then immediately change the title as much.

IMO, this command should just have a 5 minute cooldown stored in memory.

@MarcusOtter MarcusOtter changed the title 馃挕 Improvement: Better handling of thread rename rate limit 馃挕 Better handling of thread rename rate limit Feb 6, 2022
This was referenced Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement 馃挕 Improvement suggestion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants