Skip to content
This repository has been archived by the owner on Jul 11, 2024. It is now read-only.

Reaction ratelimit seems a bit slow #283

Open
Kelwing opened this issue Apr 16, 2020 · 3 comments
Open

Reaction ratelimit seems a bit slow #283

Kelwing opened this issue Apr 16, 2020 · 3 comments
Assignees

Comments

@Kelwing
Copy link
Contributor

Kelwing commented Apr 16, 2020

Describe the bug
If I try to add multiple unicode Emoji reactions to the same message, the REST calls seem to happen about once per second.

Expected behavior
The ratelimit for reactions is one every 0.25 seconds, so I would expect REST calls to happen that quickly.

Error messages
N/A

Desktop (please complete the following information):

  • Golang version: v1.13.5
  • Using Go modules? yes
  • Disgord version? v0.16.5
  • Connected to the gateway before using REST methods? yes

Additional context
I am unable to test non-unicode emoji's because of #282

@andersfylling
Copy link
Owner

So in Disgord I used to allow concurrent requests. If Discord said I had 10 calls left, along with the reset time I would just make the assumption that I could make 10 concurrent calls. Because why wouldn't I?

After talking about it in the Discord api server, I was strongly adviced to just send one call per bucket (rate limit key) as Discord is not completely consistent with how they handle rate limit info. Meaning even though you have 10 calls left, it can suddenly change and you have 4. Which causes rate limits.

I'm seriously baffled that this was a thing. And it would be awesome if I could go back to the concurrent version as it's way way faster in certain situations.

In this case, what happens is that a bucket is locked, a request is sent, on response the bucket is updated and then unlocked for a new request.

Discord response time is also somewhat slow. I remember I've seen around 200-400ms for just the request-response time without any special code.

So it being a little slower than hoped can be expected. However, once a second is way over that. That would require some investigating, as I believe I have unit tests that verify the rate limit info to be correctly handled.

@andersfylling andersfylling removed the bug label May 9, 2020
@andersfylling
Copy link
Owner

I hope that when the http.RoundTripper is used for rate limiting instead, there will be options or patches that fixes this.

@andersfylling
Copy link
Owner

andersfylling commented Nov 25, 2020

This PR will support bursts. Which will speed up reactions, quite a bit!
#353

It won't make it for v0.22, but I figure v0.23.

@andersfylling andersfylling pinned this issue Dec 30, 2021
@andersfylling andersfylling unpinned this issue Mar 21, 2024
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

2 participants