[Reviewed] fix: add folder existence checks (Item 3)#27
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent crashes when loading command/event directories by adding existence checks and auto-creating missing folders.
Changes:
- Add an
fs.existsSyncguard (and auto-create) before reading the commands directory indeploy-commands.js. - Add an
fs.existsSyncguard (and auto-create) before reading the events directory inbot.js.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/deploy-commands.js | Adds a missing-directory check and mkdirSync before readdirSync when deploying application commands. |
| src/bot.js | Adds a missing-directory check and mkdirSync before readdirSync when loading events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const commandsPath = path.join(__dirname, 'commands'); | ||
| if (!fs.existsSync(commandsPath)) { | ||
| console.warn(`[WARNING] Commands directory missing at ${commandsPath}. Auto-creating it.`); | ||
| fs.mkdirSync(commandsPath, { recursive: true }); |
There was a problem hiding this comment.
deploy-commands.js is still looking in path.join(__dirname, 'commands'), but this repo’s slash commands live under src/slashcommands/. With the new auto-create behavior, a missing src/commands will now be created and commandFiles will be empty, causing the subsequent rest.put(..., { body: [] }) to overwrite/delete all registered app commands. Update the path to the real slash command directory (and avoid creating a new empty commands dir).
| const commandsPath = path.join(__dirname, 'commands'); | |
| if (!fs.existsSync(commandsPath)) { | |
| console.warn(`[WARNING] Commands directory missing at ${commandsPath}. Auto-creating it.`); | |
| fs.mkdirSync(commandsPath, { recursive: true }); | |
| const commandsPath = path.join(__dirname, 'slashcommands'); | |
| if (!fs.existsSync(commandsPath)) { | |
| console.error(`[ERROR] Slash commands directory missing at ${commandsPath}. Aborting command deployment.`); | |
| process.exit(1); |
| if (!fs.existsSync(commandsPath)) { | ||
| console.warn(`[WARNING] Commands directory missing at ${commandsPath}. Auto-creating it.`); | ||
| fs.mkdirSync(commandsPath, { recursive: true }); | ||
| } | ||
| const commandFiles = fs.readdirSync(commandsPath).filter(f => f.endsWith('.js')); |
There was a problem hiding this comment.
Consider failing fast (non-zero exit) when the commands directory is missing or when no command files are found, rather than auto-creating the directory and continuing. As written, a CI/CD misconfiguration (wrong working dir, missing volume mount, etc.) can silently deploy an empty command set and wipe existing registered commands.
| if (!fs.existsSync(commandsPath)) { | ||
| console.warn(`[WARNING] Commands directory missing at ${commandsPath}. Auto-creating it.`); | ||
| fs.mkdirSync(commandsPath, { recursive: true }); |
There was a problem hiding this comment.
The PR title/description says "Fixes #3", but the referenced Issue #3 is about adding an /avatar slash command. This change is about directory existence checks (matching bounty-board item #21.3 instead). Please update the PR metadata (title/body/linked issue) so it closes the correct issue and doesn’t incorrectly resolve the avatar feature request.
| const eventsPath = path.join(__dirname, 'events'); | ||
| if (!fs.existsSync(eventsPath)) { | ||
| console.warn(`[WARNING] Events directory missing at ${eventsPath}. Auto-creating it.`); | ||
| fs.mkdirSync(eventsPath, { recursive: true }); |
There was a problem hiding this comment.
fs.mkdirSync(eventsPath, { recursive: true }) can still throw (e.g., read-only filesystem, missing permissions). Since the intent is "warn and keep running", wrap the mkdir in try/catch (and consider returning early or exiting) so the bot doesn’t crash with an unhandled exception when it can’t create the directory.
| fs.mkdirSync(eventsPath, { recursive: true }); | |
| try { | |
| fs.mkdirSync(eventsPath, { recursive: true }); | |
| } catch (error) { | |
| console.warn(`[WARNING] Failed to create events directory at ${eventsPath}: ${error.message}`); | |
| return; | |
| } |
This PR adds
fs.existsSyncchecks before attempting to read directories for commands and events. If they do not exist, it warns and auto-creates them instead of crashing. Claiming issue #3 from the community bounty #21.