discord.js icon indicating copy to clipboard operation
discord.js copied to clipboard

fix: remove potentially erroneous check

Open monbrey opened this issue 1 year ago • 5 comments

In fixing a bug, #8598 seems to have introduced a different bug in which the InteractionCollectors checks would incorrectly reject interactions coming from MessageComponents in a reply to other MessageComponentInteractions.

Removing this check seems to resolve the issue, and I believe I've tested the original scenario too. However, I'm not certain so would appreciate feedback from @RedGuy12 and @ImRodry who fixed/raised the original issue.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

monbrey avatar Jan 03 '24 00:01 monbrey

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2024 0:52am
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2024 0:52am

vercel[bot] avatar Jan 03 '24 00:01 vercel[bot]

I have a feeling this shouldn't work just by looking at the code, but my tests do pass and I don't think there was anything more complicated than the test provided in the initial issue, so it's fine by me. Maybe RedGuy can explain why he added that line though

ImRodry avatar Jan 03 '24 02:01 ImRodry

I have a feeling this shouldn't work just by looking at the code, but my tests do pass and I don't think there was anything more complicated than the test provided in the initial issue, so it's fine by me. Maybe RedGuy can explain why he added that line though

Exactly why I pinged you both for a sanity check 😄 Seemed too easy to be right.

monbrey avatar Jan 03 '24 03:01 monbrey

Maybe RedGuy can explain why he added that line though

Honestly can't remember, it does look like it should work with it tho /shrug

cobaltt7 avatar Jan 06 '24 19:01 cobaltt7

Do not merge this change! If you do that you will have other problems 😅 Here is a simple POC of the problem:

require('dotenv').config();

const { Client, GatewayIntentBits, ButtonBuilder, ButtonStyle, ComponentType, MessageFlags } = require('discord.js');

const client = new Client({ intents: [GatewayIntentBits.Guilds, GatewayIntentBits.GuildMessages, GatewayIntentBits.MessageContent] });

client.once('ready', () => {
    console.log('Bot is ready!');
});
client.on('messageCreate', (message) => {
    if (message.author.bot) {
        return;
    }

    if (message.content === 'ping') {
        const button = new ButtonBuilder()
            .setStyle(ButtonStyle.Primary)
            .setLabel('Click me!')
            .setCustomId('click_me');

        message.reply({ content: 'pong', components: [{ type: ComponentType.ActionRow, components: [button] }] });
    }
});

let awaitMessageComponent = false;
client.on('interactionCreate', async (interaction) => {
    if (!interaction.isMessageComponent()) {
        // Type guard
        return;
    }

    if (awaitMessageComponent) {
        // Skip when we are awaiting to show the bug (collect all component interaction)
        return;
    }

    if (interaction.message.flags.has(MessageFlags.Ephemeral)) {
        const button = new ButtonBuilder()
            .setStyle(ButtonStyle.Danger)
            .setLabel('DO NOT CLICK!')
            .setCustomId('click_me_await');

        const msg = await interaction.update({
            content: 'Do not click the button below! Click again on the button above!',
            components: [{ type: ComponentType.ActionRow, components: [button] }]
        });

        awaitMessageComponent = true;
        await msg.awaitMessageComponent({ componentType: ComponentType.Button, time: 10_000 })
            .then( i => {
                i.reply({ content: 'Component collected!', ephemeral: true });
            })
            .catch( e => {
                console.error(e);
            });
        awaitMessageComponent = false;
    } else {
        // To encounter the bug we need to call "update" on an ephemeral message in order to receive, as the return
        // of the call, an InteractionResponse instance and then call "awaitMessageComponent" on it.

        const button = new ButtonBuilder()
            .setStyle(ButtonStyle.Danger)
            .setLabel('Click this one!')
            .setCustomId('click_me');

        interaction.reply({
            content: interaction.message.content === 'pong' ? 'Click this one please' : 'Click the button below!',
            components: [{ type: ComponentType.ActionRow, components: [button] }],
            ephemeral: true
        });
    }
});

client.login(process.env.BOT_TOKEN);

You'll be able to understand and run the bot easily, I've got to go to work so I can't do anything more detailed at the moment. If you need something don't hesitate to ask (I was working on a fix because I've seen the same bug today while putting up to date discord.js)

Apokalypt avatar Jan 08 '24 17:01 Apokalypt

Given Discord now documents improved interaction metadata this PR is no longer the right approach.

monbrey avatar Mar 19 '24 21:03 monbrey