VoiceChannel.setParent() doesn't update its own variable

Makosai commented 6 years ago

Please describe the problem you are having in as much detail as possible: VoiceChannel.setParent() is setting the category as intended but is not updated the VoiceChannel.parent variable. The channel goes under the category in Discord but the program doesn't seem to update its variables until it is restarted.

Include a reproducible code sample here, if possible:

function createVoiceChan(msg, chanName) {
    msg.guild.createChannel(chanName, 'voice').then( // Create the actual voice channel.
        (chan) => {
            chan.setParent(msg.channel.parent.id).then( // Move the voice channel to the current message's parent category.
                (chan2) => {
                    console.log(`Set the category of ${chan2.name} to ${chan2.parent.name}`);
                    chan2.overwritePermissions(msg.guild.roles.find('name', '@everyone'), { 'CREATE_INSTANT_INVITE' : false }); // Give the channel some standard permissions.
                }
            ).catch(console.error);
        }
    ).catch(console.error);
}

function debugMe(msg) {
    msg.guild.channels.map(function(elem) {
        if(elem.parent != null) {
            console.log("\t" + elem.name + " | " + elem.parent.name);   
        } else {
            console.log(elem.name + " | " + elem.parentID); // undefined if it's a category, null if it's a newly created channel that failed to update its parent.
        }
    });
}

discord.js version: 11.3.2
node.js version: v6.2.1
Operating system: Windows 10
Priority this issue should have – please be realistic and elaborate if possible: medium -- It's a featured required for the automatic creation and deletion of channels. Which is one of the core features for a program I am developing. But, it's not a security or breaking bug, so it's not high.

[ ] I found this issue while running code on a user account
[ ] I have also tested the issue on latest master, commit hash:

Keswiik commented 6 years ago

I am unable to reproduce this issue on 11.3.2 using both node v8.4.0 and v6.2.1 on Windows 10.

Makosai commented 6 years ago

What is your output for debugMe()? Does it actually show the parent for the newly created channel?

Keswiik commented 6 years ago

Alright, I did end up managing to reproduce the issue (I was dumb and didn't use your exact code). It seems that, potentially due to some bad logic, <Client>.channels and <Guild>.channels are ending up with different instances of their respective channels. I'm at work so I didn't have a ton of time to look, but here's why I think that:

In ClientDataManager, we have:

  newChannel(data, guild) {
    const already = this.client.channels.has(data.id);
    let channel;
    if (data.type === Constants.ChannelTypes.DM) {
      channel = new DMChannel(this.client, data);
    } else if (data.type === Constants.ChannelTypes.GROUP_DM) {
      channel = new GroupDMChannel(this.client, data);
    } else {
      guild = guild || this.client.guilds.get(data.guild_id);
      if (guild) {
        if (data.type === Constants.ChannelTypes.TEXT) {
          channel = new TextChannel(guild, data);
          guild.channels.set(channel.id, channel);
        } else if (data.type === Constants.ChannelTypes.VOICE) {
          channel = new VoiceChannel(guild, data);
          guild.channels.set(channel.id, channel);
        } else if (data.type === Constants.ChannelTypes.CATEGORY) {
          channel = new CategoryChannel(guild, data);
          guild.channels.set(channel.id, channel);
        }
      }
    }

And in RESTMethods, we have:

  createChannel(guild, channelName, channelType, overwrites, reason) {
    . . .
    return this.rest.makeRequest('post', Endpoints.Guild(guild).channels, true, {
      name: channelName,
      type: channelType ? Constants.ChannelTypes[channelType.toUpperCase()] : 'text',
      permission_overwrites: overwrites,
    }, undefined, reason).then(data => this.client.actions.ChannelCreate.handle(data).channel);
  }

It looks as though we'll end up triggering ChannelCreateAction twice - once from the WS event and once from RestMethods#createChannel. Due to how createChannel is written, guilds will receive a new instance of an already existing channel. Here's what I got after printing a stacktrace in Collection#set (as dumb as that may be).

From the ClientCreateAction triggered from RESTMethods#createChannel, it looks like it's updating both client and guild collections:

GuildStacktrace
    at Map.set (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\util\Collection.js:29:19)
    at ClientDataManager.newChannel (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\ClientDataManager.js:61:26)
    at ChannelCreateAction.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\actions\ChannelCreate.js:6:40)
    at ChannelCreateHandler.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\packets\handlers\ChannelCreate.js:7:34)
    at WebSocketPacketManager.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\packets\WebSocketPacketManager.js:103:65)
    at WebSocketConnection.onPacket (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\WebSocketConnection.js:333:35)
    at WebSocketConnection.onMessage (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\WebSocketConnection.js:296:17)
    at WebSocket.onMessage (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\node_modules\ws\lib\event-target.js:120:16)
    at emitOne (events.js:116:13)
    at WebSocket.emit (events.js:211:7)

ClientStacktrace
    at Map.set (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\util\Collection.js:29:19)
    at ClientDataManager.newChannel (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\ClientDataManager.js:74:28)
    at ChannelCreateAction.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\actions\ChannelCreate.js:6:40)
    at ChannelCreateHandler.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\packets\handlers\ChannelCreate.js:7:34)
    at WebSocketPacketManager.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\packets\WebSocketPacketManager.js:103:65)
    at WebSocketConnection.onPacket (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\WebSocketConnection.js:333:35)
    at WebSocketConnection.onMessage (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\websocket\WebSocketConnection.js:296:17)
    at WebSocket.onMessage (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\node_modules\ws\lib\event-target.js:120:16)
    at emitOne (events.js:116:13)
    at WebSocket.emit (events.js:211:7)

Then, afterwards, we get another Collection#set triggered on the guild via WS event:

GuildStacktrace
    at Map.set (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\util\Collection.js:29:19)
    at ClientDataManager.newChannel (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\ClientDataManager.js:61:26)
    at ChannelCreateAction.handle (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\actions\ChannelCreate.js:6:40)
    at rest.makeRequest.then.data (C:\Users\dd046364\PersonalWork\test-bot\node_modules\discord.js\src\client\rest\RESTMethods.js:278:74)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

It can be fixed by changing if (guild) to if (guild && !already) in ClientDataManager#newChannel, which I tested locally, but I'm not sure what else that may break.

Makosai commented 6 years ago

Any quick fixes that you can think of on my end? If not, I'll try and look in the library and see if I can fix it on my own and post my findings.

Keswiik commented 6 years ago

Refresh the thread, I edited in a quick fix you could do by navigating to node_modules/discord.js/src/client/ClientDataManager.js#58 - i just can't guarantee it won't break anything else.

Or you can just get all channels from <Client>.channels for now, until the issue is actually fixed.

Makosai commented 6 years ago

Alright, if (guild && !already) fixed the error with setParent(). But, there's another issue where createChannel() is now returning undefined channels.

function createVoiceChan(msg, chanName, pos) {
    msg.guild.createChannel(chanName, 'voice').then( // Create the actual voice channel.
        (chan) => {
            if(typeof chan == 'undefined')
                console.log("Channel is undefined.");
            ...
}

which gives the following error when trying to use chan to call setParent():

TypeError: Cannot read property 'setParent' of undefined
    at msg.guild.createChannel.then(...)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

Makosai commented 6 years ago

      if (guild && !already) {
        if (data.type === Constants.ChannelTypes.TEXT) {
          channel = new TextChannel(guild, data);
          guild.channels.set(channel.id, channel);
        } else if (data.type === Constants.ChannelTypes.VOICE) {
          channel = new VoiceChannel(guild, data);
          guild.channels.set(channel.id, channel);
        } else if (data.type === Constants.ChannelTypes.CATEGORY) {
          channel = new CategoryChannel(guild, data);
          guild.channels.set(channel.id, channel);
        }
      } else if(already) {
          channel = this.client.channels.get(data.id);
      }

Adding an else if to return the channel seems to fix the undefined error. I tested it and it works, but I don't know if it will break randomly in the future.

Lewdcario commented 6 years ago

Should be resolved as of 11.4.1

qaisjp commented 4 years ago

Should be resolved as of 11.4.1

Which commit?

almostSouji commented 4 years ago

Which commit?

Stable has moved on to 11.5.1, if you are still on 11.4.x you should consider updating anyways. The library has since had multiple bug fixes and additional functionality to benefit from.

If you want to create a channel inside a category, for example, you can now provide it as option during creation itself without having to move it after the creation https://discord.js.org/#/docs/main/stable/class/Guild?scrollTo=createChannel

note: as we abide to semantic versioning there are no breaking changes from .x to .y, so there really is no reason to not update to 11.5.1 (current stable)

discordjs / discord.js

VoiceChannel.setParent() doesn't update its own variable #2400