Skip to content

[Reviewed] fix: handle client login rejection (Item 4)#34

Open
slendereater-sketch wants to merge 1 commit into
IN3PIRE:mainfrom
slendereater-sketch:fix/login-error-handling
Open

[Reviewed] fix: handle client login rejection (Item 4)#34
slendereater-sketch wants to merge 1 commit into
IN3PIRE:mainfrom
slendereater-sketch:fix/login-error-handling

Conversation

@slendereater-sketch
Copy link
Copy Markdown

Fixes Item 4 in Issue #21. Added .catch() to client.login() to handle connection errors and token failures gracefully. Relates to Issue #4.

@TrivCodez
Copy link
Copy Markdown
Contributor

🔍 Pull Request Review — Issue #21 (Item 4)

This PR adds error handling for client.login() to gracefully catch token and connection failures. The approach is correct, but again the branch needs rebasing.


🔍 Changes Reviewed

  • src/bot.js: Wrapped client.login() in .catch() with error logging and process.exitCode = 1
  • Total: +4 lines, -1 line across 1 file

🚨 Critical — BLOCKING MERGE

Merge conflict detected. The PR's base branch has moved ahead, blocking automatic merge.

Fix: Rebase onto current main

git fetch upstream
git checkout fix/login-error-handling
git rebase upstream/main  # Resolve any conflicts manually
git push --force-with-lease

🟡 Minor Issue

Missing trailing newline — The diff shows "No newline at end of file". Add a newline at EOF for POSIX compliance.


✅ What's Working

  • Proper .catch() error handling on Promise
  • Uses process.exitCode = 1 (graceful exit) instead of process.exit(1) (hard exit)
  • Clear error message logging
  • Addresses the exact requirement from Issue Critical & Enhancement Issues — Bounty Board #21 (Item 4)

⚖️ REQUEST CHANGES

Please rebase onto current main to resolve merge conflicts, then ping for merge. Also add the missing newline at EOF. The implementation is sound once rebased.

@TrivCodez TrivCodez changed the title fix: handle client login rejection (Item 4) [Reviewed] fix: handle client login rejection (Item 4) Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants