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

Issue #4284 [Reaction add event firing twice] is still reproducible. #5063

Closed BannerBomb closed 3 years ago

BannerBomb commented 3 years ago

I originally commented on issue #4284 about this, but since the report was closed I decided to open another bug report and reference the issue cause I don't know whether the comment woulda ever been seen.

Please describe the problem you are having in as much detail as possible:

The issue #4284 seems to still be reproducible if the bot has just started up, then you have the bot send a message, clear the message cache then react to that message while using partials it'll fire twice.

I know the code is there to resolve the information into a reaction emoji but removing these lines https://github.com/discordjs/discord.js/blob/2583ad5da7db5ab92dfa357909f9c713db0a2981/src/structures/Message.js#L477-L485 did fix the issue while keeping the normal functionality of the event listener.

The first time the event is emitted, the partial value on the reaction is false, but the second time it's emitted it is true.

Include a reproducible code sample here, if possible:

Steps to reproduce:

  1. Start-up the bot.
  2. Execute the following code.
    // The `message` variable is from the `message` event.
    const msg = await message.channel.send('this is a message');
    msg.channel.messages.cache.clear();
    await msg.react(':smile:');

Further details:

Relevant client options:

BannerBomb commented 3 years ago

Just thought I should update on this and correct what I mentioned incorrectly in the issue. The 2 blocks of code quoted below are both fired when the client reacts to a message. https://github.com/discordjs/discord.js/blob/2583ad5da7db5ab92dfa357909f9c713db0a2981/src/structures/Message.js#L477-L485

However, this one fires twice when the message isn't cached when the client reacts to the message, one when the Message#react method is used, and right after the reaction of the client is added. https://github.com/discordjs/discord.js/blob/cee6cf70ce76e9b06dc7f25bfd77498e18d7c8d4/src/client/websocket/handlers/MESSAGE_REACTION_ADD.js#L3-L5

--

I know the code is there to resolve the information into a reaction emoji but removing these lines

https://github.com/discordjs/discord.js/blob/2583ad5da7db5ab92dfa357909f9c713db0a2981/src/structures/Message.js#L477-L485 did fix the issue while keeping the normal functionality of the event listener.

Replying back to what I said in the above quote. Removing the code within the Message#react method does break because the properties of the reaction stay null as shown below. Before when I said it doesn't break it I wasn't looking at the message property so I didn't see what properties of the message were missing.

MessageReaction {
  message: Message {
    channel: [TextChannel],
    deleted: false,
    id: '123456789012345678',
    system: null,
    type: null,
    content: null,
    author: null,
    pinned: null,
    tts: null,
    nonce: null,
    embeds: [],
    attachments: Collection(0) [Map] {},
    createdTimestamp: 1612823114607,
    editedTimestamp: null,
    reactions: [ReactionManager],
    mentions: [MessageMentions],
    webhookID: null,
    application: null,
    activity: null,
    flags: [MessageFlags],
    reference: null
  },
  me: true,
  users: ReactionUserManager {
    cacheType: [class Collection extends Collection],
    cache: Collection(0) [Map] {},
    reaction: [Circular *1]
  },
  _emoji: ReactionEmoji {
    animated: undefined,
    name: 'πŸ˜„',
    id: null,
    deleted: false,
    reaction: [Circular *1]
  },
  count: null
}

And removing the code in the MESSAGE_REACTION_ADD handler file will break all reaction add events except for when using the Message#react method.

jwday commented 3 years ago

I wanted to follow up on this since I just managed to reproduce #4284 entirely independently. My digging and research led me to this chain of issues. It looks like it was supposedly fixed but it would seem otherwise. Wondering if you've dug into it any more since your last post.

This block is executed based on a bot command:

const Discord = require("discord.js");

const newEmbed = new Discord.MessageEmbed().setTitle('An Embed');
const msg = await message.channel.send(newEmbed);

await msg.react('πŸ’©').then(() => msg.react('🚩')).then(() => msg.react('πŸ’₯'));

client.on('messageReactionAdd', (reaction, user) => {
    console.log('a reaction has been added');
});

When the bot first starts up and this command is run, it works as expected:

However, when the script is run again (e.g. when someone sends the bot the command again), the behavior changes:

I would expect messageReactionAdd to fire only once when a reaction is added to a message, regardless of who is adding the reaction.

advaith1 commented 3 years ago

that looks intended? if you add an event listener then it will stay there, it won't go away by itself, you shouldn't really be registering event listeners in commands

jwday commented 3 years ago

I'm confused then. I thought the original issue as described in #4284 was the same problem I'm having.

client.on('messageReactionAdd', (messageReaction, user) => {
  console.log('test');
});
// 'test' logs twice in the console

That seems similar to my issue, though i get an inconsistent number of returns. Sometimes 2, sometimes 3.

jwday commented 3 years ago

Oops. I see it now.

// that is the only listener in all of my code for 'messageReactionAdd'

Major minor detail. Okay, that explains one phenomenon (multiple fires), but not the inconsistent number of firings after multiple listeners have been created.

CaseyMartin23 commented 3 years ago

I'm having the same problem and it's adding to records of the same reaction in my database, please do fix or tell me there are any workarounds!

` client.on("messageReactionAdd", (reaction, user) => { if (reaction.partial) { try { await reaction.fetch(); } catch (error) { console.error("Something went wrong when fetching the message: ", error); return; } }

console.log("This fires twice!!") }) `

jwday commented 3 years ago

Casey, I discovered that my code was executing the client.on("messageReactionAdd" ... more than once. Each time that line executes, a new listener is added. It would therefore call the same callback function once for each listener that was added. To fix it, I wrapped my client.on("messageReactionAdd"... line with an if statement that checks if the messageReactionAdd event is already being listened to in client._events. If it is, then it won't add another listener.

if (!client._events.messageReactionAdd) {
    client.on('messageReactionAdd', msgReactListener);
}

In my case, msgReactListener is my callback function, defined elsewhere. Hope that helps.

monbrey commented 3 years ago

I wrapped my client.on("messageReactionAdd"... line with an if statement that checks if the messageReactionAdd event is already being listened to in client._events.

This should never be necessary - sounds like your event binding is nested inside another event, causing the duplication

jwday commented 3 years ago

Weird, that's the only thing that made it work. I don't know what else could have been setting up the listener. I wrote it all myself and that's the only line that calls client.on("messageReactionAdd"...). Tbf i'm still new to a lot of this so maybe there's something I'm missing, but this was the only way I could fix it.

CaseyMartin23 commented 3 years ago

Hi, @jwday Thank you for the help but it's still firing twice. I've checked if the event is called more than once, and it's the only one in the whole app. If you have any other advice or idea's, please do share them. And sorry for the late reply!

CaseyMartin23 commented 3 years ago

And it seems to only happen at the first reaction once the bot is up and running?? Thought of pushing everything it creates within the function into an array outside its scope, filter the array and then update the database but I've been struggling to figure out how the fire a function after the cleanup because if I did it at the end of the event it'll fire twice

polypixeldev commented 3 years ago

I took a look at this issue, and even in the latest master this still seems to be reproducible. I'm not sure how to fix this, though.

heychazza commented 3 years ago

Still currently having this issue..