jowsey / discord.js-menu

💬 Create Discord.js embed-menus, with customizable reactions and unlimited pages.
https://npm.im/discord.js-menu
MIT License
120 stars 19 forks source link

fix: being rate-limited when adding reactions #10

Open OmgImAlexis opened 3 years ago

OmgImAlexis commented 3 years ago

This adds a 1s delay to each reaction. This causes them to go one by one instead of all at once.

Before this I would get a rate limit message once per menu.

OmgImAlexis commented 3 years ago

This needs more work, even after adding this I'm still seeing rate limiting messages every second or third time I change pages.

jowsey commented 3 years ago

I don't think that's an issue with this library - as far as I know, Discord.js waits and times stuff to respect rate limits automatically. Plus, from what I can see, nobody else is hitting rate limits at full speed, so I think you have another underlying issue somewhere.

OmgImAlexis commented 3 years ago

Any ideas where to look then because changing pages throws this every single time.

OmgImAlexis commented 3 years ago

Actually this part of setPage might be the issue. I'm passing in an array of pages with < and > buttons. Every time the page changes they're being re-added. Not sure if I missed something in the readme but is there a way to prevent this currently?

this.reactionCollector.stop()
this.addReactions()
this.awaitReactions()
jowsey commented 3 years ago

Are you using the latest version of the library? There's a fix implemented just recently to prevent the re-adding every single time

OmgImAlexis commented 3 years ago

Yes and found the issue, it's with it editing the message.

By the way if I remove the code in this PR I get this. So maybe it's worth adding? MEE6, etc. all have a 1s delay between adding them for rate limiting reasons.

[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%9E%A1%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%AC%85%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%9E%A1%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'delete',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%AC%85%EF%B8%8F/107834314439294976',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%AC%85%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%9E%A1%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%AC%85%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] [./src/events/rate-limit.ts:4]
[debug] [AutoMod]: Currently rate limited [
  {
    timeout: 750,
    limit: 1,
    method: 'put',
    path: '/channels/792312384277577728/messages/795043394207744051/reactions/%E2%9E%A1%EF%B8%8F/@me',
    route: '/channels/792312384277577728/messages/:id/reactions'
  }
] 
jowsey commented 3 years ago

Hey, haven't just ignored this, turns out properly fixing this in a clean way is a little more complex than I thought - quite a few edge cases due to the way I handle some stuff. Will work on this when I get a moment.

FWIW, while the rateLimit event spam is annoying, Discord.js does automatically wait for it and handle it automatically so technically everything should still work perfectly fine if you want to just temporarily filter out rateLimit event logs from that route. Thanks for reporting :)

OmgImAlexis commented 3 years ago

The only issue with that is the possibility of being limited even further. Since this is causing a 429 it’ll eventually trip the Invalid Request Limit.

In saying that I’d have to hit the rate limit 16 times a second for it to take into effect so it’s not a massive concern at the moment.

IP addresses that make too many invalid HTTP requests are automatically and temporarily restricted from accessing the Discord API. Currently, this limit is 10,000 per 10 minutes. An invalid request is one that results in 401, 403, or 429 statuses.