Discord.js: Potential memory pointer/reference issues with addRole()

[tiritto]

Please describe the problem you are having in as much detail as possible:
When I execute this code on numerous members at once (I tested it both on my 2000+ community and on another 100+ server), some weird things happen and addRole() returns <GuildMember> of WRONG member.

It seems that for some reason addRole() returns <GuildMember> that was already processed by previous function calls, regardless of addRole() being now called from under another member. Some person on DiscordJS discord suggested it might be an issue with memory pointers/references.

In my original (non-sample) code I was executing addRole() alongside with removeRoles(), both being called from the same <GuildMember> passed by main function. What is interesting, removeRoles() worked just perfectly every single time, which suggest that this issue is exclusive to addRole().

Include a reproducible code sample here, if possible:

const Discord  = require('discord.js');
const client  = new Discord.Client();

// Function that causes all problems
function assignRole(GuildMember, Role) {
    if (!GuildMember.roles.has(Role.id)) {
        GuildMember.addRole(Role).then((member) => {
            // Print MISMATCH if returned 'member' is not the same as original 'GuildMember', and print "SUCCESS" otherwise. Also include details.
            if (GuildMember.id !== member.id) console.log("MISMATCH: Expected GuildMember: ["+ GuildMember.displayName +"], returned GuildMember: ["+ member.displayName +"] ");
            else console.log("SUCCESS:  Expected GuildMember: ["+ GuildMember.displayName +"], returned GuildMember: ["+ member.displayName +"] ");
        });
    }
}

client.on('message', message => {
    if (message.content == "debugNow") {
        message.guild.fetchMembers().then((guild) => {

            // We have 5 sample roles: A, B, C, D, E
            let roleA = guild.roles.find(role => role.name === 'A');
            let roleB = guild.roles.find(role => role.name === 'B');
            let roleC = guild.roles.find(role => role.name === 'C');
            let roleD = guild.roles.find(role => role.name === 'D');
            let roleE = guild.roles.find(role => role.name === 'E');
            let roleCollection = [roleA, roleB, roleC, roleD, roleE];

            for (let GuildMember of guild.members.values()) {
                let randomizedIndex = Math.floor(Math.random() * roleCollection.length);
                assignRole(GuildMember, roleCollection[randomizedIndex]);
            }
        })
    }
});

Further details:

• discord.js version: 11.3.0
• node.js version: 9.9.0
• Operating system: Debian 9 with Linux kernel 4.9.0-5-amd64
• Priority this issue should have: 3.5 / 5 Priority, above Medium but below Very High

[iCrawl]

Can you try installing the dev branch of v11 and check again?

npm i discordjs/discord.js#11.3-dev

[kyranet]

This issue is most likely fixed:

It could happen anywhere at any moment, in 11.x, we listen to gateway events when we edit a member in order to patch it, therefore, we are creating new guildMemberUpdate listeners every time you add/remove a role, and same behavior is applied to many other endpoints. However, when we emit that event, it fires all listeners, included the ones we use in our RESTManager (where we do our "magic" to patch the members with those events), and as they weren't guarded by member id, any member that were to fire that event that didn't have that role but does after the patch would get returned, the affected line is here: https://github.com/discordjs/discord.js/blob/11.3.0/src/client/rest/RESTMethods.js#L532

Normally, we await promises when we do multiple API requests, that way we add one role after another, without getting ratelimited (it's more API friendly). In your case, you weren't awaiting them, so if you were to edit two members at the same time with the same role, both GuildMember#addRoles would resolve with the first GuildMember that successfully resolves. Fortunately, this is (most likely) fixed in 11.3-dev.

I have tested your code with some minor tweaks and I cannot reproduce it anymore:

const { Client } = require('discord.js');
new Client()
    .on('message', async (message) => {
        if (!message.content.startsWith('!test')) return;

        // Get a role
        const role = message.guild.roles.get('379333849076858885');

        for (const member of message.guild.members.values()) {
            if (member.roles.has(role.id)) {
                // If the GuildMember has the role, ignore them
                continue;
            }

            // Add the role without awaiting, to cause the racecondition
            member.addRole(role).then((patched) => console.log(patched.id !== member.id
                // From #2480
                ? `MISMATCH: Field 'id' do not match | Expected: ${member.id}, Got ${patched.id}.`
                // Else, it should be fine
                : 'SUCCESS'
            ));
        }
    })
    .on('ready', () => console.log('Ready!'))
    .on('error', console.error)
    .on('warn', console.warn)
    .login(require('./config').token);

Which logged in for a guild with 6 members:

> node .\index.js
Ready!
SUCCESS
SUCCESS
SUCCESS
SUCCESS
SUCCESS
SUCCESS

[tiritto]

I use async functions and just simply await my addRole/removeRole operations now, but for the sake of clarification, I temporarily modified my code to work in the same way as I reported in this issue. I ran few tests on my 2500+ servers and all tests were successful, therefore I can confirm that issue is fixed now. Thank you for your hard work <3