From 235e6831edee76552ec834411c8cd89ce02cce98 Mon Sep 17 00:00:00 2001 From: tegwick Date: Fri, 26 Sep 2025 16:54:44 +0200 Subject: [PATCH] docs: Add comprehensive error handling guidelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create ERROR_HANDLING_GUIDE.md with complete reference for maintaining consistent error handling patterns across the codebase: • Quick reference with DO/DON'T examples • Complete exception hierarchy documentation • Service layer and file operation patterns • Exception chaining and logging integration rules • Anti-patterns to avoid and testing guidelines • Refactoring checklist with search patterns • Migration templates for future cleanups This guide ensures: - Consistent error handling patterns - Preserved debugging context - User-friendly error messages - No silent failures - Easy future maintenance Prevents codebase coherence loss over time by providing systematic approach for identifying and fixing error handling issues. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- ERROR_HANDLING_GUIDE.md | 385 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 385 insertions(+) create mode 100644 ERROR_HANDLING_GUIDE.md diff --git a/ERROR_HANDLING_GUIDE.md b/ERROR_HANDLING_GUIDE.md new file mode 100644 index 00000000..b2eff8cd --- /dev/null +++ b/ERROR_HANDLING_GUIDE.md @@ -0,0 +1,385 @@ +# Error Handling Guidelines + +**Version**: 1.0 +**Last Updated**: 2025-09-26 +**Purpose**: Maintain consistent, debuggable error handling across the codebase + +## Quick Reference + +### ✅ DO +```python +# Specific exception handling with chaining +try: + result = api_call() +except GiteaNotFoundError as e: + raise IssueError(f"Issue #{number} not found") from e +except GiteaAuthError as e: + raise IssueError(f"Authentication failed") from e + +# Logging for unexpected errors +except Exception as e: + logger.error(f"Unexpected error in {operation}", exc_info=True) + raise DomainError(f"Operation failed: {operation}") from e +``` + +### ❌ DON'T +```python +# Overly broad exception handling +try: + result = api_call() +except Exception as e: # Too broad! + raise IssueError(f"Failed: {e}") + +# Silent error suppression +try: + process_file() +except Exception: + continue # Never do this! +``` + +## 1. Exception Hierarchy + +### Use Domain-Specific Exceptions + +**Markitect Operations:** +```python +from markitect.exceptions import ( + MarkitectError, # Base for all Markitect operations + DocumentError, # Document processing errors + ASTError, # AST parsing/processing errors + CacheError, # Cache operations errors + DatabaseError, # Database operation errors + SchemaError, # Schema validation/processing + ValidationError, # Document validation errors + GraphQLError, # GraphQL operations + ConfigurationError # Configuration/setup errors +) +``` + +**TDDAI Operations:** +```python +from tddai.exceptions import ( + TddaiError, # Base for TDDAI operations + WorkspaceError, # Workspace management + IssueError, # Issue fetching/management + TestGenerationError, # Test generation + ConfigurationError # Configuration issues +) +``` + +**Gitea Operations:** +```python +from gitea.exceptions import ( + GiteaError, # Base Gitea error + GiteaNotFoundError,# 404 responses + GiteaAuthError, # Authentication failures + GiteaApiError, # API errors with status codes + GiteaConfigError # Configuration issues +) +``` + +## 2. Exception Translation Patterns + +### Service Layer Pattern +Services should translate external exceptions to domain exceptions: + +```python +class IssueService: + def get_issue(self, issue_number: int) -> Issue: + """Get issue by number. + + Raises: + IssueError: When issue cannot be retrieved + """ + try: + return self.gitea_client.issues.get(issue_number) + except GiteaNotFoundError as e: + raise IssueError(f"Issue #{issue_number} not found") from e + except GiteaAuthError as e: + raise IssueError(f"Authentication failed") from e + except GiteaApiError as e: + raise IssueError(f"API error: {e}") from e + # Don't catch GiteaError - let specific exceptions handle it +``` + +### File Operations Pattern +```python +def read_config_file(file_path: Path) -> dict: + """Read configuration file. + + Raises: + ConfigurationError: When file cannot be read or parsed + """ + try: + content = file_path.read_text() + return json.loads(content) + except FileNotFoundError as e: + raise ConfigurationError(f"Config file not found: {file_path}") from e + except PermissionError as e: + raise ConfigurationError(f"Permission denied reading: {file_path}") from e + except json.JSONDecodeError as e: + raise ConfigurationError(f"Invalid JSON in {file_path}: {e}") from e + except Exception as e: + logger.error(f"Unexpected error reading {file_path}", exc_info=True) + raise ConfigurationError(f"Failed to read config: {file_path}") from e +``` + +## 3. Exception Chaining Rules + +### Always Chain Exceptions +Use `raise ... from e` to preserve the original exception: + +```python +# ✅ CORRECT - preserves debugging information +try: + dangerous_operation() +except SpecificError as e: + raise DomainError("User-friendly message") from e + +# ❌ WRONG - loses original exception context +try: + dangerous_operation() +except SpecificError as e: + raise DomainError(f"Failed: {e}") +``` + +### Chain Standard Exceptions +```python +try: + data = json.loads(content) +except json.JSONDecodeError as e: + raise ValidationError(f"Invalid JSON format") from e +except (ValueError, TypeError) as e: + raise ValidationError(f"Data validation failed") from e +``` + +## 4. Logging Integration + +### Log Before Re-raising +```python +import logging + +logger = logging.getLogger(__name__) + +try: + complex_operation() +except ExpectedError as e: + # Don't log expected errors - let caller decide + raise DomainError("Operation failed") from e +except Exception as e: + # Always log unexpected errors + logger.error( + "Unexpected error in complex_operation", + extra={'context': {'param1': value1}}, + exc_info=True + ) + raise DomainError("Unexpected failure") from e +``` + +### Logging Levels +- **ERROR**: Unexpected exceptions that indicate bugs +- **WARNING**: Expected exceptions that are concerning (file not found, permission denied) +- **INFO**: Normal error recovery (retries, fallbacks) +- **DEBUG**: Detailed error context for development + +## 5. Error Messages + +### User-Facing Messages +```python +# ✅ GOOD - actionable and specific +raise IssueError(f"Issue #{number} not found. Check the issue number and try again.") + +# ✅ GOOD - includes context +raise ConfigurationError(f"Missing required setting 'api_token' in {config_file}") + +# ❌ BAD - too technical +raise IssueError(f"HTTP 404 response from /api/v1/repos/owner/repo/issues/{number}") + +# ❌ BAD - too vague +raise IssueError("Something went wrong") +``` + +### Include Context +```python +# Use MarkitectError's context parameter +raise DocumentError( + "Failed to parse document", + cause=original_error, + context={ + 'file_path': str(file_path), + 'line_number': line_num, + 'operation': 'parse_markdown' + } +) +``` + +## 6. CLI Error Handling + +### Consistent CLI Pattern +```python +def cli_command(): + """CLI command that handles domain exceptions.""" + try: + result = service.perform_operation() + show_success(result) + except DomainError as e: + OutputFormatter.exit_with_error(str(e)) + # Don't catch Exception - let unexpected errors bubble up +``` + +### Exit Codes +- **0**: Success +- **1**: Expected failure (user error, missing resource) +- **2**: Configuration error +- **>2**: Unexpected error (let Python handle it) + +## 7. Anti-Patterns to Avoid + +### 1. Overly Broad Exception Handling +```python +# ❌ NEVER DO THIS +try: + operation() +except Exception as e: + raise DomainError(f"Failed: {e}") +``` + +### 2. Silent Error Suppression +```python +# ❌ NEVER DO THIS +try: + process_file(file) +except Exception: + continue # Silent failure! + +# ✅ DO THIS INSTEAD +try: + process_file(file) +except (OSError, IOError) as e: + logger.warning(f"Could not process {file}: {e}") + continue +except Exception as e: + logger.error(f"Unexpected error processing {file}: {e}", exc_info=True) + continue +``` + +### 3. Exception Conversion Without Context +```python +# ❌ WRONG - loses information +try: + api_call() +except requests.RequestException as e: + raise IssueError("API failed") + +# ✅ CORRECT - preserves context +try: + api_call() +except requests.ConnectionError as e: + raise IssueError("Cannot connect to Gitea server") from e +except requests.Timeout as e: + raise IssueError("Gitea server request timed out") from e +except requests.HTTPError as e: + raise IssueError(f"HTTP error: {e.response.status_code}") from e +``` + +## 8. Testing Error Handling + +### Test Exception Translation +```python +def test_issue_not_found(): + """Test that GiteaNotFoundError is translated to IssueError.""" + with mock.patch.object(gitea_client, 'get') as mock_get: + mock_get.side_effect = GiteaNotFoundError("Not found") + + with pytest.raises(IssueError) as exc_info: + service.get_issue(123) + + assert "Issue #123 not found" in str(exc_info.value) + assert exc_info.value.__cause__.__class__ == GiteaNotFoundError +``` + +### Test Error Messages +```python +def test_meaningful_error_messages(): + """Test that error messages are user-friendly.""" + with pytest.raises(ConfigurationError) as exc_info: + service.load_config("nonexistent.json") + + error_msg = str(exc_info.value) + assert "nonexistent.json" in error_msg + assert "not found" in error_msg.lower() +``` + +## 9. Refactoring Checklist + +When refactoring error handling, use this checklist: + +### 🔍 Identify Issues +- [ ] Search for `except Exception:` patterns +- [ ] Look for `continue` without logging in exception blocks +- [ ] Find missing exception chaining (`raise ... from e`) +- [ ] Check for generic error messages + +### 🔧 Fix Patterns +- [ ] Replace broad exceptions with specific ones +- [ ] Add proper exception chaining +- [ ] Implement logging for unexpected errors +- [ ] Improve error message clarity +- [ ] Add exception documentation to functions + +### ✅ Verify +- [ ] Test that CLI still works +- [ ] Verify error messages are user-friendly +- [ ] Check that debugging information is preserved +- [ ] Ensure no silent failures remain + +### 📚 Document +- [ ] Update function docstrings with `Raises:` sections +- [ ] Add new exceptions to relevant `__init__.py` files +- [ ] Update this guide if new patterns emerge + +## 10. Common Search Patterns + +Use these patterns to find error handling issues: + +```bash +# Find overly broad exception handling +rg "except Exception" --type py + +# Find silent error suppression +rg "except.*:\s*continue" --type py +rg "except.*:\s*pass" --type py + +# Find missing exception chaining +rg "raise.*Error.*:" --type py | grep -v "from" + +# Find exception handling without logging +rg "except.*Exception.*:" -A 3 --type py | grep -v "log" +``` + +## 11. Quick Migration Template + +Use this template for migrating old exception handling: + +```python +# OLD PATTERN +try: + operation() +except Exception as e: + raise DomainError(f"Operation failed: {e}") + +# NEW PATTERN +try: + operation() +except SpecificError1 as e: + raise DomainError(f"Specific failure case 1") from e +except SpecificError2 as e: + raise DomainError(f"Specific failure case 2") from e +except Exception as e: + logger.error("Unexpected error in operation", exc_info=True) + raise DomainError(f"Unexpected operation failure") from e +``` + +--- + +**Remember**: Good error handling makes debugging easier, provides better user experience, and prevents silent failures that hide bugs. When in doubt, be specific, preserve context, and log unexpected errors.