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

Pre-emptively halt requests to not encounter global ratelimit #650

Open
rxdn opened this issue Dec 26, 2020 · 3 comments · May be fixed by #2159
Open

Pre-emptively halt requests to not encounter global ratelimit #650

rxdn opened this issue Dec 26, 2020 · 3 comments · May be fixed by #2159
Labels
c-http Affects the http crate t-feature Addition of a new feature

Comments

@rxdn
Copy link
Contributor

rxdn commented Dec 26, 2020

From my understanding from our discussion in Discord, global ratelimits are not handled by the http module's ratelimiting function. more than the global ratelimit bucket's worth of requests may be sent at once, due to not being able to know the size and reset rate of the ratelimit bucket.

Discord assigns everyone a global cap of 50 requests per second, which can be increased after the requirements for large sharding are met or under exceptional circumstances, so it would be safe to impose a default limit of 50, unless the user provides their own limit to override with.

*EDIT: corrected first statement. Additionally, this means we will be hard-coding information that Discord does not provide, so we need to consider if this is something we want to do or, at the least, provide some sort of "cap" on in-flight requests.

@zeylahellyer zeylahellyer added c-http Affects the http crate t-feature Addition of a new feature labels Dec 26, 2020
@zeylahellyer zeylahellyer changed the title Handle global ratelimiting Pre-emptively halt requests to not encounter global ratelimit Dec 26, 2020
@7596ff 7596ff added this to the post-0.4 milestone Feb 18, 2021
@zeylahellyer zeylahellyer modified the milestones: 1.0, 0.5 Apr 20, 2021
@zeylahellyer zeylahellyer removed this from the 0.5 milestone Jul 24, 2021
@itohatweb
Copy link
Member

itohatweb commented Nov 21, 2021

There is an invalid request limit of 10_000/10 minutes that will ban you for an hour when you reach it. Maybe twilight should stop sending requests after e.g. 9_999 invalid requests to prevent getting globally rate limited for one hour too.

@zeylahellyer zeylahellyer self-assigned this Dec 24, 2021
@vilgotf
Copy link
Member

vilgotf commented May 20, 2022

Wouldn't hard-coding, even an over-writable, value would go against Discord's recommendations of using dynamic ratelimits? The cloudflare limit is however a sound exception

@AEnterprise
Copy link
Member

I think hardcoding but allowing to override the global rate limit is fine since it's explicitly documented at https://discord.com/developers/docs/topics/rate-limits#global-rate-limit

Cloudflare ones could be nice if we ensure it excludes the shared rate limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-http Affects the http crate t-feature Addition of a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants