[Reviewed] fix: add error handler for client.login() rejection (Fixes #4)

[dromero14521]

This PR adds a catch block to client.login() to cleanly exit if there's a token or network error. Claiming issue #4 from the community bounty #21.

[Copilot]

Pull request overview

This PR adds explicit error handling around client.login() so the bot logs a login failure and exits with a non-zero status instead of leaving a rejected promise unhandled.

Changes:

• Add a .catch() handler to client.login(process.env.DISCORD_TOKEN)
• Log the login error and terminate the process with an error status

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

PR metadata/title says "Fixes #4" (unknown prefix command handling), but this change only adds error handling for client.login() (bounty board item #21.4). Please update the PR title/description and/or linked issue reference so it closes the correct issue and doesn't mislead reviewers.

[Copilot]

Using process.exit(1) can terminate the process immediately and potentially cut off pending I/O (including log flushing). Prefer setting process.exitCode = 1 and returning (or rethrowing) so Node can exit naturally after the error is logged.

Suggested change

	process.exit(1);
	process.exitCode = 1;

[TrivCodez]

🔍 Pull Request Review — Issue #4

Thanks for tackling the unhandled login rejection issue, @dromero14521. Your approach is fundamentally correct, but there are several issues preventing a clean merge.

---

🚨 Critical — BLOCKING MERGE

🔀 Merge Conflict & Outdated Base

• The target src/bot.js already contains a .catch() handler for client.login() (line ~52: // Item #4: Handle potential login rejections), meaning this PR is based on an older commit
• Mergeable state is "dirty" — the branch needs rebasing against current main
• Fix Required: Rebase your branch to include the latest changes, then modify the existing catch handler rather than adding a duplicate

🐛 Incomplete Error Handling

• When login fails, the bot is in a non-functional state; process.exit(1) is appropriate
• However, your implementation is inconsistent with the repo's logging style ('Failed to login:' vs '[Error]: Discord login failed:')
• Your version also removes the .message property access, which could leak sensitive tokens in full error objects
• Fix Required:

- client.login(process.env.DISCORD_TOKEN).catch(error => {
-   console.error('[Error]: Discord login failed:', error.message);
- });
+ client.login(process.env.DISCORD_TOKEN).catch(error => {
+   console.error('[Error]: Discord login failed:', error.message);
+   process.exit(1);
+ });

🎨 Code Style & Hygiene

• Missing newline at end of file (\ No newline at end of file in diff)
• Inconsistent indentation (3 spaces instead of the file's apparent 2-space standard)
• Fix Required: Add final newline, use consistent indentation

---

💡 Suggestions

Exit Code Semantics

• Consider exiting with code 1 for token errors but 0 for intentional shutdown signals — though for a bot, any login failure is fatal, so 1 is acceptable

Error Message Clarity

• The message could be more actionable: 'Discord login failed. Check your DISCORD_TOKEN environment variable.'

---

✅ Good

• Correctly identifies the issue: unhandled promise rejection on client.login() can crash the process in newer Node.js versions
• Right approach: sync error handling with process termination for a stateless service

---

Please rebase your branch against the latest main, update the existing catch handler (don't create a second one), and ensure consistent logging style and file formatting. The core logic you propose (process.exit(1)) is the right solution — it just needs to be applied to the current codebase structure.