Skip to content
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

Known issues and required features #4

Open
1 of 7 tasks
savioxavier opened this issue Feb 16, 2022 · 5 comments
Open
1 of 7 tasks

Known issues and required features #4

savioxavier opened this issue Feb 16, 2022 · 5 comments

Comments

@savioxavier
Copy link
Owner

savioxavier commented Feb 16, 2022

Bot is currently kinda unstable (due to bad code) and needs a few fixes

  • Better interaction handling. Bot usually crashes due to an "Unknown Message" error (and usually restarts, thanks to Railway) but that's a problem that needs to be fixed
  • Better error handling, due to errors like "Unknown Interaction" and "Unknown Message" crashing the bot, I had to pass empty catch statements all over the code, which is bad practice really
  • Button interaction error: interactions usually fail sometimes. Not good.
  • Better slash command deployment handling. Currently the bot registers slash command on every run in production and I'm not really sure if that's a good idea. For some reason, I couldn't execute two scripts in succession in Railway, even with a process.exit() in the code to move on to the next script
  • A proper logger (eg: pino)
  • Command cleanup and refactoring
  • A few bug fixes. A lot of bug fixes

If you do know how to fix these issues, please submit a PR, that would be much appreciated.

@ItzDerock
Copy link

Button interaction error: interactions usually fail sometimes. Not good.

This could probably be fixed by moving the button-handling system out of neofetch.js command and into its own event.
Set the customid of the button to be <action>;<user id>. Then you won't need to implement a timeout system on the buttons and reduces memory usage.

Only downside to this is that unfortunately discord doesn't provide a way to delete the original message. You can use buttonInteraction.message.delete(), but that requires the Manage Messages permission on the bot. So a timeout for this or an error message would need to be kept/implemented.

I also looked over the code and saw,

        await action.deferUpdate();
        await action.editReply({
          content: "Here's your neofetch in mobile mode!",
          embeds: [mobileEmbed],
        });

this can be simplified to just a action.update(...) call, which achieves the same thing.

@savioxavier
Copy link
Owner Author

Thanks a lot of the suggestion @ItzDerock, the discord.js guide/docs didn't have a lot of precise info on how to handle button interactions. I had experienced the errors since I started developing the bot and I had enquired about it to the support server as well, but received no useful information.

I also looked over the code and saw,

	await action.deferUpdate();
	await action.editReply({
    	content: "Here's your neofetch in mobile mode!",
        embeds: [mobileEmbed],
 	});

this can be simplified to just a action.update(...) call, which achieves the same thing.

Right, I guess I can do that!

This could probably be fixed by moving the button-handling system out of neofetch.js command and into its own event.
Set the customid of the button to be ;. Then you won't need to implement a timeout system on the buttons and reduces memory usage.

I'm still confused about this and would need a bit more clarification. Is it possible to discuss about this with you in Discord later, if that's alright?

@ItzDerock
Copy link

I'm still confused about this and would need a bit more clarification. Is it possible to discuss about this with you in Discord later, if that's alright?

Currently, looking at the code, every time /neofetch gets ran, 3 collectors are initiated -- which then means 3 more event listeners get added to client#on. Meaning if 5 people were to run the command, 15 additional handlers would be registered to the client, which isn't ideal for memory usage and performance in a larger scale application.

What I propose is that each button handler is moved into its own interactionCreate event registered to handle all buttons from any embed, rather than how it is right now which is an event per button per interaction.

The customId is how the bot will be able to know if the correct user pressed the button.
By setting it to, for example, neofetch;mobile;273629350476382218, in the event, you can split it by ;, and get find that this button is the mobile button for neofetch, and check if interaction.user.id matches the id in the customId.

Then you can use interaction.user for the user data etc and create the mobile version and call .update(...)

This will allow for one event to handle all buttons. And you also won't need to add an expiry/timeout.

If you want to talk more on Discord, I'd be down. What's your name#tag?

@savioxavier
Copy link
Owner Author

Okay, now that does make a lot of sense, I'll try implementing it locally and see if it works

I'm guessing this will prevent the DiscordAPIError: Unknown Message errors that occur whenever I try to press a button? Because there have been too many event listeners initiated and Discord can't seem to handle them?

If you want to talk more on Discord, I'd be down. What's your name#tag?

It's Skyascii#1860. I may be slow to respond though.

@ItzDerock
Copy link

I'm guessing this will prevent the DiscordAPIError: Unknown Message errors that occur whenever I try to press a button? Because there have been too many event listeners initiated and Discord can't seem to handle them?

Without knowing more info about the stack trace of the Unknown Message errors, can't be 100% sure.

But I have a strong feeling that this will fix this. Unknown Message can come from trying to edit the reply of an interaction after it has expired.

By having the button interaction code in its own file, we are locked to the scope of the button interaction, which has a different expiry than the original interaction, meaning we can't write code that may cause an Unknown Message error to pop up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants