From 5ed91e88d232cd22c159355e1806fb9f686f59cc Mon Sep 17 00:00:00 2001 From: Aditya <97450298+1234-ad@users.noreply.github.com> Date: Mon, 12 Jan 2026 22:10:29 +0530 Subject: [PATCH 1/3] chore: add custom exception classes for better error handling Add comprehensive custom exception hierarchy: - PRReviewError: Base exception for all PR review errors - GitHubAPIError: GitHub API failures with status code tracking - MetricsCalculationError: Metrics computation failures - AILabelingError: AI labeling service failures - ReviewerAssignmentError: Reviewer assignment failures - DiscordNotificationError: Discord notification failures - ConfigurationError: Configuration validation errors - ValidationError: Input validation errors Benefits: - Better error categorization and handling - Easier debugging with specific exception types - Improved error messages and context - Enables targeted exception handling in try-except blocks --- pr_review/exceptions.py | 47 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 pr_review/exceptions.py diff --git a/pr_review/exceptions.py b/pr_review/exceptions.py new file mode 100644 index 0000000..5ce1968 --- /dev/null +++ b/pr_review/exceptions.py @@ -0,0 +1,47 @@ +""" +Custom exceptions for the PR Review System +""" + + +class PRReviewError(Exception): + """Base exception for PR Review System errors""" + pass + + +class GitHubAPIError(PRReviewError): + """Raised when GitHub API requests fail""" + + def __init__(self, message: str, status_code: int = None, response: dict = None): + self.status_code = status_code + self.response = response + super().__init__(message) + + +class MetricsCalculationError(PRReviewError): + """Raised when metrics calculation fails""" + pass + + +class AILabelingError(PRReviewError): + """Raised when AI labeling fails""" + pass + + +class ReviewerAssignmentError(PRReviewError): + """Raised when reviewer assignment fails""" + pass + + +class DiscordNotificationError(PRReviewError): + """Raised when Discord notification fails""" + pass + + +class ConfigurationError(PRReviewError): + """Raised when configuration is invalid or missing""" + pass + + +class ValidationError(PRReviewError): + """Raised when input validation fails""" + pass From bae87153f6c90e7e5f96bf2b1af62afc565492cb Mon Sep 17 00:00:00 2001 From: Aditya <97450298+1234-ad@users.noreply.github.com> Date: Mon, 12 Jan 2026 22:10:48 +0530 Subject: [PATCH 2/3] chore: add centralized logging configuration module Add comprehensive logging utilities: - setup_logging(): Configure logging with console and file output - Rotating file handler with configurable size and backup count - LoggerMixin: Add logging to any class via inheritance - LogLevel: Context manager for temporary log level changes - Structured log format with filename and line numbers - Configurable log levels and custom format strings Benefits: - Consistent logging across all modules - Easy log file rotation to prevent disk space issues - Better debugging with file/line information - Flexible logging configuration - Production-ready logging setup Usage: from logging_config import setup_logging, LoggerMixin logger = setup_logging('my_module', level='DEBUG', log_file='logs/app.log') logger.info('Application started') --- pr_review/logging_config.py | 112 ++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 pr_review/logging_config.py diff --git a/pr_review/logging_config.py b/pr_review/logging_config.py new file mode 100644 index 0000000..05320d2 --- /dev/null +++ b/pr_review/logging_config.py @@ -0,0 +1,112 @@ +""" +Centralized logging configuration for the PR Review System +""" + +import logging +import sys +from pathlib import Path +from logging.handlers import RotatingFileHandler +from typing import Optional + + +def setup_logging( + name: str = "pr_review", + level: str = "INFO", + log_file: Optional[str] = None, + max_bytes: int = 10485760, # 10MB + backup_count: int = 5, + format_string: Optional[str] = None +) -> logging.Logger: + """ + Configure logging with console and optional file output. + + Args: + name: Logger name + level: Logging level (DEBUG, INFO, WARNING, ERROR, CRITICAL) + log_file: Optional log file path + max_bytes: Maximum log file size before rotation + backup_count: Number of backup files to keep + format_string: Custom format string + + Returns: + Configured logger instance + """ + # Create logger + logger = logging.getLogger(name) + logger.setLevel(getattr(logging, level.upper())) + + # Remove existing handlers to avoid duplicates + logger.handlers.clear() + + # Default format + if format_string is None: + format_string = ( + '%(asctime)s - %(name)s - %(levelname)s - ' + '[%(filename)s:%(lineno)d] - %(message)s' + ) + + formatter = logging.Formatter(format_string, datefmt='%Y-%m-%d %H:%M:%S') + + # Console handler + console_handler = logging.StreamHandler(sys.stdout) + console_handler.setLevel(logging.DEBUG) + console_handler.setFormatter(formatter) + logger.addHandler(console_handler) + + # File handler (optional) + if log_file: + log_path = Path(log_file) + log_path.parent.mkdir(parents=True, exist_ok=True) + + file_handler = RotatingFileHandler( + log_file, + maxBytes=max_bytes, + backupCount=backup_count + ) + file_handler.setLevel(logging.DEBUG) + file_handler.setFormatter(formatter) + logger.addHandler(file_handler) + + return logger + + +def get_logger(name: str) -> logging.Logger: + """ + Get a logger instance with the given name. + + Args: + name: Logger name + + Returns: + Logger instance + """ + return logging.getLogger(name) + + +class LoggerMixin: + """Mixin class to add logging capabilities to any class""" + + @property + def logger(self) -> logging.Logger: + """Get logger for this class""" + name = f"{self.__class__.__module__}.{self.__class__.__name__}" + return get_logger(name) + + +# Context manager for temporary log level changes +class LogLevel: + """Context manager for temporarily changing log level""" + + def __init__(self, logger: logging.Logger, level: str): + self.logger = logger + self.new_level = getattr(logging, level.upper()) + self.old_level = None + + def __enter__(self): + self.old_level = self.logger.level + self.logger.setLevel(self.new_level) + return self.logger + + def __exit__(self, exc_type, exc_val, exc_tb): + self.logger.setLevel(self.old_level) + return False From f6d5c2838d53f6f2c9cd517878b5f4c12f77ebce Mon Sep 17 00:00:00 2001 From: Aditya <97450298+1234-ad@users.noreply.github.com> Date: Mon, 12 Jan 2026 22:11:14 +0530 Subject: [PATCH 3/3] chore: add retry decorator and circuit breaker for resilient API calls Add comprehensive retry and resilience utilities: - retry(): Decorator for sync functions with exponential backoff - retry_async(): Decorator for async functions with exponential backoff - CircuitBreaker: Prevent cascading failures with circuit breaker pattern - Configurable retry attempts, delays, and backoff multipliers - Custom exception handling and retry callbacks - Detailed logging of retry attempts and failures Benefits: - Resilient API calls that handle transient failures - Exponential backoff prevents overwhelming failing services - Circuit breaker prevents cascading failures - Better error handling and recovery - Production-ready reliability patterns Usage: from retry_utils import retry, CircuitBreaker @retry(max_attempts=3, delay=1.0, backoff=2.0) def fetch_github_data(): return github_client.get_pull_request(repo, pr_number) --- pr_review/utils/retry_utils.py | 205 +++++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 pr_review/utils/retry_utils.py diff --git a/pr_review/utils/retry_utils.py b/pr_review/utils/retry_utils.py new file mode 100644 index 0000000..868b9cc --- /dev/null +++ b/pr_review/utils/retry_utils.py @@ -0,0 +1,205 @@ +""" +Retry decorator and utilities for resilient API calls +""" + +import time +import functools +import logging +from typing import Callable, Type, Tuple, Optional + +logger = logging.getLogger(__name__) + + +def retry( + max_attempts: int = 3, + delay: float = 1.0, + backoff: float = 2.0, + exceptions: Tuple[Type[Exception], ...] = (Exception,), + on_retry: Optional[Callable] = None +): + """ + Decorator to retry a function on failure with exponential backoff. + + Args: + max_attempts: Maximum number of retry attempts + delay: Initial delay between retries in seconds + backoff: Multiplier for delay after each retry + exceptions: Tuple of exception types to catch and retry + on_retry: Optional callback function called on each retry + + Returns: + Decorated function with retry logic + + Example: + @retry(max_attempts=3, delay=1.0, backoff=2.0, exceptions=(requests.RequestException,)) + def fetch_data(): + response = requests.get('https://api.example.com/data') + return response.json() + """ + def decorator(func: Callable) -> Callable: + @functools.wraps(func) + def wrapper(*args, **kwargs): + current_delay = delay + last_exception = None + + for attempt in range(1, max_attempts + 1): + try: + return func(*args, **kwargs) + except exceptions as e: + last_exception = e + + if attempt == max_attempts: + logger.error( + f"Function {func.__name__} failed after {max_attempts} attempts. " + f"Last error: {str(e)}" + ) + raise + + logger.warning( + f"Function {func.__name__} failed (attempt {attempt}/{max_attempts}). " + f"Retrying in {current_delay:.1f}s... Error: {str(e)}" + ) + + if on_retry: + on_retry(attempt, e) + + time.sleep(current_delay) + current_delay *= backoff + + # This should never be reached, but just in case + if last_exception: + raise last_exception + + return wrapper + return decorator + + +def retry_async( + max_attempts: int = 3, + delay: float = 1.0, + backoff: float = 2.0, + exceptions: Tuple[Type[Exception], ...] = (Exception,), + on_retry: Optional[Callable] = None +): + """ + Async version of retry decorator for async functions. + + Args: + max_attempts: Maximum number of retry attempts + delay: Initial delay between retries in seconds + backoff: Multiplier for delay after each retry + exceptions: Tuple of exception types to catch and retry + on_retry: Optional callback function called on each retry + + Returns: + Decorated async function with retry logic + + Example: + @retry_async(max_attempts=3, delay=1.0, exceptions=(aiohttp.ClientError,)) + async def fetch_data(): + async with aiohttp.ClientSession() as session: + async with session.get('https://api.example.com/data') as response: + return await response.json() + """ + def decorator(func: Callable) -> Callable: + @functools.wraps(func) + async def wrapper(*args, **kwargs): + import asyncio + + current_delay = delay + last_exception = None + + for attempt in range(1, max_attempts + 1): + try: + return await func(*args, **kwargs) + except exceptions as e: + last_exception = e + + if attempt == max_attempts: + logger.error( + f"Async function {func.__name__} failed after {max_attempts} attempts. " + f"Last error: {str(e)}" + ) + raise + + logger.warning( + f"Async function {func.__name__} failed (attempt {attempt}/{max_attempts}). " + f"Retrying in {current_delay:.1f}s... Error: {str(e)}" + ) + + if on_retry: + if asyncio.iscoroutinefunction(on_retry): + await on_retry(attempt, e) + else: + on_retry(attempt, e) + + await asyncio.sleep(current_delay) + current_delay *= backoff + + # This should never be reached, but just in case + if last_exception: + raise last_exception + + return wrapper + return decorator + + +class CircuitBreaker: + """ + Circuit breaker pattern implementation for preventing cascading failures. + + States: + - CLOSED: Normal operation, requests pass through + - OPEN: Too many failures, requests fail immediately + - HALF_OPEN: Testing if service recovered + """ + + def __init__( + self, + failure_threshold: int = 5, + recovery_timeout: float = 60.0, + expected_exception: Type[Exception] = Exception + ): + self.failure_threshold = failure_threshold + self.recovery_timeout = recovery_timeout + self.expected_exception = expected_exception + + self.failure_count = 0 + self.last_failure_time = None + self.state = "CLOSED" + + def call(self, func: Callable, *args, **kwargs): + """Execute function with circuit breaker protection""" + if self.state == "OPEN": + if time.time() - self.last_failure_time >= self.recovery_timeout: + self.state = "HALF_OPEN" + logger.info(f"Circuit breaker entering HALF_OPEN state for {func.__name__}") + else: + raise Exception(f"Circuit breaker is OPEN for {func.__name__}") + + try: + result = func(*args, **kwargs) + self._on_success() + return result + except self.expected_exception as e: + self._on_failure() + raise e + + def _on_success(self): + """Handle successful call""" + self.failure_count = 0 + if self.state == "HALF_OPEN": + self.state = "CLOSED" + logger.info("Circuit breaker recovered, state: CLOSED") + + def _on_failure(self): + """Handle failed call""" + self.failure_count += 1 + self.last_failure_time = time.time() + + if self.failure_count >= self.failure_threshold: + self.state = "OPEN" + logger.error( + f"Circuit breaker opened after {self.failure_count} failures. " + f"Will retry after {self.recovery_timeout}s" + )