From 8f995fc59681344629403e95e8de57b14f31aa50 Mon Sep 17 00:00:00 2001 From: imfozilbek Date: Mon, 1 Dec 2025 15:50:30 +0500 Subject: [PATCH] feat(ipuaro): add error handling matrix and ErrorHandler service Implemented comprehensive error handling system according to v0.16.0 roadmap: - ERROR_MATRIX with 9 error types (redis, parse, llm, file, command, conflict, validation, timeout, unknown) - Enhanced IpuaroError with options, defaultOption, context properties - New methods: getMeta(), hasOption(), toDisplayString() - ErrorHandler service with handle(), wrap(), withRetry() methods - Utility functions: getErrorOptions(), isRecoverableError(), toIpuaroError() - 59 new tests (27 for IpuaroError, 32 for ErrorHandler) - Coverage maintained at 97.59% Breaking changes: - IpuaroError constructor signature changed to (type, message, options?) - ErrorChoice deprecated in favor of ErrorOption --- packages/ipuaro/CHANGELOG.md | 59 ++++ packages/ipuaro/ROADMAP.md | 66 +++- .../application/use-cases/HandleMessage.ts | 5 +- .../src/infrastructure/tools/registry.ts | 7 +- .../ipuaro/src/shared/errors/ErrorHandler.ts | 295 ++++++++++++++++ .../ipuaro/src/shared/errors/IpuaroError.ts | 214 ++++++++++-- packages/ipuaro/src/shared/errors/index.ts | 1 + packages/ipuaro/src/shared/types/index.ts | 4 + packages/ipuaro/src/tui/hooks/useSession.ts | 4 +- .../unit/shared/errors/ErrorHandler.test.ts | 327 ++++++++++++++++++ .../unit/shared/errors/IpuaroError.test.ts | 177 +++++++++- 11 files changed, 1089 insertions(+), 70 deletions(-) create mode 100644 packages/ipuaro/src/shared/errors/ErrorHandler.ts create mode 100644 packages/ipuaro/tests/unit/shared/errors/ErrorHandler.test.ts diff --git a/packages/ipuaro/CHANGELOG.md b/packages/ipuaro/CHANGELOG.md index 98ba515..4bf524f 100644 --- a/packages/ipuaro/CHANGELOG.md +++ b/packages/ipuaro/CHANGELOG.md @@ -5,6 +5,65 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.16.0] - 2025-12-01 - Error Handling + +### Added + +- **Error Handling Matrix (0.16.2)** + - `ERROR_MATRIX`: Defines behavior for each error type + - Per-type options: retry, skip, abort, confirm, regenerate + - Per-type defaults and recoverability settings + - Comprehensive error type support: redis, parse, llm, file, command, conflict, validation, timeout, unknown + +- **IpuaroError Enhancements (0.16.1)** + - `ErrorOption` type: New type for available recovery options + - `ErrorMeta` interface: Error metadata with type, recoverable flag, options, and default + - `options` property: Available recovery options from matrix + - `defaultOption` property: Default option for the error type + - `context` property: Optional context data for debugging + - `getMeta()`: Returns full error metadata + - `hasOption()`: Checks if an option is available + - `toDisplayString()`: Formatted error message with suggestion + - New factory methods: `llmTimeout()`, `fileNotFound()`, `commandBlacklisted()`, `unknown()` + +- **ErrorHandler Service** + - `handle()`: Async error handling with user callback + - `handleSync()`: Sync error handling with defaults + - `wrap()`: Wraps async functions with error handling + - `withRetry()`: Wraps functions with automatic retry logic + - `resetRetries()`: Resets retry counters + - `getRetryCount()`: Gets current retry count + - `isMaxRetriesExceeded()`: Checks if max retries reached + - Configurable options: maxRetries, autoSkipParseErrors, autoRetryLLMErrors + +- **Utility Functions** + - `getErrorOptions()`: Get available options for error type + - `getDefaultErrorOption()`: Get default option for error type + - `isRecoverableError()`: Check if error type is recoverable + - `toIpuaroError()`: Convert any error to IpuaroError + - `createErrorHandler()`: Factory function for ErrorHandler + +### Changed + +- **IpuaroError Constructor** + - New signature: `(type, message, options?)` with options object + - Options include: recoverable, suggestion, context + - Matrix-based defaults for all properties + +- **ErrorChoice → ErrorOption** + - `ErrorChoice` type deprecated in shared/types + - Use `ErrorOption` from shared/errors instead + - Updated HandleMessage and useSession to use ErrorOption + +### Technical Details + +- Total tests: 1420 (59 new tests) +- Coverage: 97.59% maintained +- New test files: ErrorHandler.test.ts +- Updated test file: IpuaroError.test.ts + +--- + ## [0.15.0] - 2025-12-01 - CLI Entry Point ### Added diff --git a/packages/ipuaro/ROADMAP.md b/packages/ipuaro/ROADMAP.md index a238b7a..22c2d68 100644 --- a/packages/ipuaro/ROADMAP.md +++ b/packages/ipuaro/ROADMAP.md @@ -1223,37 +1223,71 @@ ipuaro index // Index only (no TUI) --- -## Version 0.16.0 - Error Handling ⚠️ ⬜ +## Version 0.16.0 - Error Handling ⚠️ ✅ **Priority:** HIGH -**Status:** NEXT MILESTONE — IpuaroError exists (v0.1.0), need full error matrix implementation +**Status:** Complete (v0.16.0 released) -### 0.16.1 - Error Types +### 0.16.1 - Error Types ✅ ```typescript // src/shared/errors/IpuaroError.ts -type ErrorType = "redis" | "parse" | "llm" | "file" | "command" | "conflict" +type ErrorType = "redis" | "parse" | "llm" | "file" | "command" | "conflict" | "validation" | "timeout" | "unknown" +type ErrorOption = "retry" | "skip" | "abort" | "confirm" | "regenerate" + +interface ErrorMeta { + type: ErrorType + recoverable: boolean + options: ErrorOption[] + defaultOption: ErrorOption +} class IpuaroError extends Error { type: ErrorType recoverable: boolean suggestion?: string + options: ErrorOption[] + defaultOption: ErrorOption + context?: Record + + getMeta(): ErrorMeta + hasOption(option: ErrorOption): boolean + toDisplayString(): string } ``` -### 0.16.2 - Error Handling Matrix +### 0.16.2 - Error Handling Matrix ✅ -| Error | Recoverable | Options | -|-------|-------------|---------| -| Redis unavailable | No | Retry / Abort | -| AST parse failed | Yes | Skip file / Abort | -| LLM timeout | Yes | Retry / Skip / Abort | -| File not found | Yes | Skip / Abort | -| Command not in whitelist | Yes | Confirm / Skip / Abort | -| Edit conflict | Yes | Apply / Skip / Regenerate | +| Error | Recoverable | Options | Default | +|-------|-------------|---------|---------| +| Redis unavailable | No | Retry / Abort | Abort | +| AST parse failed | Yes | Skip / Abort | Skip | +| LLM timeout | Yes | Retry / Skip / Abort | Retry | +| File not found | Yes | Skip / Abort | Skip | +| Command not in whitelist | Yes | Confirm / Skip / Abort | Confirm | +| Edit conflict | Yes | Skip / Regenerate / Abort | Skip | +| Validation error | Yes | Skip / Abort | Skip | +| Timeout | Yes | Retry / Skip / Abort | Retry | +| Unknown | No | Abort | Abort | + +### 0.16.3 - ErrorHandler Service ✅ + +```typescript +// src/shared/errors/ErrorHandler.ts +class ErrorHandler { + handle(error: IpuaroError, contextKey?: string): Promise + handleSync(error: IpuaroError, contextKey?: string): ErrorHandlingResult + wrap(fn: () => Promise, errorType: ErrorType, contextKey?: string): Promise + withRetry(fn: () => Promise, errorType: ErrorType, contextKey: string): Promise + resetRetries(contextKey?: string): void + getRetryCount(contextKey: string): number + isMaxRetriesExceeded(contextKey: string): boolean +} +``` **Tests:** -- [ ] Unit tests for error handling +- [x] Unit tests for IpuaroError (27 tests) +- [x] Unit tests for ErrorHandler (32 tests) --- @@ -1265,7 +1299,7 @@ class IpuaroError extends Error { - [x] All 18 tools implemented and tested ✅ (v0.9.0) - [x] TUI fully functional ✅ (v0.11.0, v0.12.0) - [x] Session persistence working ✅ (v0.10.0) -- [ ] Error handling complete (partial) +- [x] Error handling complete ✅ (v0.16.0) - [ ] Performance optimized - [ ] Documentation complete - [x] 80%+ test coverage ✅ (~98%) @@ -1347,4 +1381,4 @@ sessions:list # List **Last Updated:** 2025-12-01 **Target Version:** 1.0.0 -**Current Version:** 0.15.0 \ No newline at end of file +**Current Version:** 0.16.0 \ No newline at end of file diff --git a/packages/ipuaro/src/application/use-cases/HandleMessage.ts b/packages/ipuaro/src/application/use-cases/HandleMessage.ts index b611bb0..b96b11e 100644 --- a/packages/ipuaro/src/application/use-cases/HandleMessage.ts +++ b/packages/ipuaro/src/application/use-cases/HandleMessage.ts @@ -14,8 +14,7 @@ import { import type { ToolCall } from "../../domain/value-objects/ToolCall.js" import { createErrorResult, type ToolResult } from "../../domain/value-objects/ToolResult.js" import { createUndoEntry, type UndoEntry } from "../../domain/value-objects/UndoEntry.js" -import { IpuaroError } from "../../shared/errors/IpuaroError.js" -import type { ErrorChoice } from "../../shared/types/index.js" +import { type ErrorOption, IpuaroError } from "../../shared/errors/IpuaroError.js" import { buildInitialContext, type ProjectStructure, @@ -58,7 +57,7 @@ export interface HandleMessageEvents { onToolCall?: (call: ToolCall) => void onToolResult?: (result: ToolResult) => void onConfirmation?: (message: string, diff?: DiffInfo) => Promise - onError?: (error: IpuaroError) => Promise + onError?: (error: IpuaroError) => Promise onStatusChange?: (status: HandleMessageStatus) => void onUndoEntry?: (entry: UndoEntry) => void } diff --git a/packages/ipuaro/src/infrastructure/tools/registry.ts b/packages/ipuaro/src/infrastructure/tools/registry.ts index 73ff3e4..a817658 100644 --- a/packages/ipuaro/src/infrastructure/tools/registry.ts +++ b/packages/ipuaro/src/infrastructure/tools/registry.ts @@ -16,12 +16,7 @@ export class ToolRegistry implements IToolRegistry { */ register(tool: ITool): void { if (this.tools.has(tool.name)) { - throw new IpuaroError( - "validation", - `Tool "${tool.name}" is already registered`, - true, - "Use a different tool name or unregister the existing tool first", - ) + throw IpuaroError.validation(`Tool "${tool.name}" is already registered`) } this.tools.set(tool.name, tool) } diff --git a/packages/ipuaro/src/shared/errors/ErrorHandler.ts b/packages/ipuaro/src/shared/errors/ErrorHandler.ts new file mode 100644 index 0000000..922d4d6 --- /dev/null +++ b/packages/ipuaro/src/shared/errors/ErrorHandler.ts @@ -0,0 +1,295 @@ +/** + * ErrorHandler service for handling errors with user interaction. + * Implements the error handling matrix from ROADMAP.md. + */ + +import { ERROR_MATRIX, type ErrorOption, type ErrorType, IpuaroError } from "./IpuaroError.js" + +/** + * Result of error handling. + */ +export interface ErrorHandlingResult { + action: ErrorOption + shouldContinue: boolean + retryCount?: number +} + +/** + * Callback for requesting user choice on error. + */ +export type ErrorChoiceCallback = ( + error: IpuaroError, + availableOptions: ErrorOption[], + defaultOption: ErrorOption, +) => Promise + +/** + * Options for ErrorHandler. + */ +export interface ErrorHandlerOptions { + maxRetries?: number + autoSkipParseErrors?: boolean + autoRetryLLMErrors?: boolean + onError?: ErrorChoiceCallback +} + +const DEFAULT_MAX_RETRIES = 3 + +/** + * Error handler service with matrix-based logic. + */ +export class ErrorHandler { + private readonly maxRetries: number + private readonly autoSkipParseErrors: boolean + private readonly autoRetryLLMErrors: boolean + private readonly onError?: ErrorChoiceCallback + + private readonly retryCounters = new Map() + + constructor(options: ErrorHandlerOptions = {}) { + this.maxRetries = options.maxRetries ?? DEFAULT_MAX_RETRIES + this.autoSkipParseErrors = options.autoSkipParseErrors ?? true + this.autoRetryLLMErrors = options.autoRetryLLMErrors ?? false + this.onError = options.onError + } + + /** + * Handle an error and determine the action to take. + */ + async handle(error: IpuaroError, contextKey?: string): Promise { + const key = contextKey ?? error.message + const currentRetries = this.retryCounters.get(key) ?? 0 + + if (this.shouldAutoHandle(error)) { + const autoAction = this.getAutoAction(error, currentRetries) + if (autoAction) { + return this.createResult(autoAction, key, currentRetries) + } + } + + if (!error.recoverable) { + return { + action: "abort", + shouldContinue: false, + } + } + + if (this.onError) { + const choice = await this.onError(error, error.options, error.defaultOption) + return this.createResult(choice, key, currentRetries) + } + + return this.createResult(error.defaultOption, key, currentRetries) + } + + /** + * Handle an error synchronously with default behavior. + */ + handleSync(error: IpuaroError, contextKey?: string): ErrorHandlingResult { + const key = contextKey ?? error.message + const currentRetries = this.retryCounters.get(key) ?? 0 + + if (this.shouldAutoHandle(error)) { + const autoAction = this.getAutoAction(error, currentRetries) + if (autoAction) { + return this.createResult(autoAction, key, currentRetries) + } + } + + if (!error.recoverable) { + return { + action: "abort", + shouldContinue: false, + } + } + + return this.createResult(error.defaultOption, key, currentRetries) + } + + /** + * Reset retry counters. + */ + resetRetries(contextKey?: string): void { + if (contextKey) { + this.retryCounters.delete(contextKey) + } else { + this.retryCounters.clear() + } + } + + /** + * Get retry count for a context. + */ + getRetryCount(contextKey: string): number { + return this.retryCounters.get(contextKey) ?? 0 + } + + /** + * Check if max retries exceeded for a context. + */ + isMaxRetriesExceeded(contextKey: string): boolean { + return this.getRetryCount(contextKey) >= this.maxRetries + } + + /** + * Wrap a function with error handling. + */ + async wrap( + fn: () => Promise, + errorType: ErrorType, + contextKey?: string, + ): Promise<{ success: true; data: T } | { success: false; result: ErrorHandlingResult }> { + try { + const data = await fn() + if (contextKey) { + this.resetRetries(contextKey) + } + return { success: true, data } + } catch (err) { + const error = + err instanceof IpuaroError + ? err + : new IpuaroError(errorType, err instanceof Error ? err.message : String(err)) + + const result = await this.handle(error, contextKey) + return { success: false, result } + } + } + + /** + * Wrap a function with retry logic. + */ + async withRetry(fn: () => Promise, errorType: ErrorType, contextKey: string): Promise { + const key = contextKey + + while (!this.isMaxRetriesExceeded(key)) { + try { + const result = await fn() + this.resetRetries(key) + return result + } catch (err) { + const error = + err instanceof IpuaroError + ? err + : new IpuaroError( + errorType, + err instanceof Error ? err.message : String(err), + ) + + const handlingResult = await this.handle(error, key) + + if (handlingResult.action !== "retry" || !handlingResult.shouldContinue) { + throw error + } + } + } + + throw new IpuaroError( + errorType, + `Max retries (${String(this.maxRetries)}) exceeded for: ${key}`, + ) + } + + private shouldAutoHandle(error: IpuaroError): boolean { + if (error.type === "parse" && this.autoSkipParseErrors) { + return true + } + if ((error.type === "llm" || error.type === "timeout") && this.autoRetryLLMErrors) { + return true + } + return false + } + + private getAutoAction(error: IpuaroError, currentRetries: number): ErrorOption | null { + if (error.type === "parse" && this.autoSkipParseErrors) { + return "skip" + } + + if ((error.type === "llm" || error.type === "timeout") && this.autoRetryLLMErrors) { + if (currentRetries < this.maxRetries) { + return "retry" + } + return "abort" + } + + return null + } + + private createResult( + action: ErrorOption, + key: string, + currentRetries: number, + ): ErrorHandlingResult { + if (action === "retry") { + this.retryCounters.set(key, currentRetries + 1) + const newRetryCount = currentRetries + 1 + + if (newRetryCount > this.maxRetries) { + return { + action: "abort", + shouldContinue: false, + retryCount: newRetryCount, + } + } + + return { + action: "retry", + shouldContinue: true, + retryCount: newRetryCount, + } + } + + this.retryCounters.delete(key) + + return { + action, + shouldContinue: action === "skip" || action === "confirm" || action === "regenerate", + retryCount: currentRetries, + } + } +} + +/** + * Get available options for an error type. + */ +export function getErrorOptions(errorType: ErrorType): ErrorOption[] { + return ERROR_MATRIX[errorType].options +} + +/** + * Get default option for an error type. + */ +export function getDefaultErrorOption(errorType: ErrorType): ErrorOption { + return ERROR_MATRIX[errorType].defaultOption +} + +/** + * Check if an error type is recoverable by default. + */ +export function isRecoverableError(errorType: ErrorType): boolean { + return ERROR_MATRIX[errorType].recoverable +} + +/** + * Convert any error to IpuaroError. + */ +export function toIpuaroError(error: unknown, defaultType: ErrorType = "unknown"): IpuaroError { + if (error instanceof IpuaroError) { + return error + } + + if (error instanceof Error) { + return new IpuaroError(defaultType, error.message, { + context: { originalError: error.name }, + }) + } + + return new IpuaroError(defaultType, String(error)) +} + +/** + * Create a default ErrorHandler instance. + */ +export function createErrorHandler(options?: ErrorHandlerOptions): ErrorHandler { + return new ErrorHandler(options) +} diff --git a/packages/ipuaro/src/shared/errors/IpuaroError.ts b/packages/ipuaro/src/shared/errors/IpuaroError.ts index e4338f8..fe97f05 100644 --- a/packages/ipuaro/src/shared/errors/IpuaroError.ts +++ b/packages/ipuaro/src/shared/errors/IpuaroError.ts @@ -12,6 +12,72 @@ export type ErrorType = | "timeout" | "unknown" +/** + * Available options for error recovery. + */ +export type ErrorOption = "retry" | "skip" | "abort" | "confirm" | "regenerate" + +/** + * Error metadata with available options. + */ +export interface ErrorMeta { + type: ErrorType + recoverable: boolean + options: ErrorOption[] + defaultOption: ErrorOption +} + +/** + * Error handling matrix - defines behavior for each error type. + */ +export const ERROR_MATRIX: Record> = { + redis: { + recoverable: false, + options: ["retry", "abort"], + defaultOption: "abort", + }, + parse: { + recoverable: true, + options: ["skip", "abort"], + defaultOption: "skip", + }, + llm: { + recoverable: true, + options: ["retry", "skip", "abort"], + defaultOption: "retry", + }, + file: { + recoverable: true, + options: ["skip", "abort"], + defaultOption: "skip", + }, + command: { + recoverable: true, + options: ["confirm", "skip", "abort"], + defaultOption: "confirm", + }, + conflict: { + recoverable: true, + options: ["skip", "regenerate", "abort"], + defaultOption: "skip", + }, + validation: { + recoverable: true, + options: ["skip", "abort"], + defaultOption: "skip", + }, + timeout: { + recoverable: true, + options: ["retry", "skip", "abort"], + defaultOption: "retry", + }, + unknown: { + recoverable: false, + options: ["abort"], + defaultOption: "abort", + }, +} + /** * Base error class for ipuaro. */ @@ -19,60 +85,142 @@ export class IpuaroError extends Error { readonly type: ErrorType readonly recoverable: boolean readonly suggestion?: string + readonly options: ErrorOption[] + readonly defaultOption: ErrorOption + readonly context?: Record - constructor(type: ErrorType, message: string, recoverable = true, suggestion?: string) { + constructor( + type: ErrorType, + message: string, + options?: { + recoverable?: boolean + suggestion?: string + context?: Record + }, + ) { super(message) this.name = "IpuaroError" this.type = type - this.recoverable = recoverable - this.suggestion = suggestion + + const meta = ERROR_MATRIX[type] + this.recoverable = options?.recoverable ?? meta.recoverable + this.options = meta.options + this.defaultOption = meta.defaultOption + this.suggestion = options?.suggestion + this.context = options?.context } - static redis(message: string): IpuaroError { - return new IpuaroError( - "redis", - message, - false, - "Please ensure Redis is running: redis-server", - ) + /** + * Get error metadata. + */ + getMeta(): ErrorMeta { + return { + type: this.type, + recoverable: this.recoverable, + options: this.options, + defaultOption: this.defaultOption, + } + } + + /** + * Check if an option is available for this error. + */ + hasOption(option: ErrorOption): boolean { + return this.options.includes(option) + } + + /** + * Create a formatted error message with suggestion. + */ + toDisplayString(): string { + let result = `[${this.type}] ${this.message}` + if (this.suggestion) { + result += `\n Suggestion: ${this.suggestion}` + } + return result + } + + static redis(message: string, context?: Record): IpuaroError { + return new IpuaroError("redis", message, { + suggestion: "Please ensure Redis is running: redis-server", + context, + }) } static parse(message: string, filePath?: string): IpuaroError { const msg = filePath ? `${message} in ${filePath}` : message - return new IpuaroError("parse", msg, true, "File will be skipped") + return new IpuaroError("parse", msg, { + suggestion: "File will be skipped during indexing", + context: filePath ? { filePath } : undefined, + }) } - static llm(message: string): IpuaroError { - return new IpuaroError( - "llm", - message, - true, - "Please ensure Ollama is running and model is available", - ) + static llm(message: string, context?: Record): IpuaroError { + return new IpuaroError("llm", message, { + suggestion: "Please ensure Ollama is running and model is available", + context, + }) } - static file(message: string): IpuaroError { - return new IpuaroError("file", message, true) + static llmTimeout(message: string): IpuaroError { + return new IpuaroError("timeout", message, { + suggestion: "The LLM request timed out. Try again or check Ollama status.", + }) } - static command(message: string): IpuaroError { - return new IpuaroError("command", message, true) + static file(message: string, filePath?: string): IpuaroError { + return new IpuaroError("file", message, { + suggestion: "Check if the file exists and you have permission to access it", + context: filePath ? { filePath } : undefined, + }) } - static conflict(message: string): IpuaroError { - return new IpuaroError( - "conflict", - message, - true, - "File was modified externally. Regenerate or skip.", - ) + static fileNotFound(filePath: string): IpuaroError { + return new IpuaroError("file", `File not found: ${filePath}`, { + suggestion: "Check the file path and try again", + context: { filePath }, + }) } - static validation(message: string): IpuaroError { - return new IpuaroError("validation", message, true) + static command(message: string, command?: string): IpuaroError { + return new IpuaroError("command", message, { + suggestion: "Command requires confirmation or is not in whitelist", + context: command ? { command } : undefined, + }) } - static timeout(message: string): IpuaroError { - return new IpuaroError("timeout", message, true, "Try again or increase timeout") + static commandBlacklisted(command: string): IpuaroError { + return new IpuaroError("command", `Command is blacklisted: ${command}`, { + recoverable: false, + suggestion: "This command is not allowed for security reasons", + context: { command }, + }) + } + + static conflict(message: string, filePath?: string): IpuaroError { + return new IpuaroError("conflict", message, { + suggestion: "File was modified externally. Regenerate or skip the change.", + context: filePath ? { filePath } : undefined, + }) + } + + static validation(message: string, field?: string): IpuaroError { + return new IpuaroError("validation", message, { + suggestion: "Please check the input and try again", + context: field ? { field } : undefined, + }) + } + + static timeout(message: string, timeoutMs?: number): IpuaroError { + return new IpuaroError("timeout", message, { + suggestion: "Try again or increase the timeout value", + context: timeoutMs ? { timeoutMs } : undefined, + }) + } + + static unknown(message: string, originalError?: Error): IpuaroError { + return new IpuaroError("unknown", message, { + context: originalError ? { originalError: originalError.message } : undefined, + }) } } diff --git a/packages/ipuaro/src/shared/errors/index.ts b/packages/ipuaro/src/shared/errors/index.ts index 1f5bd90..92db90b 100644 --- a/packages/ipuaro/src/shared/errors/index.ts +++ b/packages/ipuaro/src/shared/errors/index.ts @@ -1,2 +1,3 @@ // Shared errors export * from "./IpuaroError.js" +export * from "./ErrorHandler.js" diff --git a/packages/ipuaro/src/shared/types/index.ts b/packages/ipuaro/src/shared/types/index.ts index def57d2..89534c2 100644 --- a/packages/ipuaro/src/shared/types/index.ts +++ b/packages/ipuaro/src/shared/types/index.ts @@ -19,9 +19,13 @@ export type ConfirmChoice = "apply" | "cancel" | "edit" /** * User choice for errors. + * @deprecated Use ErrorOption from shared/errors instead */ export type ErrorChoice = "retry" | "skip" | "abort" +// Re-export ErrorOption for convenience +export type { ErrorOption } from "../errors/IpuaroError.js" + /** * Project structure node. */ diff --git a/packages/ipuaro/src/tui/hooks/useSession.ts b/packages/ipuaro/src/tui/hooks/useSession.ts index d3b230e..a9b1301 100644 --- a/packages/ipuaro/src/tui/hooks/useSession.ts +++ b/packages/ipuaro/src/tui/hooks/useSession.ts @@ -10,7 +10,7 @@ import type { ISessionStorage } from "../../domain/services/ISessionStorage.js" import type { IStorage } from "../../domain/services/IStorage.js" import type { DiffInfo } from "../../domain/services/ITool.js" import type { ChatMessage } from "../../domain/value-objects/ChatMessage.js" -import type { ErrorChoice } from "../../shared/types/index.js" +import type { ErrorOption } from "../../shared/errors/IpuaroError.js" import type { IToolRegistry } from "../../application/interfaces/IToolRegistry.js" import { HandleMessage, @@ -34,7 +34,7 @@ export interface UseSessionDependencies { export interface UseSessionOptions { autoApply?: boolean onConfirmation?: (message: string, diff?: DiffInfo) => Promise - onError?: (error: Error) => Promise + onError?: (error: Error) => Promise } export interface UseSessionReturn { diff --git a/packages/ipuaro/tests/unit/shared/errors/ErrorHandler.test.ts b/packages/ipuaro/tests/unit/shared/errors/ErrorHandler.test.ts new file mode 100644 index 0000000..fe21d89 --- /dev/null +++ b/packages/ipuaro/tests/unit/shared/errors/ErrorHandler.test.ts @@ -0,0 +1,327 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { + createErrorHandler, + ErrorHandler, + getDefaultErrorOption, + getErrorOptions, + isRecoverableError, + toIpuaroError, +} from "../../../../src/shared/errors/ErrorHandler.js" +import { IpuaroError } from "../../../../src/shared/errors/IpuaroError.js" + +describe("ErrorHandler", () => { + let handler: ErrorHandler + + beforeEach(() => { + handler = new ErrorHandler() + }) + + describe("handle", () => { + it("should abort non-recoverable errors", async () => { + const error = IpuaroError.redis("Connection failed") + + const result = await handler.handle(error) + + expect(result.action).toBe("abort") + expect(result.shouldContinue).toBe(false) + }) + + it("should use default option for recoverable errors without callback", async () => { + const error = IpuaroError.llm("Timeout") + + const result = await handler.handle(error) + + expect(result.action).toBe("retry") + expect(result.shouldContinue).toBe(true) + }) + + it("should call onError callback when provided", async () => { + const onError = vi.fn().mockResolvedValue("skip") + const handler = new ErrorHandler({ onError }) + const error = IpuaroError.llm("Timeout") + + const result = await handler.handle(error) + + expect(onError).toHaveBeenCalledWith(error, error.options, error.defaultOption) + expect(result.action).toBe("skip") + }) + + it("should auto-skip parse errors when enabled", async () => { + const handler = new ErrorHandler({ autoSkipParseErrors: true }) + const error = IpuaroError.parse("Syntax error") + + const result = await handler.handle(error) + + expect(result.action).toBe("skip") + expect(result.shouldContinue).toBe(true) + }) + + it("should auto-retry LLM errors when enabled", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true }) + const error = IpuaroError.llm("Timeout") + + const result = await handler.handle(error, "test-key") + + expect(result.action).toBe("retry") + expect(result.shouldContinue).toBe(true) + expect(result.retryCount).toBe(1) + }) + + it("should track retry count", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true }) + const error = IpuaroError.llm("Timeout") + + await handler.handle(error, "test-key") + await handler.handle(error, "test-key") + const result = await handler.handle(error, "test-key") + + expect(result.retryCount).toBe(3) + }) + + it("should abort after max retries", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true, maxRetries: 2 }) + const error = IpuaroError.llm("Timeout") + + await handler.handle(error, "test-key") + await handler.handle(error, "test-key") + const result = await handler.handle(error, "test-key") + + expect(result.action).toBe("abort") + expect(result.shouldContinue).toBe(false) + }) + }) + + describe("handleSync", () => { + it("should abort non-recoverable errors", () => { + const error = IpuaroError.redis("Connection failed") + + const result = handler.handleSync(error) + + expect(result.action).toBe("abort") + expect(result.shouldContinue).toBe(false) + }) + + it("should use default option for recoverable errors", () => { + const error = IpuaroError.file("Not found") + + const result = handler.handleSync(error) + + expect(result.action).toBe("skip") + expect(result.shouldContinue).toBe(true) + }) + }) + + describe("resetRetries", () => { + it("should reset specific context", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true }) + const error = IpuaroError.llm("Timeout") + + await handler.handle(error, "key1") + await handler.handle(error, "key1") + handler.resetRetries("key1") + + expect(handler.getRetryCount("key1")).toBe(0) + }) + + it("should reset all contexts when no key provided", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true }) + const error = IpuaroError.llm("Timeout") + + await handler.handle(error, "key1") + await handler.handle(error, "key2") + handler.resetRetries() + + expect(handler.getRetryCount("key1")).toBe(0) + expect(handler.getRetryCount("key2")).toBe(0) + }) + }) + + describe("getRetryCount", () => { + it("should return 0 for unknown context", () => { + expect(handler.getRetryCount("unknown")).toBe(0) + }) + }) + + describe("isMaxRetriesExceeded", () => { + it("should return false when retries not exceeded", () => { + expect(handler.isMaxRetriesExceeded("test")).toBe(false) + }) + + it("should return true when retries exceeded", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true, maxRetries: 1 }) + const error = IpuaroError.llm("Timeout") + + await handler.handle(error, "test") + + expect(handler.isMaxRetriesExceeded("test")).toBe(true) + }) + }) + + describe("wrap", () => { + it("should return success result on success", async () => { + const fn = vi.fn().mockResolvedValue("result") + + const result = await handler.wrap(fn, "llm") + + expect(result.success).toBe(true) + if (result.success) { + expect(result.data).toBe("result") + } + }) + + it("should return error result on failure", async () => { + const fn = vi.fn().mockRejectedValue(new Error("Failed")) + + const result = await handler.wrap(fn, "llm") + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.result.action).toBe("retry") + } + }) + + it("should handle IpuaroError directly", async () => { + const fn = vi.fn().mockRejectedValue(IpuaroError.file("Not found")) + + const result = await handler.wrap(fn, "llm") + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.result.action).toBe("skip") + } + }) + + it("should reset retries on success", async () => { + const handler = new ErrorHandler({ autoRetryLLMErrors: true }) + const error = IpuaroError.llm("Timeout") + + await handler.handle(error, "test-key") + await handler.wrap(() => Promise.resolve("ok"), "llm", "test-key") + + expect(handler.getRetryCount("test-key")).toBe(0) + }) + }) + + describe("withRetry", () => { + it("should return result on success", async () => { + const fn = vi.fn().mockResolvedValue("result") + + const result = await handler.withRetry(fn, "llm", "test") + + expect(result).toBe("result") + }) + + it("should retry on failure", async () => { + const fn = vi + .fn() + .mockRejectedValueOnce(new Error("Fail 1")) + .mockResolvedValueOnce("success") + const handler = new ErrorHandler({ + onError: vi.fn().mockResolvedValue("retry"), + }) + + const result = await handler.withRetry(fn, "llm", "test") + + expect(result).toBe("success") + expect(fn).toHaveBeenCalledTimes(2) + }) + + it("should throw after max retries", async () => { + const fn = vi.fn().mockRejectedValue(new Error("Always fails")) + const handler = new ErrorHandler({ + maxRetries: 2, + onError: vi.fn().mockResolvedValue("retry"), + }) + + await expect(handler.withRetry(fn, "llm", "test")).rejects.toThrow("Max retries") + }) + + it("should throw immediately when skip is chosen", async () => { + const fn = vi.fn().mockRejectedValue(new Error("Fail")) + const handler = new ErrorHandler({ + onError: vi.fn().mockResolvedValue("skip"), + }) + + await expect(handler.withRetry(fn, "llm", "test")).rejects.toThrow("Fail") + }) + }) +}) + +describe("utility functions", () => { + describe("getErrorOptions", () => { + it("should return options for error type", () => { + const options = getErrorOptions("llm") + + expect(options).toEqual(["retry", "skip", "abort"]) + }) + }) + + describe("getDefaultErrorOption", () => { + it("should return default option for error type", () => { + expect(getDefaultErrorOption("llm")).toBe("retry") + expect(getDefaultErrorOption("parse")).toBe("skip") + expect(getDefaultErrorOption("redis")).toBe("abort") + }) + }) + + describe("isRecoverableError", () => { + it("should return true for recoverable errors", () => { + expect(isRecoverableError("llm")).toBe(true) + expect(isRecoverableError("parse")).toBe(true) + expect(isRecoverableError("file")).toBe(true) + }) + + it("should return false for non-recoverable errors", () => { + expect(isRecoverableError("redis")).toBe(false) + expect(isRecoverableError("unknown")).toBe(false) + }) + }) + + describe("toIpuaroError", () => { + it("should return IpuaroError as-is", () => { + const error = IpuaroError.llm("Timeout") + + const result = toIpuaroError(error) + + expect(result).toBe(error) + }) + + it("should convert Error to IpuaroError", () => { + const error = new Error("Something went wrong") + + const result = toIpuaroError(error, "llm") + + expect(result).toBeInstanceOf(IpuaroError) + expect(result.type).toBe("llm") + expect(result.message).toBe("Something went wrong") + }) + + it("should convert string to IpuaroError", () => { + const result = toIpuaroError("Error message", "file") + + expect(result).toBeInstanceOf(IpuaroError) + expect(result.type).toBe("file") + expect(result.message).toBe("Error message") + }) + + it("should use unknown type by default", () => { + const result = toIpuaroError("Error") + + expect(result.type).toBe("unknown") + }) + }) + + describe("createErrorHandler", () => { + it("should create handler with default options", () => { + const handler = createErrorHandler() + + expect(handler).toBeInstanceOf(ErrorHandler) + }) + + it("should create handler with custom options", () => { + const handler = createErrorHandler({ maxRetries: 5 }) + + expect(handler).toBeInstanceOf(ErrorHandler) + }) + }) +}) diff --git a/packages/ipuaro/tests/unit/shared/errors/IpuaroError.test.ts b/packages/ipuaro/tests/unit/shared/errors/IpuaroError.test.ts index 108c7e4..28b9021 100644 --- a/packages/ipuaro/tests/unit/shared/errors/IpuaroError.test.ts +++ b/packages/ipuaro/tests/unit/shared/errors/IpuaroError.test.ts @@ -1,22 +1,93 @@ import { describe, it, expect } from "vitest" -import { IpuaroError } from "../../../../src/shared/errors/IpuaroError.js" +import { ERROR_MATRIX, IpuaroError } from "../../../../src/shared/errors/IpuaroError.js" describe("IpuaroError", () => { describe("constructor", () => { it("should create error with all fields", () => { - const error = new IpuaroError("file", "Not found", true, "Check path") + const error = new IpuaroError("file", "Not found", { + suggestion: "Check path", + context: { filePath: "/test.ts" }, + }) expect(error.name).toBe("IpuaroError") expect(error.type).toBe("file") expect(error.message).toBe("Not found") expect(error.recoverable).toBe(true) expect(error.suggestion).toBe("Check path") + expect(error.context).toEqual({ filePath: "/test.ts" }) }) - it("should default recoverable to true", () => { - const error = new IpuaroError("parse", "Parse failed") + it("should use matrix defaults for recoverable", () => { + const redisError = new IpuaroError("redis", "Connection failed") + const parseError = new IpuaroError("parse", "Parse failed") - expect(error.recoverable).toBe(true) + expect(redisError.recoverable).toBe(false) + expect(parseError.recoverable).toBe(true) + }) + + it("should allow overriding recoverable", () => { + const error = new IpuaroError("command", "Blacklisted", { + recoverable: false, + }) + + expect(error.recoverable).toBe(false) + }) + + it("should have options from matrix", () => { + const error = new IpuaroError("llm", "Timeout") + + expect(error.options).toEqual(["retry", "skip", "abort"]) + expect(error.defaultOption).toBe("retry") + }) + }) + + describe("getMeta", () => { + it("should return error metadata", () => { + const error = IpuaroError.conflict("File changed") + + const meta = error.getMeta() + + expect(meta.type).toBe("conflict") + expect(meta.recoverable).toBe(true) + expect(meta.options).toEqual(["skip", "regenerate", "abort"]) + expect(meta.defaultOption).toBe("skip") + }) + }) + + describe("hasOption", () => { + it("should return true for available option", () => { + const error = IpuaroError.llm("Timeout") + + expect(error.hasOption("retry")).toBe(true) + expect(error.hasOption("skip")).toBe(true) + expect(error.hasOption("abort")).toBe(true) + }) + + it("should return false for unavailable option", () => { + const error = IpuaroError.parse("Syntax error") + + expect(error.hasOption("retry")).toBe(false) + expect(error.hasOption("regenerate")).toBe(false) + }) + }) + + describe("toDisplayString", () => { + it("should format error with suggestion", () => { + const error = IpuaroError.redis("Connection refused") + + const display = error.toDisplayString() + + expect(display).toContain("[redis]") + expect(display).toContain("Connection refused") + expect(display).toContain("Suggestion:") + }) + + it("should format error without suggestion", () => { + const error = new IpuaroError("unknown", "Something went wrong") + + const display = error.toDisplayString() + + expect(display).toBe("[unknown] Something went wrong") }) }) @@ -27,6 +98,13 @@ describe("IpuaroError", () => { expect(error.type).toBe("redis") expect(error.recoverable).toBe(false) expect(error.suggestion).toContain("Redis") + expect(error.options).toEqual(["retry", "abort"]) + }) + + it("should create redis error with context", () => { + const error = IpuaroError.redis("Connection failed", { host: "localhost" }) + + expect(error.context).toEqual({ host: "localhost" }) }) it("should create parse error", () => { @@ -35,12 +113,14 @@ describe("IpuaroError", () => { expect(error.type).toBe("parse") expect(error.message).toContain("test.ts") expect(error.recoverable).toBe(true) + expect(error.context).toEqual({ filePath: "test.ts" }) }) it("should create parse error without file", () => { const error = IpuaroError.parse("Syntax error") expect(error.message).toBe("Syntax error") + expect(error.context).toBeUndefined() }) it("should create llm error", () => { @@ -51,36 +131,113 @@ describe("IpuaroError", () => { expect(error.suggestion).toContain("Ollama") }) + it("should create llmTimeout error", () => { + const error = IpuaroError.llmTimeout("Request timed out") + + expect(error.type).toBe("timeout") + expect(error.suggestion).toContain("timed out") + }) + it("should create file error", () => { - const error = IpuaroError.file("Not found") + const error = IpuaroError.file("Not found", "/path/to/file.ts") expect(error.type).toBe("file") + expect(error.context).toEqual({ filePath: "/path/to/file.ts" }) + }) + + it("should create fileNotFound error", () => { + const error = IpuaroError.fileNotFound("/path/to/file.ts") + + expect(error.type).toBe("file") + expect(error.message).toContain("/path/to/file.ts") + expect(error.context).toEqual({ filePath: "/path/to/file.ts" }) }) it("should create command error", () => { - const error = IpuaroError.command("Blacklisted") + const error = IpuaroError.command("Not in whitelist", "rm -rf /") expect(error.type).toBe("command") + expect(error.context).toEqual({ command: "rm -rf /" }) + }) + + it("should create commandBlacklisted error", () => { + const error = IpuaroError.commandBlacklisted("rm -rf /") + + expect(error.type).toBe("command") + expect(error.recoverable).toBe(false) + expect(error.message).toContain("blacklisted") }) it("should create conflict error", () => { - const error = IpuaroError.conflict("File changed") + const error = IpuaroError.conflict("File changed", "test.ts") expect(error.type).toBe("conflict") expect(error.suggestion).toContain("Regenerate") + expect(error.context).toEqual({ filePath: "test.ts" }) }) it("should create validation error", () => { - const error = IpuaroError.validation("Invalid param") + const error = IpuaroError.validation("Invalid param", "name") expect(error.type).toBe("validation") + expect(error.context).toEqual({ field: "name" }) }) it("should create timeout error", () => { - const error = IpuaroError.timeout("Request timeout") + const error = IpuaroError.timeout("Request timeout", 5000) expect(error.type).toBe("timeout") expect(error.suggestion).toContain("timeout") + expect(error.context).toEqual({ timeoutMs: 5000 }) + }) + + it("should create unknown error", () => { + const original = new Error("Something broke") + const error = IpuaroError.unknown("Unknown error", original) + + expect(error.type).toBe("unknown") + expect(error.recoverable).toBe(false) + expect(error.context).toEqual({ originalError: "Something broke" }) + }) + }) + + describe("ERROR_MATRIX", () => { + it("should have all error types defined", () => { + const types = [ + "redis", + "parse", + "llm", + "file", + "command", + "conflict", + "validation", + "timeout", + "unknown", + ] + + for (const type of types) { + expect(ERROR_MATRIX[type as keyof typeof ERROR_MATRIX]).toBeDefined() + } + }) + + it("should have correct non-recoverable errors", () => { + expect(ERROR_MATRIX.redis.recoverable).toBe(false) + expect(ERROR_MATRIX.unknown.recoverable).toBe(false) + }) + + it("should have correct recoverable errors", () => { + expect(ERROR_MATRIX.parse.recoverable).toBe(true) + expect(ERROR_MATRIX.llm.recoverable).toBe(true) + expect(ERROR_MATRIX.file.recoverable).toBe(true) + expect(ERROR_MATRIX.command.recoverable).toBe(true) + expect(ERROR_MATRIX.conflict.recoverable).toBe(true) + expect(ERROR_MATRIX.timeout.recoverable).toBe(true) + }) + + it("should have abort option for all error types", () => { + for (const config of Object.values(ERROR_MATRIX)) { + expect(config.options).toContain("abort") + } }) }) })