Sapphire - Imagine a framework•2y ago

Very intense application command fetching

I'm using the new sapphire, v4.0.1 , with discord js v14.7.1 Suddenly, the the bot does an incredible amount of fetching of application commands, resulting in rate limit.

{"timeToReset":1100,"limit":50,"method":"GET","hash":"Global(GET:/applications/:id/commands)","url":"https://discord.com/api/v10/applications/ID/commands?with_localizations=true","route":"/applications/:id/commands","majorParameter":"global","global":true}

I have no clue why as I do not use any application commands. Neither any clue to what triggers it. It could be a discord.js problem of course but since I dont have any code at all relating to application commands, I think it's more likely sapphire?

Solution:

@bomi can you test the changes made in the PR release if they achieve the same end result?

npm install @sapphire/framework@pr-598

npm install @sapphire/framework@pr-598

...

GitHub

fix(Command): Do not attempt to execute application command reloadi...

Back in v2.5.1 the Command class did not have an overwritten reload method at all: https://github.com/sapphiredev/framework/blob/v2.5.1/src/lib/structures/Command.ts This suggests that we didn'...

Jump to solution

37 Replies

bomiOP•2y ago

Is there a way to disable application command registries?

drainpixie•2y ago

@Favna this sounds interesting

bomiOP•2y ago

Do you know some more logging I could add around this, I am not familiar with the ApplicationCommandRegistries

Favna•2y ago

there is no way to disable it but if there are no commands to register then it simply does nothing there is basically an if (!something.length) return in there like it's one of the first lines called

bomiOP•2y ago

And it only adds the command to the array/map/etc to be processed if you call something like this?

registerApplicationCommands(registry) {
    registry.registerChatInputCommand((builder) =>
      builder.setName('ping').setDescription('Ping bot to see if it is alive')
    );
  }

registerApplicationCommands(registry) {
    registry.registerChatInputCommand((builder) =>
      builder.setName('ping').setDescription('Ping bot to see if it is alive')
    );
  }

Favna•2y ago

correct

bomiOP•2y ago

It has happened... Hmm, about 5 times now

Favna•2y ago

@Vladdy tbh

bomiOP•2y ago

What if there is some old slash command registered on discord could that affect it?

vladdy•2y ago

I'm gonna need a bit more than this, as we only fetch global commands once on ready, and then whenever a command or a store is reloaded

bomiOP•2y ago

Oh oh oh

vladdy•2y ago

Also hitting the ratelimit isn't a big deal unless it slows down some other code, djs handles them for you already

bomiOP•2y ago

Yes I have been reloading

vladdy•2y ago

How often have you been reloading

bomiOP•2y ago

Unfortunately in this case there was 10k calls resulting in global rate limit

vladdy•2y ago

Uh yeah so that's not what the 10k limit is 10k limit is for 403 or 401, aka missing access or missing permissions 10k 429s in 10 minutes sounds kinda impossible

Favna•2y ago

could it be that we don't properly check if it should register at all when calling command#reload tho Thonkang

bomiOP•2y ago

Actuall yeah sorry. I havent counted them. But I am looking at the lines in the rate limit log. There was 10k lines of rate limit logging

let cmdsrelo = botclient.stores.get("commands")

for (let [key, value] of cmdsrelo) {
  value.reload()
}

let cmdsrelo = botclient.stores.get("commands")

for (let [key, value] of cmdsrelo) {
  value.reload()
}

This is what I do hehe

vladdy•2y ago

Oh god Please just use commands.reloadAll() If you want to reload all commands

bomiOP•2y ago

let cmdsrelo = await botclient.stores.get("commands")
cmdsrelo.reloadAll()

let cmdsrelo = await botclient.stores.get("commands")
cmdsrelo.reloadAll()

like this?

vladdy•2y ago

Ye I don't remember fully if that's the method name

bomiOP•2y ago

Im scared

vladdy•2y ago

@Favna maybe you can help as I'm currently away from laptop

bomiOP•2y ago

There is CommandStore.loadAll() ?

public override async loadAll() {
        await super.loadAll();

        // If we don't have an application, that means this was called on login...
        if (!this.container.client.application) return;

        // super.loadAll() currently deletes all application command registries while unloading old pieces,
        // re-register application commands to ensure allGuildIdsToFetchCommandsFor has new guild ids for getNeededRegistryParameters
        for (const command of this.values()) {
            if (command.registerApplicationCommands) {
                try {
                    await command.registerApplicationCommands(command.applicationCommandRegistry);
                } catch (error) {
                    emitRegistryError(error, command);
                }
            }
        }

        const { applicationCommands, globalCommands, guildCommands } = await getNeededRegistryParameters(allGuildIdsToFetchCommandsFor);

        for (const command of this.values()) {
            // eslint-disable-next-line @typescript-eslint/dot-notation
            await command.applicationCommandRegistry['runAPICalls'](applicationCommands, globalCommands, guildCommands);

            // Reinitialize the aliases
            for (const nameOrId of command.applicationCommandRegistry.chatInputCommands) {
                this.aliases.set(nameOrId, command);
            }

            for (const nameOrId of command.applicationCommandRegistry.contextMenuCommands) {
                this.aliases.set(nameOrId, command);
            }
        }
    }

public override async loadAll() {
        await super.loadAll();

        // If we don't have an application, that means this was called on login...
        if (!this.container.client.application) return;

        // super.loadAll() currently deletes all application command registries while unloading old pieces,
        // re-register application commands to ensure allGuildIdsToFetchCommandsFor has new guild ids for getNeededRegistryParameters
        for (const command of this.values()) {
            if (command.registerApplicationCommands) {
                try {
                    await command.registerApplicationCommands(command.applicationCommandRegistry);
                } catch (error) {
                    emitRegistryError(error, command);
                }
            }
        }

        const { applicationCommands, globalCommands, guildCommands } = await getNeededRegistryParameters(allGuildIdsToFetchCommandsFor);

        for (const command of this.values()) {
            // eslint-disable-next-line @typescript-eslint/dot-notation
            await command.applicationCommandRegistry['runAPICalls'](applicationCommands, globalCommands, guildCommands);

            // Reinitialize the aliases
            for (const nameOrId of command.applicationCommandRegistry.chatInputCommands) {
                this.aliases.set(nameOrId, command);
            }

            for (const nameOrId of command.applicationCommandRegistry.contextMenuCommands) {
                this.aliases.set(nameOrId, command);
            }
        }
    }

vladdy•2y ago

Yes Which i haven't updated to support bulk overwrite fuck me

bomiOP•2y ago

Well I don't know that much about this code and how it works but it seems like it fetches the application commands from discord upon reload I dont really dare trying it but I think the problem is replicable

async reload() {
    // This looks like it runs even when its not a slash/application command?

    const { applicationCommands, globalCommands, guildCommands } = await getNeededParameters_js.getNeededRegistryParameters(updatedRegistry.guildIdsToFetch);
    await updatedRegistry["runAPICalls"](applicationCommands, globalCommands, guildCommands);


    for (const nameOrId of updatedRegistry.chatInputCommands) {
      store.aliases.set(nameOrId, updatedPiece);
    }
    for (const nameOrId of updatedRegistry.contextMenuCommands) {
      store.aliases.set(nameOrId, updatedPiece);
    }
  }

async reload() {
    // This looks like it runs even when its not a slash/application command?

    const { applicationCommands, globalCommands, guildCommands } = await getNeededParameters_js.getNeededRegistryParameters(updatedRegistry.guildIdsToFetch);
    await updatedRegistry["runAPICalls"](applicationCommands, globalCommands, guildCommands);


    for (const nameOrId of updatedRegistry.chatInputCommands) {
      store.aliases.set(nameOrId, updatedPiece);
    }
    for (const nameOrId of updatedRegistry.contextMenuCommands) {
      store.aliases.set(nameOrId, updatedPiece);
    }
  }

Favna•2y ago

ill see what I can find after I finish 2 more cyberpunk missions and have dinner been meaning to look into reload because of bulk overwrite anyway

bomiOP•2y ago

there is 10k servers about 120 commands, 16-32 shards

Favna•2y ago

thought you said you dont use application commands?

bomiOP•2y ago

Yes, sorry. 0 Application commands, 120 message.content commands

Favna•2y ago

yeah those dont care outside of the using loadAll vs reload

bomiOP•2y ago

I think that a command type can be determined Like so command.supportsChatInputCommands() So I am tempted to try this modified version of reload()

async reload() {
    const store = this.store;


    // if it's a chatInputCommand, ContextMenuCommand, or AutocompleteCommand, then it's a command that uses application commands. Thus it needs to be reloaded.
    if (this.supportsChatInputCommands() || this.supportsContextMenuCommands() || this.supportsAutocompleteInteractions()) {

      const registry = this.applicationCommandRegistry;
      for (const nameOrId of registry.chatInputCommands) {
        const aliasedPiece = store.aliases.get(nameOrId);
        if (aliasedPiece === this) {
          store.aliases.delete(nameOrId);
        }
      }
      for (const nameOrId of registry.contextMenuCommands) {
        const aliasedPiece = store.aliases.get(nameOrId);
        if (aliasedPiece === this) {
          store.aliases.delete(nameOrId);
        }
      }
      registry.chatInputCommands.clear();
      registry.contextMenuCommands.clear();
      registry.guildIdsToFetch.clear();
      registry["apiCalls"].length = 0;
    }
    await super.reload();
    const updatedPiece = store.get(this.name);
    if (!updatedPiece)
      return;

    if (this.supportsChatInputCommands() || this.supportsContextMenuCommands() || this.supportsAutocompleteInteractions()) {
      const updatedRegistry = updatedPiece.applicationCommandRegistry;
      if (updatedPiece.registerApplicationCommands) {
        try {
          await updatedPiece.registerApplicationCommands(updatedRegistry);
        } catch (err) {
          emitRegistryError_js.emitRegistryError(err, updatedPiece);
          return;
        }
      }
      const { applicationCommands, globalCommands, guildCommands } = await getNeededParameters_js.getNeededRegistryParameters(updatedRegistry.guildIdsToFetch);
      await updatedRegistry["runAPICalls"](applicationCommands, globalCommands, guildCommands);
      for (const nameOrId of updatedRegistry.chatInputCommands) {
        store.aliases.set(nameOrId, updatedPiece);
      }
      for (const nameOrId of updatedRegistry.contextMenuCommands) {
        store.aliases.set(nameOrId, updatedPiece);
      }
    }

async reload() {
    const store = this.store;


    // if it's a chatInputCommand, ContextMenuCommand, or AutocompleteCommand, then it's a command that uses application commands. Thus it needs to be reloaded.
    if (this.supportsChatInputCommands() || this.supportsContextMenuCommands() || this.supportsAutocompleteInteractions()) {

      const registry = this.applicationCommandRegistry;
      for (const nameOrId of registry.chatInputCommands) {
        const aliasedPiece = store.aliases.get(nameOrId);
        if (aliasedPiece === this) {
          store.aliases.delete(nameOrId);
        }
      }
      for (const nameOrId of registry.contextMenuCommands) {
        const aliasedPiece = store.aliases.get(nameOrId);
        if (aliasedPiece === this) {
          store.aliases.delete(nameOrId);
        }
      }
      registry.chatInputCommands.clear();
      registry.contextMenuCommands.clear();
      registry.guildIdsToFetch.clear();
      registry["apiCalls"].length = 0;
    }
    await super.reload();
    const updatedPiece = store.get(this.name);
    if (!updatedPiece)
      return;

    if (this.supportsChatInputCommands() || this.supportsContextMenuCommands() || this.supportsAutocompleteInteractions()) {
      const updatedRegistry = updatedPiece.applicationCommandRegistry;
      if (updatedPiece.registerApplicationCommands) {
        try {
          await updatedPiece.registerApplicationCommands(updatedRegistry);
        } catch (err) {
          emitRegistryError_js.emitRegistryError(err, updatedPiece);
          return;
        }
      }
      const { applicationCommands, globalCommands, guildCommands } = await getNeededParameters_js.getNeededRegistryParameters(updatedRegistry.guildIdsToFetch);
      await updatedRegistry["runAPICalls"](applicationCommands, globalCommands, guildCommands);
      for (const nameOrId of updatedRegistry.chatInputCommands) {
        store.aliases.set(nameOrId, updatedPiece);
      }
      for (const nameOrId of updatedRegistry.contextMenuCommands) {
        store.aliases.set(nameOrId, updatedPiece);
      }
    }

This modifiaction was seemingly successful.

vladdy•2y ago

hmmmmmmmmmmmmmmmmm

bomiOP•2y ago

Its probably not the way you would want it, but I think it would be nice if reload had this check

Solution

Favna•2y ago

@bomi can you test the changes made in the PR release if they achieve the same end result?

npm install @sapphire/framework@pr-598

npm install @sapphire/framework@pr-598

https://github.com/sapphiredev/framework/pull/598 Edit: Resolved with this PR, will be released in v4.1.0

GitHub

fix(Command): Do not attempt to execute application command reloadi...

Back in v2.5.1 the Command class did not have an overwritten reload method at all: https://github.com/sapphiredev/framework/blob/v2.5.1/src/lib/structures/Command.ts This suggests that we didn'...

bomiOP•2y ago

I can But it would be nice if there was someway I could log any times the bot tries to fetch application command endpoints to make testing safer but idk how to I'll just... Take the chance.. Hmm trying it on one shard only I did not see any problem It seems to be successful on all shards. Thanks!!

Favna•2y ago

good then we can merge it

Gaming

Programming

Very intense application command fetching