docs: Add comprehensive error handling guidelines
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 <noreply@anthropic.com>
This commit is contained in:
385
ERROR_HANDLING_GUIDE.md
Normal file
385
ERROR_HANDLING_GUIDE.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user