Enhanced Error Handling

[BillChirico]

Implement comprehensive error handling with user-friendly error messages, automatic retry for transient failures, and detailed error logging for debugging.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Enhanced error handling with user-friendly messages and actionable guidance for common issues including network failures, timeouts, rate limiting, and Discord-specific problems.
  • Automatic retry mechanism with intelligent backoff to gracefully handle transient errors.

Walkthrough

Two new utility modules are introduced: errors.js provides error classification, user-friendly messaging, and actionable guidance across categories like network, timeout, API, Discord, and config errors. retry.js implements configurable async retry logic with exponential backoff, integrating error classification to determine retryability.

Changes

Cohort / File(s)	Summary
Error Classification Utility src/utils/errors.js	New module exports ErrorType enum with 13 error categories and four functions: classifyError (maps errors to types), getUserFriendlyMessage (localized messaging), getSuggestedNextSteps (actionable guidance), and isRetryable (transience check for NETWORK, TIMEOUT, API_SERVER_ERROR, API_RATE_LIMIT).
Retry Mechanism Utility src/utils/retry.js	New module exports withRetry (async retry executor with exponential backoff, configurable maxRetries, baseDelay, maxDelay, and shouldRetry predicate) and createRetryWrapper (factory for pre-configured retry wrappers). Integrates error classification to determine eligibility for retry and includes context-aware logging.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant withRetry
    participant ErrorClassifier
    participant Operation
    participant Backoff

    Client->>withRetry: call withRetry(fn, options)
    
    loop Retry Loop (up to maxRetries)
        withRetry->>Operation: execute operation
        alt Operation Succeeds
            Operation-->>withRetry: return result
            withRetry-->>Client: return success
        else Operation Fails
            Operation-->>withRetry: throw error
            withRetry->>ErrorClassifier: classifyError(error)
            ErrorClassifier-->>withRetry: ErrorType
            withRetry->>ErrorClassifier: isRetryable(error)
            ErrorClassifier-->>withRetry: boolean
            
            alt Retryable & Retries Remaining
                withRetry->>Backoff: calculateBackoff(attempt)
                Backoff-->>withRetry: delay time
                withRetry->>withRetry: sleep(delay)
                Note over withRetry: Log retry attempt, loop continues
            else Non-Retryable or No Retries Left
                withRetry-->>Client: throw error
            end
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Enhanced Error Handling' is concise (23 characters), clearly descriptive, and directly reflects the main changeset which adds comprehensive error handling utilities.
Description check	✅ Passed	The description accurately describes the changeset, mentioning error classification, user-friendly messages, automatic retries for transient failures, and error logging—all core features implemented in the added modules.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch auto-claude/003-enhanced-error-handling

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In `@src/utils/errors.js`:
- Around line 91-94: The current detection only inspects the error message
string (`message`) to map to ErrorType.CONFIG_MISSING; update the logic in the
function in src/utils/errors.js to also check the error object's `code` property
(e.g., `code === 'ENOENT'`) and handle cases where the input may be an Error
object rather than a string, so that when either message includes 'config.json
not found' or `code === 'ENOENT'` you return ErrorType.CONFIG_MISSING; reference
the existing `message` variable and the `ErrorType.CONFIG_MISSING` symbol when
making this change.
- Around line 114-146: The messages map inside getUserFriendlyMessage is
recreated on every call; move the messages object to module scope (e.g., const
MESSAGES = { ... }) and have getUserFriendlyMessage reference
MESSAGES[errorType]. Update references to ErrorType keys accordingly and keep
the default fallback to MESSAGES[ErrorType.UNKNOWN] so behavior is unchanged
while avoiding repeated allocations.
- Around line 51-56: The ETIMEDOUT branch is currently classified as NETWORK and
thus never reaches the TIMEOUT check; update the logic in the function that maps
error codes (uses the variables code, message and the ErrorType enum) so
ETIMEDOUT maps to ErrorType.TIMEOUT: remove ETIMEDOUT from the first conditional
(the one returning ErrorType.NETWORK) or explicitly check ETIMEDOUT before the
NETWORK branch, and ensure the second conditional (checking ETIMEDOUT or
message.includes('timeout')) returns ErrorType.TIMEOUT.

In `@src/utils/retry.js`:
- Around line 27-33: The calculateBackoff function uses pure exponential backoff
which risks synchronized retries; modify calculateBackoff(attempt, baseDelay,
maxDelay) to add randomized jitter to the computed delay (for example by
applying a random factor or selecting a random value within a range around the
exponential value) and still cap the result at maxDelay: compute the exponential
delay as now, apply jitter via Math.random to spread retries, then return
Math.min(jitteredDelay, maxDelay). Ensure the jitter keeps delays non-negative
and documented in the function comment.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0286a77 and c2bfc4a.

📒 Files selected for processing (2)

• src/utils/errors.js
• src/utils/retry.js

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-11-26T01:57:34.920Z

Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/**/*.{ts,tsx} : Use `reportError(context, error)` from `src/lib/logger.ts` to report errors to Sentry with context metadata, falling back to console.error if Sentry is disabled

Applied to files:

• src/utils/errors.js

🧬 Code graph analysis (2)

src/utils/retry.js (2)

src/utils/errors.js (5)

• isRetryable (192-204)
• errorType (115-115)
• errorType (156-156)
• errorType (193-193)
• classifyError (43-105)

src/logger.js (3)

• warn (225-227)
• error (232-234)
• debug (211-213)

src/utils/errors.js (2)

src/logger.js (1)

• error (232-234)

src/utils/retry.js (2)

• errorType (67-67)
• isRetryable (49-55)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Cursor Bugbot

🔇 Additional comments (7)

src/utils/errors.js (3)

11-34: LGTM!

The ErrorType enum is well-organized with clear categorization of error types (network, API, Discord-specific, configuration, and unknown). Good practice to use string values for easier debugging and logging.

---

155-183: LGTM!

The function appropriately returns null for error types where no actionable suggestion exists (API_ERROR, UNKNOWN). The documented return type correctly indicates this behavior.

---

192-204: LGTM!

Good selection of retryable error types. Rate limits are correctly included since they're transient. The actual backoff handling in retry.js will manage the delay appropriately.

src/utils/retry.js (4)

8-9: LGTM!

Imports are appropriate - using the error classification utilities from the sibling module and logging functions from the logger.

---

16-18: LGTM!

Standard sleep implementation using Promise and setTimeout.

---

48-118: LGTM!

The retry logic is well-structured:

• Correctly handles the initial attempt plus retries
• Logs appropriately at different levels (warn on first failure, debug for retry attempts, error on final failure)
• Properly integrates with the error classification system
• Clean separation of retryable vs non-retryable error handling

---

126-129: LGTM!

Clean factory pattern allowing pre-configured retry wrappers. Option merging order correctly allows per-call overrides.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

⚠️ Potential issue | 🟠 Major

ETIMEDOUT classification is unreachable for TIMEOUT.

ETIMEDOUT is checked at line 51 and returns NETWORK, so the ETIMEDOUT check at line 54 for TIMEOUT will never execute. Since ETIMEDOUT specifically indicates a connection timeout, it should likely return TIMEOUT, not NETWORK.

🐛 Proposed fix

   // Network errors
-  if (code === 'ECONNREFUSED' || code === 'ENOTFOUND' || code === 'ETIMEDOUT') {
+  if (code === 'ECONNREFUSED' || code === 'ENOTFOUND') {
     return ErrorType.NETWORK;
   }
   if (code === 'ETIMEDOUT' || message.includes('timeout')) {
     return ErrorType.TIMEOUT;
   }

🤖 Prompt for AI Agents

In `@src/utils/errors.js` around lines 51 - 56, The ETIMEDOUT branch is currently
classified as NETWORK and thus never reaches the TIMEOUT check; update the logic
in the function that maps error codes (uses the variables code, message and the
ErrorType enum) so ETIMEDOUT maps to ErrorType.TIMEOUT: remove ETIMEDOUT from
the first conditional (the one returning ErrorType.NETWORK) or explicitly check
ETIMEDOUT before the NETWORK branch, and ensure the second conditional (checking
ETIMEDOUT or message.includes('timeout')) returns ErrorType.TIMEOUT.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

ENOENT detection may miss file-not-found errors.

Node.js filesystem errors set error.code = 'ENOENT', but this code only checks the message. Consider also checking the code variable.

🛠️ Proposed fix

   // Config errors
-  if (message.includes('config.json not found') || message.includes('enoent')) {
+  if (message.includes('config.json not found') || message.includes('enoent') || code === 'ENOENT') {
     return ErrorType.CONFIG_MISSING;
   }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Config errors
	if (message.includes('config.json not found') || message.includes('enoent')) {
	return ErrorType.CONFIG_MISSING;
	}
	// Config errors
	if (message.includes('config.json not found') || message.includes('enoent') || code === 'ENOENT') {
	return ErrorType.CONFIG_MISSING;
	}

🤖 Prompt for AI Agents

In `@src/utils/errors.js` around lines 91 - 94, The current detection only
inspects the error message string (`message`) to map to
ErrorType.CONFIG_MISSING; update the logic in the function in
src/utils/errors.js to also check the error object's `code` property (e.g.,
`code === 'ENOENT'`) and handle cases where the input may be an Error object
rather than a string, so that when either message includes 'config.json not
found' or `code === 'ENOENT'` you return ErrorType.CONFIG_MISSING; reference the
existing `message` variable and the `ErrorType.CONFIG_MISSING` symbol when
making this change.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider moving the messages object outside the function.

The messages object is recreated on every call. Moving it to module scope would avoid repeated object creation, though the impact is minor.

♻️ Proposed refactor

+const USER_FRIENDLY_MESSAGES = {
+  [ErrorType.NETWORK]: "I'm having trouble connecting to my brain right now. Check if the AI service is running and try again!",
+  [ErrorType.TIMEOUT]: "That took too long to process. Try again with a shorter message, or wait a moment and retry!",
+  // ... remaining messages
+  [ErrorType.UNKNOWN]: "Something unexpected happened. Try again, and if it keeps happening, check the logs for details.",
+};
+
 export function getUserFriendlyMessage(error, context = {}) {
   const errorType = classifyError(error, context);
-  const messages = {
-    // ... all messages
-  };
-  return messages[errorType] || messages[ErrorType.UNKNOWN];
+  return USER_FRIENDLY_MESSAGES[errorType] || USER_FRIENDLY_MESSAGES[ErrorType.UNKNOWN];
 }

🤖 Prompt for AI Agents

In `@src/utils/errors.js` around lines 114 - 146, The messages map inside
getUserFriendlyMessage is recreated on every call; move the messages object to
module scope (e.g., const MESSAGES = { ... }) and have getUserFriendlyMessage
reference MESSAGES[errorType]. Update references to ErrorType keys accordingly
and keep the default fallback to MESSAGES[ErrorType.UNKNOWN] so behavior is
unchanged while avoiding repeated allocations.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider adding jitter to prevent thundering herd.

Pure exponential backoff without jitter can cause synchronized retry storms when multiple operations fail simultaneously. Adding randomized jitter spreads out retries.

♻️ Proposed fix with jitter

 function calculateBackoff(attempt, baseDelay, maxDelay) {
   // Exponential backoff: baseDelay * 2^attempt
   const delay = baseDelay * Math.pow(2, attempt);
 
-  // Cap at maxDelay
-  return Math.min(delay, maxDelay);
+  // Cap at maxDelay and add jitter (±25%)
+  const cappedDelay = Math.min(delay, maxDelay);
+  const jitter = cappedDelay * 0.25 * (Math.random() * 2 - 1);
+  return Math.max(0, Math.floor(cappedDelay + jitter));
 }

🤖 Prompt for AI Agents

In `@src/utils/retry.js` around lines 27 - 33, The calculateBackoff function uses
pure exponential backoff which risks synchronized retries; modify
calculateBackoff(attempt, baseDelay, maxDelay) to add randomized jitter to the
computed delay (for example by applying a random factor or selecting a random
value within a range around the exponential value) and still cap the result at
maxDelay: compute the exponential delay as now, apply jitter via Math.random to
spread retries, then return Math.min(jitteredDelay, maxDelay). Ensure the jitter
keeps delays non-negative and documented in the function comment.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before Autofix could start.}

[cursor]

ETIMEDOUT classified as NETWORK instead of TIMEOUT

Medium Severity

The error code ETIMEDOUT is checked in the network errors condition (line 51) before the timeout condition (line 54). When an error has code === 'ETIMEDOUT', it returns ErrorType.NETWORK instead of ErrorType.TIMEOUT. The second check for ETIMEDOUT on line 54 becomes unreachable for that code path. This causes timeout errors to show network-related user messages instead of timeout-specific guidance.