discordjs / discord.js

A powerful JavaScript library for interacting with the Discord API
https://discord.js.org
Apache License 2.0
25.36k stars 3.97k forks source link

Suggestion: Rate Limits #1637

Closed RealBlazeIt closed 6 years ago

RealBlazeIt commented 7 years ago

Rate Limits

On the official Discord Documentation, there is a topic dedicated to rate limits: https://discordapp.com/developers/docs/topics/rate-limits

This page explains how Discord rate limits are constantly varying, and in order to prevent your bot from exceeding these rate limits, they highly suggest that your bot/application should parse for our rate limits in response headers and locally prevent exceeding of the limits as they change.

Problem

With no way of being able to handle any rate limits, loads of problems arise with public bots. Users in other servers can spam multiple commands which will infinitely queue up, the last ones executing long after they were originally triggered: https://prnt.sc/fpn7na. Issue #1176 went over this topic, and most replies simply said that Discord.js already handles rate limiting.

With abuse of this in multiple different text channels it will end up by globally rate limiting your bot preventing it from functioning properly. Now, there are multiple obvious solutions to this, such as:

However, none of these are genuine solutions to the problem at hand, not to mention that Discord themselves explicitly stated that rate limits should not be hard coded into your bot/application.

Solution

There are two solutions that I see:

  1. Allow users to toggle an option which discards all messages in the rate limit buckets.
  2. Allow users to access all rate limit information to implement in their bot.

The first solution would be more of a cover up than an actual fix. The commands would still be executed but the messages sent inside of them would never go through.

The second solution allows for a much better handling of rate limits, and could be done with either events or methods. An example of the method implementation could be as following:

The return value could be something along the lines of Array<RateLimit> containing the following information as presented in the Discord documentation:

[
  {
    "channelId": "222197033908436994",
    "limit": 5,
    "remaining": 0,
    "reset": 1470173023
  },
  {
    "channelId": "222078374472843266",
    "limit": 5,
    "remaining": 2,
    "reset": 1470174861
  }
]

An example of usage would be:

client.fetchMessageRateLimits()
  .then(rateLimits => {
    for (rateLimit of rateLimits) {
      if (rateLimit.remaining === 0) {
        const msUntilReset = rateLimit.reset - Date.now();
        commandHandler.ignoreChannel(rateLimit.channelId, msUntilReset);
      }
    }
  });
eslachance commented 7 years ago

The ratelimits are not hardcoded into discord.js. The docs are somewhat vague on the subject, but, essentially, you're not forced to use a library to access the API. You can do so straight-up and if you do, you have to handle the ratelimits yourself as prescribed by the docs.

And the docs say, as you quote them yourself, an application should parse for our rate limits in response headers and locally prevent exceeding of the limits as they change.. This is precisely what Discord.js is doing, by using the concept of "Buckets" (which is common throughout most popular libraries). In fact, for a library to be somewhat official, it is required to correctly handle rate limits as per the documentation. That doc is addressed to whatever connects to the API, basically.

Obviously, dropping messages is not an option for most bot owners, so your desire to drop those messages completely is by no means a common requirement.

I'm not saying the feature request for an option to drop extra requests if they exceed the ratelimit is bad. I'm just indicating that you're wrong in saying we're not following the docs. In fact, we are exactly following the specs and acting accordingly, so bot owners do not have to (and could not since they don't interact directly with the HTTP API).

amishshah commented 7 years ago

100% coverage of the Discord API means that we conform to the API, there is no requirement for us to expose such information to the user. It might be something we look into for the future, however.

iCrawl commented 7 years ago

Is this handling it?

Reading the source and the intern structure of it should show you that you will never hit a 429, which is the code for you getting ratelimited. If you would get multiple 429 in a row that would get you banned very easily.

However, none of these are genuine solutions to the problem at hand, not to mention that Discord themselves explicitly stated that rate limits should not be hard coded into your bot/application.

Discord also explicitly states that libraries interacting with the API need to handle ratelimits, which we do, with a bucket system.

Allow users to toggle an option which discards all messages in the rate limit buckets.

This seems like a horrible solution.

Allow users to directly handle rate limits.

And this seems even worse. Casually handling ratelimits yourself can get you in real trouble real fast. And we would have to advise against it very strongly, since 90% of the userbase would not know how to do this correctly.

HOWEVER. There is a rateLimit mode in discord.js which is called "burst" which is the exact opposite of what "sequential" does. It will send messages not in the sequence you received them and keeping the order, it will just try to squeeze in as many as it can. This will not prevent the "spamming" issue you are talking about though.

RealBlazeIt commented 7 years ago

@eslachance I never stated that discord.js hardcoded any rate limits. I was simply stating that the rate limit information should be directly available to the user, allowing them to handle it as they wish and not only having it internally handled.

RealBlazeIt commented 7 years ago

@iCrawl I do not want Discord.js rate limit handling to be replaced by direct user handling, I want both to be available. Discord.js would handle rate limits as they normally do, however, you would be able to gain access to the rate limit information allowing you to build your bot around it.

Receiving additional information about rate limits in no way modifies removes Discord.js's current system. It would simply add an additional layer to how it is already handling, allowing for more customization instead of it all being internally handled. You have completely misunderstood this entire suggestion.

amishshah commented 7 years ago

We are planning to implement a ratelimit event to provide rate-limiting information to the user.

iCrawl commented 7 years ago

Allow users to directly handle rate limits.

I do not want Discord.js rate limit handling to be replaced by direct user handling

You have completely misunderstood this entire suggestion.

I give up. But yes, exposing information is fine. I have never said anything against that, you have completely misunderstood my response to your "Allow users to directly handle rate limits."

RealBlazeIt commented 7 years ago

@iCrawl I suppose my formulation could have been different, but that does not change the fact that directly handling does not mean replacing.

atom0s commented 7 years ago

Hate to cause emails on an older topic, but was this something that was ever added? (Specifically, a way to obtain the rate-limiting information.)

I am working on a small bot for a side project and am hitting rate limits with certain things such as updating the bots game info (setPresence) too often. I'd like to be able to mold my update events around the rate limit so I am not hitting it or causing issues with the API. It would be very useful to have this information available in some manner.

I just started messing with Discord.js, so I am sorry if I missed this in the docs if it was already added/covered. I saw this ticket was still open and assumed it wasn't.

devsnek commented 7 years ago

in v12 there is a new ratelimiter which allows you to pass a custom ratelimit handler and a ratelimited method, and those ratelimits you have posted are wrong and should not be used

Lewdcario commented 6 years ago

This has been addressed by #2019.