Discord.js: Reactions getting stuck

[Yimura]

Please describe the problem you are having in as much detail as possible:
Reacting on a message made by my bot hangs. This hang gets resolved by pushing more reacts through the rest queue. I was unable to reproduce this problem but if contacted would be willing to give my code.

Include a reproducible code sample here, if possible:
Tried reproducing was unable to however my bot is a living example of this problem.
https://cdn.discordapp.com/attachments/334350562659139584/662982934097166336/music_-_Discord_2020-01-04_12-35-54.mp4
^^ Video where you can see the exact problem

Further details:

• discord.js version: v12
• Node.js version: v10.16.3 & v12.14.0
• Operating system: Debian 10.2 & Debian 10.1.0
• Priority this issue should have – please be realistic and elaborate if possible:
  Not very high as it happens in really rare cases and as how I was unable to reproduce this

Prior to v12 when my bot was still using v11.x this problem was not present

• [x] I have also tested the issue on latest master, commit hash: d2ef02906c5618d883b1adfb348fd248a4db616f

[vladfrangu]

This is probably very easy to reproduce, looking at what your bot did before it "broke".

1. Queue up 5 reactions on a message
2. Create a new message and queue any amount of reactions
3. Delete the original message

My gut says that's a 100% reproducible issue. I also know why it's broken and I'll see about fixing soon!

[Yimura]

Alright I'm happy you know what is causing this.
I tried creating embeds and adding reactions to them but I didn't delete them in my try to reproduce this problem.

[Yimura]

Is there any progress on this issue?

[Yimura]

Managed to fix this by adding the following:

this.run();

above the following line in the file:
src/rest/RequestHandler.js#170

Before:

      try {
        const data = await parseResponse(res);
        if (res.status >= 400 && res.status < 500) {
          return reject(new DiscordAPIError(request.path, data, request.method, res.status)); // Before this line
        }
        return null;
      } catch (err) {
        return reject(
          new HTTPError(err.message, err.constructor.name, err.status, request.method, request.path)
        );
      }

After:

      try {
        const data = await parseResponse(res);
        if (res.status >= 400 && res.status < 500) {
          this.run(); // Added this
          return reject(new DiscordAPIError(request.path, data, request.method, res.status));
        }
        return null;
      } catch (err) {
        return reject(
          new HTTPError(err.message, err.constructor.name, err.status, request.method, request.path)
        );
      }

I'm using this as a hot fix as I'm assuming this is not the proper solution to this problem.

[Trezub]

Any update on this issue? I'm having this problem on 12.0.1

[Yimura]

As far as I know this hasn't been resolved, you can attempt to fix it yourself with the solution I gave above. The internals of the file don't seem to have changed since then.

Add below line 169 in file src/rest/RequestHandler.js

this.run();

[Trezub]

you can attempt to fix it yourself with the solution I gave above

This works! Thanks

Adding multiple reactions seems slow on this version, though

[Yimura]

you can attempt to fix it yourself with the solution I gave above

This works! Thanks

Adding multiple reactions seems slow on this version, though

Not sure, I think stricter timeouts have been added in discord.js or from Discord themselves.

[timotejroiko]

From what i understand, discord's API rate limits for reactions are 250ms, but discord.js has a rateLimitOffset which can be configured in the client options. The default value of the offset is 550ms, which effectively makes it wait at least 800ms before attempting to add the next reaction.

Reducing the offset in the client options will make the reactions faster, but you will need to add your own timings to them to avoid throwing rate limit errors.

Im sure this is something that could be improved in the future, but it would potentially require reworking a part of discord.js's internal rate limit prevention system

[Trezub]

@timotejroiko cool, i hadn't thought of tweaking the client options, gonna try that. Thanks

[Yimura]

Please keep discussion on topic.

[Yimura]

It has been exactly 6 months since I opened this issue, the problem is still present and I'm still using the solution I mentioned in earlier replies of this thread. I know it's a lot of work to maintain something of this size and that it's sometimes hard to feel like working on something... I just want to know if anyone has been looking of implementing a proper fix for this and/or if the problem goes deeper than inherently visible?

[skick1234]

This problem still occurs on v12.3.0

[Fizz55]

I have the same issue. Have not tried this workaround yet, but would love to see a solution to this.
It's bad right now, because once it fails to post reactions, it'll fail on subsequent messages as well. It seems after this problem occurs, the ID of the message object is not pointing to the correct message anymore, but to the one that came before it (I think).

[SammyWhamy]

Same issue, after failing to react the whole channel practically gets blocked to any new reactions until I restart the whole bot

[Yimura]

Use the workaround from above, there have been no approved fixes yet for this issue.

[advaith1]

[kyranet]

Hey, I wanted to make you aware a solution is already available in a PR: #4835. Can you please try and check if it works? By design, the tweaked request handler cannot dead-lock.

[Yimura]

Sure, will check it when I get the time!

[Yimura]

@kyranet I have confirmed your PR to be working for this issue, well done!

Also, a little idea, maybe you can extend your AsyncQueue class with Array then you can just overwrite the existing methods of the array class.

[kyranet]

Glad to hear the PR solves your issue! And enjoy the new stack traces too 😃

AsyncQueue's array is internal, it's not meant to be exported nor used, although I could use #privates as they got support starting with Node.js 12.4.0, but I think the library's minimum is on 12.0.0, so we're using a regular field.

Either way that's out of this issue's topic, please contact me in Discord (kyra#0001) for further discussion :)