New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove potentially erroneous check #10068
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
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. |
Honestly can't remember, it does look like it should work with it tho /shrug |
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) |
@@ -40,7 +40,7 @@ class InteractionCollector extends Collector { | |||
* The message from which to collect interactions, if provided | |||
* @type {?Snowflake} | |||
*/ | |||
this.messageId = options.message?.id ?? options.interactionResponse?.interaction.message?.id ?? null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this condition will lead to invalid interaction collected when we want to collect component interaction after the call of the "update" function on an ephemeral message
(see my long comment with a POC)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I suspected there would be an edge case this would miss. Will keep looking into it.
In fixing a bug, #8598 seems to have introduced a different bug in which the
InteractionCollector
s checks would incorrectly reject interactions coming fromMessageComponent
s in a reply to otherMessageComponentInteraction
s.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: