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>
386 lines
10 KiB
Markdown
386 lines
10 KiB
Markdown
# 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.
|