discord-nestjs icon indicating copy to clipboard operation
discord-nestjs copied to clipboard

Interaction already acknowledged when using two interactable messages

Open OdedItkinOW opened this issue 2 years ago • 8 comments

Describe the bug I have a simple /test command, that creates a simple message with a few buttons and a select menu. I have a simple collector used by that command, that calls interaction.update() on the original message whenever any button is interacted with.

If I only run /test once, everything works fine. However, if I run /test twice, without closing the collector in between (and hence have two messages that i'm supposed to collect from), any interaction with any of the collected messages (or any other one left from previous builds for that matter), will cause both collectors to receive the same interaction, meaning that the second one will throw: DiscordAPIError[40060]: Interaction has already been acknowledged error to be thrown

To Reproduce Steps to reproduce the behavior:

  1. run /test
  2. run /test again
  3. click any button on any interactable message
  4. "interaction has already been acknowledged" error is thrown

Expected behavior No double-creation of collectors. Or if they are doubly created, then a way to distinguish them between their target messages.

Desktop (please complete the following information):

  • OS: Windows 11
  • Browser: N/A
  • Version: 4.3.0 discord-nestjs

OdedItkinOW avatar Sep 15 '22 14:09 OdedItkinOW

Can you provide repo with reproduce?

fjodor-rybakov avatar Sep 15 '22 17:09 fjodor-rybakov

Not really, but it's a generated nestjs project with three classes that were changed (two services and a module, the module being loaded by the main thing), which are the following:

on-start.module.ts:

@Module({
  imports: [
    DiscordModule.forRootAsync({
        useFactory: () => ({
            token: '${discord.token}',
            discordClientOptions: {
                intents: [GatewayIntentBits.Guilds]
            },
            autoLogin: true
        })
    })
  ],
  providers: [OnStartService]
})
export class OnStartModule {}

on-start.service.ts

@Command({
    name: 'test',
    description: 'helloooooooo'
})
@UseCollectors(CollectorService)
@Injectable()
export class OnStartService implements DiscordCommand {
    handler(interaction: CommandInteraction): InteractionReplyOptions {
        const back = new ActionRowBuilder<ButtonBuilder>()
        
        .addComponents(
            new ButtonBuilder()
                .setCustomId('1')
                .setLabel('1')
                .setStyle(ButtonStyle.Primary)
            ,
            new ButtonBuilder()
                .setLabel('2')
                .setStyle(ButtonStyle.Link)
                .setURL('https://google.com')
            ,
            new ButtonBuilder()
                .setCustomId('3')
                .setLabel('3')
                .setStyle(ButtonStyle.Danger)
            ,
            new ButtonBuilder()
                .setCustomId('4')
                .setLabel('4')
                .setStyle(ButtonStyle.Secondary)
            ,
            new ButtonBuilder()
                .setCustomId('5')
                .setLabel('5')
                .setStyle(ButtonStyle.Success)
        );
        const dropdown = new ActionRowBuilder<SelectMenuBuilder>()
            .addComponents(
                new SelectMenuBuilder()
                    .setCustomId('select')
                    .setPlaceholder('Nothing selected')
                    .addOptions(
                        {
                            label: 'Select me',
                            description: 'This is a description',
                            value: 'hello',
                        },
                        {
                            label: 'You can select me too',
                            description: 'This is also a description',
                            value: 'second_option',
                        },
                    )
            )

        return { ephemeral: true, content: 'Pong!', components: [dropdown, back] };
    }
}

collector.service.ts

@InteractionEventCollector({
})
@Injectable({ scope: Scope.REQUEST })
export class CollectorService {
    constructor(
        @InjectCollector()
        private readonly collector: InteractionCollector<ButtonInteraction>,
      ) {
        console.log("created", collector) // this runs twice at project setup, and once per command invocation
      }
    
    @On('collect')
    async onCollect(interaction: ButtonInteraction): Promise<void> {
        await interaction.update({
            content: `A button was clicked! Specifically: ${interaction["values"] ? interaction["values"][0] : interaction["customId"]}`,
            components: [],
        });
        this.collector.stop();
        return;
    }
    
    @Once('end')
    onEnd(): void {
      console.log('end');
    }
}

OdedItkinOW avatar Sep 15 '22 18:09 OdedItkinOW

Update: After further debugging the issue, it seems like it stems from the fact that every message creates a collector, but the collectors themselves are not filtered on a per-message basis. This in turn simply means that every collector subscribes to all events from that channel.

It seems that the simplest solution would most likely be for me to pass a filter to each collector, only allowing it to interact with the message created by the given command. However, I'm currently uncertain of how exactly that could be achieved. I'm looking into the code further, and will update if I find a clean method of doing it

OdedItkinOW avatar Sep 18 '22 09:09 OdedItkinOW

Hey, sorry I don't want to hijack this thread, but I think it shows that a more complete example would be helpful and I think the reproduction is related to what I am requesting here: https://github.com/fjodor-rybakov/discord-nestjs/issues/869

@OdedItkinOW thanks for showing some example code with a MenuBuilder ;)

Phoscur avatar Sep 18 '22 11:09 Phoscur

Final update I think: It is possible to achieve this behavior by slightly circumventing the API. Specifically, if we go to on-start.service.ts, rather than directly returning the message components, it is possible to just add the following:

        const reply = await interaction.reply({ ephemeral: true, content: 'Pong!', components: [dropdown, back] });
        const collector = reply.createMessageComponentCollector()

And then to simply remove the UseCollectors() annotation.

Then, collector itself can be used to collect the interactions from this individual message. This works. But it's a "pure" discord.js solution, rather than properly utilizing discord-nestjs itself, and requires a partial re-implementation of discord-nestjs functionality, since it doesn't seem like this raw discord.js collector can then be cleanly inserted into the UseCollectors() annotation's flow.

I'll probably be using this at least for now, since my implementation is somewhat time-sensitive, but leaving this issue here and opened since I feel it is important, and would love to move to a cleaner approach in the future when possible.

edit: ~~it does however seem, that if it was possible to, in this particular line, add some logic to allow reply.createMessageComponentCollector() to be ran instead, it would work as a fix~~

edit 2: I started actually digging into it. Collector creation is currently done before the reply to a command is sent, so this approach is invalid. After discussing it further with a colleague, I decided to try and see if I can create a clean pull request that implements this feature

OdedItkinOW avatar Sep 18 '22 13:09 OdedItkinOW

discord-nestjs\node_modules@discordjs\rest\src\lib\handlers\SequentialHandler.ts:287 this.manager.globalReset = Date.now() + 1000; ^ DiscordAPIError[40060]: Interaction has already been acknowledged. at SequentialHandler.runRequest (\discord-nestjs\node_modules@discordjs\rest\src\lib\handlers\SequentialHandler.ts:287:15) [...] at PostInteractionCollector.onCollect (\discord-nestjs\packages\sample\interaction-collector\src\bot\post-interaction-collector.ts:8:5)

@fjodor-rybakov Reproduction within this repo in sample/interaction-collector: Run /song xyzmeh twice, then try to "play" both grafik

Phoscur avatar Sep 18 '22 15:09 Phoscur

@OdedItkinOW great to read that you want to provide a PR, not sure if I can help but I would like to! (Maybe let me have a look at your WIP for code review?)

I am still struggling with discord.js fundamentals... When the bot sample/interaction-collector is restarted, it won't "play" from previously sent commands before the restart. How to set up a collector which keeps collecting and never acknowledges the interaction?

Phoscur avatar Sep 18 '22 16:09 Phoscur

I'm still looking at the source code, I'll see about notifying you if and when I do start working on the pr.

As for the other question, I can answer that, but that's off-topic for the issue, so I'd rather move it somewhere else to avoid clutter

OdedItkinOW avatar Sep 18 '22 16:09 OdedItkinOW

In new version it will be possible to inject the event with which the collector was created. In @Filter() method you can filter the message so that the collector is attached to the original message. In this case, the error will no longer appear.

@Injectable({ scope: Scope.REQUEST })
@InteractionEventCollector({ time: 15000 })
export class PostInteractionCollector {
  constructor(
    @InjectCauseEvent()
    private readonly causeInteraction: ChatInputCommandInteraction,
  ) {}

  @Filter()
  filter(interaction: ButtonInteraction): boolean {
    return this.causeInteraction.id === interaction.message.interaction.id;
  }

  @On('collect')
  async onCollect(interaction: ButtonInteraction): Promise<void> {
    await interaction.update({
      content: 'A button was clicked!',
      components: [],
    });
  }

  @Once('end')
  onEnd(): void {
    console.log('end');
  }
}

fjodor-rybakov avatar Jan 29 '23 12:01 fjodor-rybakov