I am writing a discord bot that can start and stop my minecraft server. It currently works, but I am still learning NodeJS and I could use some tips on how to optimize my code.
Calling a command will initiate a cooldown that will be checked. The status of the server is checked before attempting to start and stop it.
I think checking whether is-active returns "active" is redundant because it returns error codes by default.
require('dotenv').config();
const { Client, GatewayIntentBits, ActivityType, REST, Routes } = require('discord.js');
const { exec, execSync } = require('child_process')
const client = new Client({
intents: [GatewayIntentBits.Guilds]
});
// Command cooldown in milliseconds (m * s * ms)
const cooldownLength = 1 * 20 * 1000;
const commands = [
{
name: 'start',
description: 'Starts the server'
},
{
name: 'stop',
description: 'Stops the server'
}
];
const rest = new REST({ version: '10'}).setToken(process.env.TOKEN);
client.on('ready', () => {
// Attempts to register command
(async () => {
try {
console.log('Registering commands...');
await rest.put(
Routes.applicationGuildCommands(process.env.CLIENT_ID, process.env.GUILD_ID),
{ body: commands}
);
await rest.put(
Routes.applicationGuildCommands(process.env.CLIENT_ID, process.env.DEV_GUILD_ID),
{ body: commands}
);
console.log('Commands registered.');
} catch (error) {
console.log(`Error: ${error}`);
};
})();
console.log(`Logged in as ${client.user.tag}.`);
client.user.setActivity({
name: 'Minecraft',
type: ActivityType.Playing
});
});
// Stores when the cooldown is over
let cooldownEndtime = 0;
// Handles commands
client.on('interactionCreate', async (interaction) => {
if (!interaction.isChatInputCommand()) { return; }
if(interaction.guild.id == process.env.GUILD_ID && !interaction.member.roles.cache.has(process.env.ROLE)){
return interaction.reply("You do not have permission.")
}
// Start command
if (interaction.commandName == 'start') {
// Cooldown check
if(Date.now() < cooldownEndtime) {
return interaction.reply(`The server was recently started/stopped. Please wait ${Math.ceil((cooldownEndtime - currentTime) / 1000)} seconds.`);
}
// Checks whether the server is running
try {
const status = execSync(`sudo systemctl is-active ${process.env.SERVICE}`).toString().trim();
if (status === "active") {
return interaction.reply("Server is already running!");
}
} catch (error) {
// If systemctl fails, assume the service is not running
}
exec(`sudo systemctl start ${process.env.SERVICE}`, (error, stdout, stderr) => {
if(error){
console.log(error)
return interaction.reply(`Node error: ${error.message}`);
}
if (stderr) {
console.log(stderr)
return interaction.reply(`System error: ${stderr}`);
}
interaction.reply("Minecraft server started!");
cooldownEndtime = Date.now() + cooldownLength;
});
}
1 Answer 1
Having went down the rabbit hole of writing a Discord bot using discord.js myself, I think I can write a review for this.
Project (Infra)Structure
Having similarly started my Discord bot with a single .js file before, here are details on how my journey went that I think are applicable here too.
Use TypeScript, a Linter, and a Formatter
As mentioned above, my Discord bot started off as a single *.js file. However, as the code gets polished and more features get added, it helps having a stronger guarantee that I pass in the correct things to the correct places and general error-handling. Being able to document and guarantee that a function that accepts a number doesn't get passed in a string or that the function you're calling really does only ever return a boolean (instead of sometimes returning a number) saved a lot of my sanity.
It does cost you a build step (Node.js only very recently learned to read TypeScript, and even then it only manages to do it by ignoring the types, so not all TypeScript features work, so for best results you should still run tsc), but I think the benefits outweigh the cost. Plus, it's an opportunity to learn DevOps :)
Then again I come from a static and compiled language (C, C++, Java, C#), so maybe your experience was/is different.
Additionallly, while your code is readable, it has (IMO) dubious constructs such as == instead of === and formatting inconsistencies. Consider using a linter such as ESLint to catch those dubious constructs and a formatter such as Prettier or ESLint Stylistic (the former being more "standard" and opinionated, the latter being more customizable to your personal taste) to achieve a consistently formatted codebase with less hassle.
Use ES Modules instead of CommonJS
The Node.js ecosystem as a whole is moving over from CJS (require() and module.exports) to ESM (import and export). It brings various improvements, such as top-level await, i.e., using await outside of a function. Might as well keep up with it and deal with problems now rather than kicking the learning bucket down the road.
TS also uses import and export, even if it eventually turns into CJS at the end.
Setup a dedicated user
You mentioned in the comments that you run the bot as your personal user, and further, giving the access to the entirety of systemctl with no password, including starting and stopping arbitrary daemons with no password, not just the Minecraft ones. While it is unlikely your bot will be hacked, consider getting used to good security practices and setup a dedicated user to run your bot that can start and stop only the Minecraft ones with no password, leaving permission checks for others intact.
General Code
These are observations applicable to any project. Observations specific to discord.js further down.
Use a proper config system
Eventually you might want to load and store more complex data than just simple string values. Maybe you want it more structured than a simple KV pair, e.g., have sections and subsections. Maybe you want to ensure your data is a valid number or boolean rather than just some arbitrary string and hoping it is valid.
There might be a complete package to handle configs somewhere, but when I wrote my bot I used JSON (standard format, tools to write and parse those are ubiquitous) and wrote my own config loading/saving/reloading system.
Verify your config before using it
You load your .env file at the top. It works great on your dev machine. Then you move it to a different machine, but you forgot a config or two. Your bot explodes, possibly not where you expect it to.
Sure, you can read the stack trace and probably quickly remember what you forgot and where, but I think it would be nice for the bot to tell you that you're forgetting something from the start instead of when it becomes a problem. It might run fine for a while before exploding, and you might not be around to fix it when it happens...
Consider using something like Zod to ensure the data you get from untrusted sources is the format you expect it to be. Even if you don't want to bring in an entire library or write an entire config system, at least check upfront whether the data you will need are available. It could be as simple as const FOO = process.env.FOO ?? 42;.
Keep command handlers near the commands
You currently store a list of function names to register with Discord. But when a command gets used, you duplicate that list of names in the check. This is prone to errors, e.g., typos, forgetting to update the list, etc. Consider keeping the handlers in there as well, e.g.,
const commands = [
{
name: 'start',
exec: (interaction) => { /* ... */ }
},
// ...
]
client.on(Events.InteractionCreate, (interaction) => {
// Permission checks here
const command = commands.find(c => c.name === interaction.commandName)
if (!command) throw new Error('Unknown command') // Should never happen unless something goes wrong on Discord's side
command.exec(interaction)
})
As a bonus, this significantly reduces the chances you forget to implement a command, which I will assume you have with the stop command, since it is not in the code you have shown. Failing to respond to a command will cause Discord to report a rather mysterious "interaction failed" to the user, and given you have no logging system in place, you are oblivious to this error unless a user reports it to you directly.
Direct errors to stderr
Consider playing nice with the host system by directing errors to stderr using console.error() instead of the standard stdout using console.log(). This has the benefit that error messages can be more easily filtered out among the non-critical logs.
Use a main function
Or, at least organize them into a setup function (contanining, e.g., require('dotenv').config() and the various client.on()s) and a run function (containing, e.g., client.login(), which I cannot find in the code you show, so I'm not sure how you're running this bot).
Your bot is currently very simple, as was mine when it started. But as it grows, I find it more helpful to have a better organized control flow, instead of doing everything inline.
Discord Bot
These are observations for specifically writing a Discord bot with discord.js.
Use discord.js utilities where you can
I have never needed to manually use REST and Routes. Chances are, there's a nice abstraction for it already, e.g., GuildApplicationCommandManager#create, accessible via, e.g., for (const guild of client.guilds.fetch()) guild.commands.create(), and SlashCommandBuilder.
Additionally, some constants can be replaced with discord.js equivalents, e.g., use client.on(Events.InteractionCreate) instead of client.on('interactionCreate').
Using magic values and calling APIs manually negates a lot of the benefits of using a library.
Do not use .cache.foo(x)
What if the value you're looking for is not in the cache? You're going to see strange errors, e.g., missing data where it clearly isn't.
Use .fetch(x) instead. It already handles caching for you.
Conclusion
While many things can be improved, especially if OP plans to add more functionality (and thus complexity), I think this is a fine start to a Discord bot. Certainly reminds me a lot of when I started mine...
sudo something? What's the corresponding sudoers configuration? As which user does the bot process run?sudoin a program is very suspicious and should be at least accompanied by a long explanatory comment. \$\endgroup\$