-
Notifications
You must be signed in to change notification settings - Fork 2
Enhanced Error Handling #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,204 @@ | ||||||||||||||||||
| /** | ||||||||||||||||||
| * Error Classification and User-Friendly Messages | ||||||||||||||||||
| * | ||||||||||||||||||
| * Provides utilities for classifying errors and generating | ||||||||||||||||||
| * helpful error messages for users. | ||||||||||||||||||
| */ | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Error type classifications | ||||||||||||||||||
| */ | ||||||||||||||||||
| export const ErrorType = { | ||||||||||||||||||
| // Network-related errors | ||||||||||||||||||
| NETWORK: 'network', | ||||||||||||||||||
| TIMEOUT: 'timeout', | ||||||||||||||||||
|
|
||||||||||||||||||
| // API errors | ||||||||||||||||||
| API_ERROR: 'api_error', | ||||||||||||||||||
| API_RATE_LIMIT: 'api_rate_limit', | ||||||||||||||||||
| API_UNAUTHORIZED: 'api_unauthorized', | ||||||||||||||||||
| API_NOT_FOUND: 'api_not_found', | ||||||||||||||||||
| API_SERVER_ERROR: 'api_server_error', | ||||||||||||||||||
|
|
||||||||||||||||||
| // Discord-specific errors | ||||||||||||||||||
| DISCORD_PERMISSION: 'discord_permission', | ||||||||||||||||||
| DISCORD_CHANNEL_NOT_FOUND: 'discord_channel_not_found', | ||||||||||||||||||
| DISCORD_MISSING_ACCESS: 'discord_missing_access', | ||||||||||||||||||
|
|
||||||||||||||||||
| // Configuration errors | ||||||||||||||||||
| CONFIG_MISSING: 'config_missing', | ||||||||||||||||||
| CONFIG_INVALID: 'config_invalid', | ||||||||||||||||||
|
|
||||||||||||||||||
| // Unknown/generic errors | ||||||||||||||||||
| UNKNOWN: 'unknown', | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Classify an error into a specific error type | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param {Error} error - The error to classify | ||||||||||||||||||
| * @param {Object} context - Optional context (response, statusCode, etc.) | ||||||||||||||||||
| * @returns {string} Error type from ErrorType enum | ||||||||||||||||||
| */ | ||||||||||||||||||
| export function classifyError(error, context = {}) { | ||||||||||||||||||
| if (!error) return ErrorType.UNKNOWN; | ||||||||||||||||||
|
|
||||||||||||||||||
| const message = error.message?.toLowerCase() || ''; | ||||||||||||||||||
| const code = error.code || context.code; | ||||||||||||||||||
| const status = error.status || context.status || context.statusCode; | ||||||||||||||||||
|
|
||||||||||||||||||
| // Network errors | ||||||||||||||||||
| if (code === 'ECONNREFUSED' || code === 'ENOTFOUND' || code === 'ETIMEDOUT') { | ||||||||||||||||||
| return ErrorType.NETWORK; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (code === 'ETIMEDOUT' || message.includes('timeout')) { | ||||||||||||||||||
| return ErrorType.TIMEOUT; | ||||||||||||||||||
| } | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ETIMEDOUT classified as NETWORK instead of TIMEOUTMedium Severity The error code |
||||||||||||||||||
| if (message.includes('fetch failed') || message.includes('network')) { | ||||||||||||||||||
| return ErrorType.NETWORK; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // HTTP status code errors | ||||||||||||||||||
| if (status) { | ||||||||||||||||||
| if (status === 401 || status === 403) { | ||||||||||||||||||
| return ErrorType.API_UNAUTHORIZED; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (status === 404) { | ||||||||||||||||||
| return ErrorType.API_NOT_FOUND; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (status === 429) { | ||||||||||||||||||
| return ErrorType.API_RATE_LIMIT; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (status >= 500) { | ||||||||||||||||||
| return ErrorType.API_SERVER_ERROR; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (status >= 400) { | ||||||||||||||||||
| return ErrorType.API_ERROR; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Discord-specific errors | ||||||||||||||||||
| if (code === 50001 || message.includes('missing access')) { | ||||||||||||||||||
| return ErrorType.DISCORD_MISSING_ACCESS; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (code === 50013 || message.includes('missing permissions')) { | ||||||||||||||||||
| return ErrorType.DISCORD_PERMISSION; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (code === 10003 || message.includes('unknown channel')) { | ||||||||||||||||||
| return ErrorType.DISCORD_CHANNEL_NOT_FOUND; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Config errors | ||||||||||||||||||
| if (message.includes('config.json not found') || message.includes('enoent')) { | ||||||||||||||||||
| return ErrorType.CONFIG_MISSING; | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+91
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Node.js filesystem errors set 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| if (message.includes('invalid') && message.includes('config')) { | ||||||||||||||||||
| return ErrorType.CONFIG_INVALID; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // API errors (generic) | ||||||||||||||||||
| if (message.includes('api error') || context.isApiError) { | ||||||||||||||||||
| return ErrorType.API_ERROR; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return ErrorType.UNKNOWN; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Get a user-friendly error message based on error type | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param {Error} error - The error object | ||||||||||||||||||
| * @param {Object} context - Optional context for more specific messages | ||||||||||||||||||
| * @returns {string} User-friendly error message | ||||||||||||||||||
| */ | ||||||||||||||||||
| export function getUserFriendlyMessage(error, context = {}) { | ||||||||||||||||||
| const errorType = classifyError(error, context); | ||||||||||||||||||
|
|
||||||||||||||||||
| const 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!", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_RATE_LIMIT]: "Whoa, too many requests! Let's take a quick breather. Try again in a minute.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_UNAUTHORIZED]: "I'm having authentication issues with the AI service. An admin needs to check the API credentials.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_NOT_FOUND]: "The AI service endpoint isn't responding. Please check if it's configured correctly.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_SERVER_ERROR]: "The AI service is having technical difficulties. It should recover automatically - try again in a moment!", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_ERROR]: "Something went wrong with the AI service. Give it another shot in a moment!", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.DISCORD_PERMISSION]: "I don't have permission to do that! An admin needs to check my role permissions.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.DISCORD_CHANNEL_NOT_FOUND]: "I can't find that channel. It might have been deleted, or I don't have access to it.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.DISCORD_MISSING_ACCESS]: "I don't have access to that resource. Please check my permissions!", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.CONFIG_MISSING]: "Configuration file not found! Please create a config.json file (you can copy from config.example.json).", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.CONFIG_INVALID]: "The configuration file has errors. Please check config.json for syntax errors or missing required fields.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.UNKNOWN]: "Something unexpected happened. Try again, and if it keeps happening, check the logs for details.", | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| return messages[errorType] || messages[ErrorType.UNKNOWN]; | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+114
to
+146
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider moving the The ♻️ 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 |
||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Get suggested next steps for an error | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param {Error} error - The error object | ||||||||||||||||||
| * @param {Object} context - Optional context | ||||||||||||||||||
| * @returns {string|null} Suggested next steps or null if none | ||||||||||||||||||
| */ | ||||||||||||||||||
| export function getSuggestedNextSteps(error, context = {}) { | ||||||||||||||||||
| const errorType = classifyError(error, context); | ||||||||||||||||||
|
|
||||||||||||||||||
| const suggestions = { | ||||||||||||||||||
| [ErrorType.NETWORK]: "Make sure the AI service (OpenClaw) is running and accessible.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.TIMEOUT]: "Try a shorter message or wait a moment before retrying.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_RATE_LIMIT]: "Wait 60 seconds before trying again.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_UNAUTHORIZED]: "Check the OPENCLAW_TOKEN environment variable and API credentials.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_NOT_FOUND]: "Verify the OPENCLAW_URL environment variable points to the correct endpoint.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.API_SERVER_ERROR]: "The service should recover automatically. If it persists, restart the AI service.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.DISCORD_PERMISSION]: "Grant the bot appropriate permissions in Server Settings > Roles.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.DISCORD_CHANNEL_NOT_FOUND]: "Update the channel ID in config.json or verify the channel exists.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.DISCORD_MISSING_ACCESS]: "Ensure the bot has access to the required channels and roles.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.CONFIG_MISSING]: "Create config.json from config.example.json and fill in your settings.", | ||||||||||||||||||
|
|
||||||||||||||||||
| [ErrorType.CONFIG_INVALID]: "Validate your config.json syntax using a JSON validator.", | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| return suggestions[errorType] || null; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Check if an error is retryable (transient failure) | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param {Error} error - The error to check | ||||||||||||||||||
| * @param {Object} context - Optional context | ||||||||||||||||||
| * @returns {boolean} True if the error should be retried | ||||||||||||||||||
| */ | ||||||||||||||||||
| export function isRetryable(error, context = {}) { | ||||||||||||||||||
| const errorType = classifyError(error, context); | ||||||||||||||||||
|
|
||||||||||||||||||
| // Only retry transient failures, not user/config errors | ||||||||||||||||||
| const retryableTypes = [ | ||||||||||||||||||
| ErrorType.NETWORK, | ||||||||||||||||||
| ErrorType.TIMEOUT, | ||||||||||||||||||
| ErrorType.API_SERVER_ERROR, | ||||||||||||||||||
| ErrorType.API_RATE_LIMIT, | ||||||||||||||||||
| ]; | ||||||||||||||||||
|
|
||||||||||||||||||
| return retryableTypes.includes(errorType); | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| /** | ||
| * Retry Utility with Exponential Backoff | ||
| * | ||
| * Provides utilities for retrying operations with configurable | ||
| * exponential backoff and integration with error classification. | ||
| */ | ||
|
|
||
| import { isRetryable, classifyError } from './errors.js'; | ||
| import { warn, error, debug } from '../logger.js'; | ||
|
|
||
| /** | ||
| * Sleep for a specified duration | ||
| * @param {number} ms - Milliseconds to sleep | ||
| * @returns {Promise<void>} | ||
| */ | ||
| function sleep(ms) { | ||
| return new Promise(resolve => setTimeout(resolve, ms)); | ||
| } | ||
|
|
||
| /** | ||
| * Calculate delay with exponential backoff | ||
| * @param {number} attempt - Current attempt number (0-indexed) | ||
| * @param {number} baseDelay - Base delay in milliseconds | ||
| * @param {number} maxDelay - Maximum delay in milliseconds | ||
| * @returns {number} Delay in milliseconds | ||
| */ | ||
| 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); | ||
| } | ||
|
Comment on lines
+27
to
+33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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 |
||
|
|
||
| /** | ||
| * Retry an async operation with exponential backoff | ||
| * | ||
| * @param {Function} fn - Async function to retry | ||
| * @param {Object} options - Retry configuration options | ||
| * @param {number} options.maxRetries - Maximum number of retry attempts (default: 3) | ||
| * @param {number} options.baseDelay - Initial delay in milliseconds (default: 1000) | ||
| * @param {number} options.maxDelay - Maximum delay in milliseconds (default: 30000) | ||
| * @param {Function} options.shouldRetry - Custom function to determine if error is retryable | ||
| * @param {Object} options.context - Optional context for logging | ||
| * @returns {Promise<any>} Result of the function | ||
| * @throws {Error} Throws the last error if all retries fail | ||
| */ | ||
| export async function withRetry(fn, options = {}) { | ||
| const { | ||
| maxRetries = 3, | ||
| baseDelay = 1000, | ||
| maxDelay = 30000, | ||
| shouldRetry = isRetryable, | ||
| context = {}, | ||
| } = options; | ||
|
|
||
| let lastError; | ||
|
|
||
| for (let attempt = 0; attempt <= maxRetries; attempt++) { | ||
| try { | ||
| // Execute the function | ||
| return await fn(); | ||
| } catch (err) { | ||
| lastError = err; | ||
|
|
||
| // Check if we should retry | ||
| const errorType = classifyError(err, context); | ||
| const canRetry = shouldRetry(err, context); | ||
|
|
||
| // Log the error | ||
| if (attempt === 0) { | ||
| warn(`Operation failed: ${err.message}`, { | ||
| ...context, | ||
| errorType, | ||
| attempt: attempt + 1, | ||
| maxRetries: maxRetries + 1, | ||
| }); | ||
| } | ||
|
|
||
| // If this was the last attempt or error is not retryable, throw | ||
| if (attempt >= maxRetries || !canRetry) { | ||
| if (!canRetry) { | ||
| error('Operation failed with non-retryable error', { | ||
| ...context, | ||
| errorType, | ||
| attempt: attempt + 1, | ||
| error: err.message, | ||
| }); | ||
| } else { | ||
| error('Operation failed after all retries', { | ||
| ...context, | ||
| errorType, | ||
| totalAttempts: attempt + 1, | ||
| error: err.message, | ||
| }); | ||
| } | ||
| throw err; | ||
| } | ||
|
|
||
| // Calculate backoff delay | ||
| const delay = calculateBackoff(attempt, baseDelay, maxDelay); | ||
|
|
||
| debug(`Retrying in ${delay}ms`, { | ||
| ...context, | ||
| attempt: attempt + 1, | ||
| maxRetries: maxRetries + 1, | ||
| delay, | ||
| errorType, | ||
| }); | ||
|
|
||
| // Wait before retrying | ||
| await sleep(delay); | ||
| } | ||
| } | ||
|
|
||
| // Should never reach here, but just in case | ||
| throw lastError; | ||
| } | ||
|
|
||
| /** | ||
| * Create a retry wrapper with pre-configured options | ||
| * | ||
| * @param {Object} defaultOptions - Default retry options | ||
| * @returns {Function} Configured retry function | ||
| */ | ||
| export function createRetryWrapper(defaultOptions = {}) { | ||
| return (fn, options = {}) => { | ||
| return withRetry(fn, { ...defaultOptions, ...options }); | ||
| }; | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ETIMEDOUTclassification is unreachable forTIMEOUT.ETIMEDOUTis checked at line 51 and returnsNETWORK, so theETIMEDOUTcheck at line 54 forTIMEOUTwill never execute. SinceETIMEDOUTspecifically indicates a connection timeout, it should likely returnTIMEOUT, notNETWORK.🐛 Proposed fix
🤖 Prompt for AI Agents