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

Problem with updating TextChannel Topic #4327

Closed
1 task
PanSzelescik opened this issue May 31, 2020 · 11 comments
Closed
1 task

Problem with updating TextChannel Topic #4327

PanSzelescik opened this issue May 31, 2020 · 11 comments

Comments

@PanSzelescik
Copy link
Contributor

PanSzelescik commented May 31, 2020

Please describe the problem you are having in as much detail as possible:
My bot updating 6 channels' topics every minute. I've never had problems with it. Today I noticed that the topics are not refreshing every minute, but randomly (after 5 or 10 minutes). I don't have any errors on the console. I thought it might be because the channels are not in the cache.

client.channels.cache

I quickly prepared a version using:

client.channels.fetch()

but the effect was the same. Because setTopic returns Promise I added .then():

channel.setTopic(text).then(console.log).catch(console.error);

and even he doesn't call.

Further details:

  • discord.js version: 12.2.0
  • Node.js version: 12.16.1 (Windows), 12.17.0 (Debian)
  • Operating system: Windows 10 2004, Debian 10.4
  • Priority this issue should have – please be realistic and elaborate if possible: low
  • I have also tested the issue on latest master, commit hash:
@SpaceEEC
Copy link
Member

This is not because of caching or the like, it's "just" you being rate limited.
You can confirm this by adding a listener to Client#rateLimit.

There was an announcement about this on the official Discord Developers guild:

Hi everyone--wanted to make official note of the rate limit change on channel updates that was deployed yesterday. While most of you won't notice anything at all, there are known and unknown use cases that this effects.

The new rate limit for channel NAME AND TOPIC updates is 2 updates per 10 minutes, per channel. (Sorry that was unclear at first)

This is reflected in the normal rate limits headers so no library changes should need to happen.

For a bit of context, frequent channel updates have significant impact on our infrastructure and performance, especially in the case of large guilds and mobile clients, hence the limit changes.

I apologize that this went out without a prior announcement. While in general we do not announce rate limit changes as they are dynamically handled by libraries, this change was significant enough to warrant one. If crucial functionality has broken for your implementation because of these changes, please feel free to reach out to me directly (@mason in the Discord Developers server) so that I can understand!

@PanSzelescik
Copy link
Contributor Author

I didn't know, so I close

@PanSzelescik
Copy link
Contributor Author

However, in this case it could throw an error in catch

@keybraker
Copy link

So this cannot be bypassed. We will have to work around it, correct ?

@skeddles
Copy link

skeddles commented Jul 22, 2020

This should absolutely be mentioned in the documentation, especially since it didn't used to be the case. I just wasted half an hour debugging my bot that used to work fine before finding this thread. It could at least throw an error though rather than just not doing anything at all. Super confusing.

@vaporoxx
Copy link
Contributor

No, ratelimits aren't mentioned in any other part of the documentation and also shouldn't be in this case. Discord.js isn't supposed to warn you about changes on Discord's end that don't affect the usage of the library.

@PanSzelescik
Copy link
Contributor Author

But it should throw an error, and not I am waiting and promise is always pending

@skeddles
Copy link

skeddles commented Jul 24, 2020

The problem is that "changes on Discord's end" DO "affect the usage of the library". And I don't see why you wouldn't want to make the documentation as helpful as possible to the people using the library (at the very least. it would still be far better if it just threw an error, rather than doing NOTHING which no code should ever do)

@didinele
Copy link
Member

didinele commented Jul 24, 2020

👎 There is no error thrown because the library has not yet given up on the HTTP request. Every request that replies with a code
of either 429 (rate limit) or > 500 & < 600 is re-tried for a couple of times (which can be configured) until it'll actually reject the promise.

In your cases, it fails once with a rate limit, which makes it wait for its duration & re-tries afterwards, most likely succeeding.

@almostSouji
Copy link
Member

Throwing an error

But it should throw an error, and not I am waiting and promise is always pending

[...] at the very least, it would still be far better if it just threw an error, rather than doing NOTHING which no code should ever do

You are suggesting that instead of delaying rate limited actions and sending them when the rate limit backoff is over we should just throw an error when ever you are experiencing a rate limit? One of the major points of using a library is to actually not have to deal with rate limits (along with getting the cached data and valid structures when websocket events emit).

Throwing an error every time a action can not be immediately executed would be an absolutely terrible user experience and make the point of handling the rate limit backoff obsolete. If you are of a different opinion you seem to be lacking understanding of how rate limits work and why they are dynamically computed by the API. Feel free to review discords API documentation on the topic.

Which leads us to the next point:

Documenting rate limits

Rate limits are dynamically set by the API based on current load. Approximations in rate limits can be made over time however it might change at any point during operation without further notice. Discord themselves do not document rate limits. The stance is the following:
Libraries (or developers, if you want to connect without a library) have to respect and handle the rate limits based on the response headers the library returns. Rate limits can change at any point without further notice.

Or the actual quote from discords API documentation (emphasis by me) :

Because we MAY CHANGE RATE LIMITS AT ANY TIME and rate limits CAN BE DIFFERENT PER APPLICATION, rate limits should not be hard coded into your bot/application. In order to properly support our dynamic rate limits, your bot/application should parse for our rate limits in response headers and locally prevent exceeding the limits as they change.

There are approximations of certain endpoints limits floating around the internet and in the case of topic and name changes it turns out to be roughly 2 per 10 minutes, however nothing guarantees that these values will stay.

Documenting rate limits on our (library) end is just not feasible.

Doing Nothing

The library returns a promise, a promise represents "the eventual completion (or failure) of an asynchronous operation, and its resulting value.", which is precisely what's happening here. After the back off the request will be made and the promise will resolve. If an API error occurs the promise will reject, as per specification.

And lastly:

Bypassing/Working around rate limits

No. And if you don't want the same to happen to channel creation or other endpoints you best don't apply the same logic to other endpoints. This was a deliberate update to stop bots from editing channels every x seconds as clocks, timers, ping displays, dynamically changing member statistics and whatever other applications for it exist(ed).

This follows directly after people abusing role color changes for "rainbow roles". A subset of people abuses endpoints and now we have to deal with exorbitantly high channel name and topic change limits.

@iCrawl
Copy link
Member

iCrawl commented Jul 24, 2020

The problem is that "changes on Discord's end" DO "affect the usage of the library". And I don't see why you wouldn't want to make the documentation as helpful as possible to the people using the library (at the very least. it would still be far better if it just threw an error, rather than doing NOTHING which no code should ever do)

We don't wanna update the documentation all the time in case they change their ratelimits and do not announce it. It would simply be wrong and someone would open an issue like this again.

Lets just not build clocks or real-time updates into channel names/topics or any other real-time shenanigans, shall we?

Soujis explanation should cover the rest. I'll close this since there is really no need for a discussion around this anymore.

@discordjs discordjs locked as resolved and limited conversation to collaborators Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants