Fix critical deployment issues: Redis connection, Ollama URL sanitization, Hugging Face provider, config validation, and enhanced error handling

api/status.py

@@ -1,12 +1,76 @@
-from fastapi import APIRouter
+from fastapi import APIRouter, HTTPException
+import logging
+from core.redis_client import redis_client
 from services.ollama_monitor import check_ollama_status
+from utils.config import config
+from utils.config_validator import ConfigValidator
 
+logger = logging.getLogger(__name__)
 router = APIRouter()
 
 @router.get("/ollama-status")
 async def get_ollama_status():
-    """
-    Returns the current status of the Ollama service.
-    """
-    status = check_ollama_status()
-    return status
+    """Returns the current status of the Ollama service."""
+    try:
+        status = check_ollama_status()
+        return status
+    except Exception as e:
+        logger.error(f"Error checking Ollama status: {e}")
+        raise HTTPException(status_code=500, detail=str(e))
+
+@router.get("/health")
+async def health_check():
+    """Comprehensive health check for all services"""
+    try:
+        # Check configuration
+        validator = ConfigValidator()
+        config_report = validator.validate_all()
+        
+        # Check Redis
+        redis_healthy = redis_client.is_healthy() if redis_client.get_client() else False
+        
+        # Check Ollama
+        try:
+            ollama_status = check_ollama_status()
+            ollama_healthy = ollama_status.get("running", False)
+        except Exception as e:
+            logger.error(f"Error checking Ollama status: {e}")
+            ollama_healthy = False
+            ollama_status = {"error": str(e)}
+        
+        # Overall health
+        overall_healthy = redis_healthy and ollama_healthy and config_report['valid']
+        
+        return {
+            "status": "healthy" if overall_healthy else "degraded",
+            "services": {
+                "redis": "healthy" if redis_healthy else "unhealthy",
+                "ollama": "healthy" if ollama_healthy else "unhealthy",
+                "configuration": "valid" if config_report['valid'] else "invalid"
+            },
+            "details": {
+                "redis_healthy": redis_healthy,
+                "ollama_status": ollama_status,
+                "config_validation": config_report
+            }
+        }
+    except Exception as e:
+        logger.error(f"Health check failed: {e}")
+        raise HTTPException(status_code=500, detail=str(e))
+
+@router.get("/config")
+async def config_status():
+    """Return current configuration status"""
+    validator = ConfigValidator()
+    report = validator.validate_all()
+    
+    return {
+        "configuration": {
+            "ollama_host": config.ollama_host,
+            "hf_api_url": config.hf_api_url,
+            "redis_host": config.redis_host,
+            "redis_port": config.redis_port,
+            "use_fallback": config.use_fallback
+        },
+        "validation": report
+    }

core/memory.py

@@ -1,94 +1,23 @@
 import json
 import time
 import logging
+from core.redis_client import redis_client
 from utils.config import config
 
 # Set up logging
 logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger(__name__)
 
-# Try to import redis, but handle if not available
-try:
-    import redis
-    REDIS_AVAILABLE = True
-except ImportError:
-    REDIS_AVAILABLE = False
-    redis = None
-
-def get_redis_client():
-    """Create and return Redis client with retry logic"""
-    if not REDIS_AVAILABLE:
-        return None
-    
-    # Debug print to verify secrets are loaded (separate lines to avoid formatting issues)
-    logger.info(f"[DEBUG] Redis Host = {config.redis_host}")
-    logger.info(f"[DEBUG] Redis Port = {config.redis_port}")
-    
-    last_exception = None
-    
-    for attempt in range(config.redis_retries + 1):
-        try:
-            # Handle empty username/password gracefully
-            # Try with SSL first (required for many cloud providers)
-            redis_client = redis.Redis(
-                host=config.redis_host,
-                port=config.redis_port,
-                username=config.redis_username if config.redis_username else None,
-                password=config.redis_password if config.redis_password else None,
-                decode_responses=True,
-                socket_connect_timeout=5,
-                socket_timeout=5,
-                ssl=True,  # Enable TLS (required for Redis Cloud and many providers)
-                ssl_cert_reqs=None  # Skip cert verification (commonly required)
-            )
-            # Test the connection
-            redis_client.ping()
-            logger.info("Successfully connected to Redis with SSL")
-            return redis_client
-        except Exception as e:
-            # If SSL fails, try without SSL
-            try:
-                redis_client = redis.Redis(
-                    host=config.redis_host,
-                    port=config.redis_port,
-                    username=config.redis_username if config.redis_username else None,
-                    password=config.redis_password if config.redis_password else None,
-                    decode_responses=True,
-                    socket_connect_timeout=5,
-                    socket_timeout=5
-                )
-                # Test the connection
-                redis_client.ping()
-                logger.info("Successfully connected to Redis without SSL")
-                return redis_client
-            except Exception as e2:
-                last_exception = e2
-                if attempt < config.redis_retries:
-                    time.sleep(config.redis_retry_delay * (2 ** attempt))  # Exponential backoff
-                continue
-    
-    logger.warning(f"Could not connect to Redis after {config.redis_retries + 1} attempts: {last_exception}")
-    return None
-
-# Initialize Redis connection with better error handling
-redis_client = None
-try:
-    redis_client = get_redis_client()
-    if redis_client is None and REDIS_AVAILABLE:
-        logger.warning("Could not connect to Redis - using in-memory storage")
-except Exception as e:
-    logger.error(f"Redis initialization failed: {e}")
-
 def save_user_state(user_id: str, state: dict):
     """Save user state to Redis with fallback to in-memory storage"""
-    global redis_client
-    if not redis_client:
+    client = redis_client.get_client()
+    if not client:
         # Fallback: use in-memory storage (will not persist across restarts)
         logger.info("Redis not available, using in-memory storage for user state")
         return False
-    
+
     try:
-        redis_client.hset(f"user:{user_id}", mapping=state)
+        client.hset(f"user:{user_id}", mapping=state)
         return True
     except Exception as e:
         logger.error(f"Error saving user state: {e}")
@@ -96,24 +25,17 @@ def save_user_state(user_id: str, state: dict):
 
 def load_user_state(user_id: str):
     """Load user state from Redis with fallback"""
-    global redis_client
-    if not redis_client:
+    client = redis_client.get_client()
+    if not client:
         logger.info("Redis not available, returning empty state")
         return {}
-    
+
     try:
-        return redis_client.hgetall(f"user:{user_id}")
+        return client.hgetall(f"user:{user_id}")
     except Exception as e:
         logger.error(f"Error loading user state: {e}")
         return {}
 
 def check_redis_health():
     """Check if Redis is healthy"""
-    global redis_client
-    if not redis_client:
-        return False
-    try:
-        redis_client.ping()
-        return True
-    except:
-        return False
+    return redis_client.is_healthy()

core/providers/base.py

@@ -14,6 +14,8 @@ class LLMProvider(ABC):
         self.timeout = timeout
         self.max_retries = max_retries
         self.is_available = True
+        self.failure_count = 0
+        self.last_failure_time = None
     
     @abstractmethod
     def generate(self, prompt: str, conversation_history: List[Dict]) -> Optional[str]:
@@ -31,16 +33,30 @@ class LLMProvider(ABC):
         pass
     
     def _retry_with_backoff(self, func, *args, **kwargs):
-        """Retry logic with exponential backoff"""
+        """Retry logic with exponential backoff and circuit breaker"""
         last_exception = None
         
         for attempt in range(self.max_retries):
             try:
-                return func(*args, **kwargs)
+                # Simple circuit breaker - fail fast if too many recent failures
+                if self.failure_count > 5 and self.last_failure_time:
+                    time_since_failure = time.time() - self.last_failure_time
+                    if time_since_failure < 60:  # Wait 1 minute after 5 failures
+                        raise Exception("Circuit breaker tripped - too many recent failures")
+                
+                result = func(*args, **kwargs)
+                # Reset failure count on success
+                self.failure_count = 0
+                self.last_failure_time = None
+                return result
+                
             except Exception as e:
                 last_exception = e
+                self.failure_count += 1
+                self.last_failure_time = time.time()
+                
                 if attempt < self.max_retries - 1:  # Don't sleep on last attempt
-                    sleep_time = (2 ** attempt) * 0.5  # Exponential backoff starting at 0.5s
+                    sleep_time = min((2 ** attempt) * 1.0, 10.0)  # Cap at 10 seconds
                     logger.warning(f"Attempt {attempt + 1} failed: {str(e)}. Retrying in {sleep_time}s...")
                     time.sleep(sleep_time)
                 else:
@@ -48,14 +64,29 @@ class LLMProvider(ABC):
         
         raise last_exception
     
-    def _is_rate_limited(self, error: Exception) -> bool:
-        """Check if the error is related to rate limiting"""
+    def _classify_error(self, error: Exception) -> str:
+        """Classify error type for better handling"""
         error_str = str(error).lower()
-        rate_limit_indicators = [
-            "rate limit",
-            "too many requests",
-            "quota exceeded",
-            "429",
-            "limit exceeded"
-        ]
-        return any(indicator in error_str for indicator in rate_limit_indicators)
+        
+        # Network errors
+        if any(term in error_str for term in ['connection', 'timeout', 'resolve', 'unreachable']):
+            return 'network'
+        
+        # Authentication errors
+        if any(term in error_str for term in ['auth', 'unauthorized', 'invalid token', '401', '403']):
+            return 'authentication'
+        
+        # Rate limiting
+        if any(term in error_str for term in ['rate limit', 'too many requests', 'quota exceeded', '429']):
+            return 'rate_limit'
+        
+        # Server errors
+        if any(term in error_str for term in ['500', '502', '503', 'server error']):
+            return 'server'
+        
+        return 'other'
+    
+    def _is_recoverable_error(self, error: Exception) -> bool:
+        """Determine if error is likely recoverable"""
+        error_type = self._classify_error(error)
+        return error_type in ['network', 'rate_limit', 'server']

core/providers/huggingface.py

@@ -25,6 +25,7 @@ class HuggingFaceProvider(LLMProvider):
         if not config.hf_token:
             raise ValueError("HF_TOKEN not set - required for Hugging Face provider")
             
+        # Fixed OpenAI client instantiation without deprecated parameters
         self.client = OpenAI(
             base_url=config.hf_api_url,
             api_key=config.hf_token
@@ -49,7 +50,6 @@ class HuggingFaceProvider(LLMProvider):
     def validate_model(self) -> bool:
         """Validate if the model is available"""
         # For Hugging Face endpoints, we'll assume the model is valid if we can connect
-        # In production, you might want to ping the endpoint specifically
         try:
             # Simple connectivity check
             self.client.models.list()

core/providers/ollama.py

@@ -1,5 +1,6 @@
 import requests
 import logging
+import re
 from typing import List, Dict, Optional, Union
 from core.providers.base import LLMProvider
 from utils.config import config
@@ -11,13 +12,30 @@ class OllamaProvider(LLMProvider):
     
     def __init__(self, model_name: str, timeout: int = 30, max_retries: int = 3):
         super().__init__(model_name, timeout, max_retries)
-        self.host = config.ollama_host or "http://localhost:11434"
+        self.host = self._sanitize_host(config.ollama_host or "http://localhost:11434")
         # Headers to skip ngrok browser warning
         self.headers = {
             "ngrok-skip-browser-warning": "true",
             "User-Agent": "AI-Life-Coach-Ollama"
         }
     
+    def _sanitize_host(self, host: str) -> str:
+        """Sanitize host URL by removing whitespace and control characters"""
+        if not host:
+            return "http://localhost:11434"
+        
+        # Remove leading/trailing whitespace and control characters
+        host = host.strip()
+        
+        # Remove any newlines or control characters
+        host = re.sub(r'[\r\n\t\0]+', '', host)
+        
+        # Ensure URL has a scheme
+        if not host.startswith(('http://', 'https://')):
+            host = 'http://' + host
+            
+        return host
+    
     def generate(self, prompt: str, conversation_history: List[Dict]) -> Optional[str]:
         """Generate a response synchronously"""
         try:

core/redis_client.py

@@ -0,0 +1,120 @@
+import redis
+import logging
+import time
+import re
+from typing import Optional
+from utils.config import config
+
+logger = logging.getLogger(__name__)
+
+class RedisClient:
+    """Enhanced Redis client with proper connection handling"""
+    
+    _instance = None
+    _redis_client = None
+    
+    def __new__(cls):
+        if cls._instance is None:
+            cls._instance = super(RedisClient, cls).__new__(cls)
+        return cls._instance
+    
+    def __init__(self):
+        if not hasattr(self, '_initialized'):
+            self._initialized = True
+            self._connect()
+    
+    def _connect(self):
+        """Establish Redis connection with proper error handling"""
+        if not config.redis_host or config.redis_host == "localhost":
+            logger.info("Redis not configured, skipping connection")
+            return None
+            
+        try:
+            # Parse host and port if port is included in host
+            host, port = self._parse_host_port(config.redis_host, config.redis_port)
+            
+            logger.info(f"Connecting to Redis at {host}:{port}")
+            
+            # Try SSL connection first (required for Redis Cloud)
+            self._redis_client = redis.Redis(
+                host=host,
+                port=port,
+                username=config.redis_username or None,
+                password=config.redis_password or None,
+                decode_responses=True,
+                socket_connect_timeout=5,
+                socket_timeout=5,
+                ssl=True,
+                ssl_cert_reqs=None,  # Skip cert verification for Redis Cloud
+                retry_on_timeout=True,
+                health_check_interval=30
+            )
+            
+            # Test connection
+            self._redis_client.ping()
+            logger.info("Successfully connected to Redis with SSL")
+            
+        except Exception as e:
+            logger.warning(f"SSL connection failed: {e}, trying without SSL")
+            try:
+                # Fallback to non-SSL connection
+                host, port = self._parse_host_port(config.redis_host, config.redis_port)
+                
+                self._redis_client = redis.Redis(
+                    host=host,
+                    port=port,
+                    username=config.redis_username or None,
+                    password=config.redis_password or None,
+                    decode_responses=True,
+                    socket_connect_timeout=5,
+                    socket_timeout=5,
+                    retry_on_timeout=True,
+                    health_check_interval=30
+                )
+                
+                # Test connection
+                self._redis_client.ping()
+                logger.info("Successfully connected to Redis without SSL")
+                
+            except Exception as e2:
+                logger.error(f"Could not connect to Redis: {e2}")
+                self._redis_client = None
+    
+    def _parse_host_port(self, host_string: str, default_port: int) -> tuple:
+        """Parse host and port from host string"""
+        # Remove any whitespace or control characters
+        host_string = host_string.strip()
+        
+        # Check if port is included in host (e.g., "host:port")
+        if ':' in host_string and not host_string.startswith('['):  # IPv6 addresses start with [
+            parts = host_string.rsplit(':', 1)  # Split only on the last colon
+            try:
+                host = parts[0]
+                port = int(parts[1])
+                return host, port
+            except ValueError:
+                # Port is not a valid integer
+                return host_string, default_port
+        
+        return host_string, default_port
+    
+    def get_client(self) -> Optional[redis.Redis]:
+        """Get Redis client instance"""
+        return self._redis_client
+    
+    def is_healthy(self) -> bool:
+        """Check if Redis connection is healthy"""
+        if not self._redis_client:
+            return False
+        try:
+            self._redis_client.ping()
+            return True
+        except:
+            return False
+    
+    def reconnect(self):
+        """Reconnect to Redis"""
+        self._connect()
+
+# Global Redis client instance
+redis_client = RedisClient()

core/session.py

@@ -12,8 +12,7 @@ class SessionManager:
     """Manages user sessions and conversation context"""
     
     def __init__(self, session_timeout: int = 3600):
-        """
-        Initialize session manager
+        """Initialize session manager
         
         Args:
             session_timeout: Session timeout in seconds (default: 1 hour)
@@ -21,8 +20,7 @@ class SessionManager:
         self.session_timeout = session_timeout
 
     def get_session(self, user_id: str) -> Dict[str, Any]:
-        """
-        Retrieve user session data
+        """Retrieve user session data
         
         Args:
             user_id: Unique identifier for the user
@@ -35,21 +33,20 @@ class SessionManager:
             if not state:
                 logger.info(f"Creating new session for user {user_id}")
                 return self._create_new_session()
-            
+
             # Check if session has expired
             last_activity = float(state.get('last_activity', 0))
             if time.time() - last_activity > self.session_timeout:
                 logger.info(f"Session expired for user {user_id}, creating new session")
                 return self._create_new_session()
-            
+
             return state
         except Exception as e:
             logger.error(f"Error retrieving session for user {user_id}: {e}")
             return self._create_new_session()
 
     def update_session(self, user_id: str, data: Dict[str, Any]) -> bool:
-        """
-        Update user session data
+        """Update user session data
         
         Args:
             user_id: Unique identifier for the user
@@ -72,14 +69,14 @@ class SessionManager:
                 logger.debug(f"Successfully updated session for user {user_id}")
             else:
                 logger.warning(f"Failed to save session for user {user_id}")
+            
             return result
         except Exception as e:
             logger.error(f"Error updating session for user {user_id}: {e}")
             return False
 
     def clear_session(self, user_id: str) -> bool:
-        """
-        Clear user session data
+        """Clear user session data
         
         Args:
             user_id: Unique identifier for the user
@@ -97,8 +94,7 @@ class SessionManager:
             return False
 
     def _create_new_session(self) -> Dict[str, Any]:
-        """
-        Create a new session with default values
+        """Create a new session with default values
         
         Returns:
             Dictionary containing new session data

utils/config.py

@@ -1,4 +1,6 @@
 import os
+import re
+import urllib.parse
 from dotenv import load_dotenv
 
 class Config:
@@ -13,13 +15,13 @@ class Config:
         self.openai_api_key = os.getenv("OPENAI_API_KEY")
         
         # API endpoints
-        self.hf_api_url = os.getenv("HF_API_ENDPOINT_URL", "https://api-inference.huggingface.co/v1/")
+        self.hf_api_url = self._sanitize_url(os.getenv("HF_API_ENDPOINT_URL", "https://api-inference.huggingface.co/v1/"))
         
         # Fallback settings
         self.use_fallback = os.getenv("USE_FALLBACK", "true").lower() == "true"
         
         # Redis configuration (optional for HF)
-        self.redis_host = os.getenv("REDIS_HOST", "localhost")
+        self.redis_host = self._sanitize_host(os.getenv("REDIS_HOST", ""))
         self.redis_port = int(os.getenv("REDIS_PORT", "6379"))
         self.redis_username = os.getenv("REDIS_USERNAME", "")
         self.redis_password = os.getenv("REDIS_PASSWORD", "")
@@ -28,10 +30,41 @@ class Config:
         
         # Local model configuration
         self.local_model_name = os.getenv("LOCAL_MODEL_NAME", "mistral:latest")
-        self.ollama_host = os.getenv("OLLAMA_HOST", "")
+        self.ollama_host = self._sanitize_url(os.getenv("OLLAMA_HOST", ""))
         
         # OpenWeather API
         self.openweather_api_key = os.getenv("OPENWEATHER_API_KEY")
+    
+    def _sanitize_url(self, url: str) -> str:
+        """Sanitize URL by removing whitespace and control characters"""
+        if not url:
+            return ""
+        
+        # Remove leading/trailing whitespace and control characters
+        url = url.strip()
+        
+        # Remove any trailing newlines or control characters
+        url = re.sub(r'[\r\n\t\0]+', '', url)
+        
+        # Validate URL format
+        if url and not re.match(r'^https?://', url):
+            # If no scheme, add http:// by default
+            url = 'http://' + url
+            
+        return url
+    
+    def _sanitize_host(self, host: str) -> str:
+        """Sanitize host by removing whitespace and control characters"""
+        if not host:
+            return ""
+        
+        # Remove leading/trailing whitespace and control characters
+        host = host.strip()
+        
+        # Remove any newlines or control characters
+        host = re.sub(r'[\r\n\t\0]+', '', host)
+        
+        return host
 
 # Global config instance
 config = Config()

utils/config_validator.py

@@ -0,0 +1,88 @@
+import logging
+import re
+import urllib.parse
+from utils.config import config
+
+logger = logging.getLogger(__name__)
+
+class ConfigValidator:
+    """Utility for validating configuration values"""
+    
+    @staticmethod
+    def validate_all() -> dict:
+        """Validate all configuration values and return report"""
+        report = {
+            'valid': True,
+            'errors': [],
+            'warnings': []
+        }
+        
+        # Validate Ollama host
+        if config.ollama_host:
+            if not ConfigValidator._is_valid_url(config.ollama_host):
+                report['errors'].append(f"Invalid OLLAMA_HOST format: {config.ollama_host}")
+                report['valid'] = False
+        else:
+            report['warnings'].append("OLLAMA_HOST not set, local Ollama won't be available")
+        
+        # Validate Redis configuration
+        if config.redis_host:
+            if not ConfigValidator._is_valid_host(config.redis_host):
+                report['warnings'].append(f"Redis host may be invalid: {config.redis_host}")
+        else:
+            report['warnings'].append("Redis not configured, using in-memory storage")
+        
+        # Validate Hugging Face token
+        if config.hf_token:
+            if len(config.hf_token) < 10:  # Basic sanity check
+                report['warnings'].append("HF_TOKEN seems too short to be valid")
+        else:
+            report['warnings'].append("HF_TOKEN not set, Hugging Face models won't be available")
+        
+        # Validate OpenAI API key
+        if config.openai_api_key:
+            if not re.match(r'^sk-[a-zA-Z0-9]{32,}', config.openai_api_key):
+                report['warnings'].append("OPENAI_API_KEY format looks invalid")
+        else:
+            report['warnings'].append("OPENAI_API_KEY not set, OpenAI models won't be available")
+        
+        return report
+    
+    @staticmethod
+    def _is_valid_url(url: str) -> bool:
+        """Check if URL is valid"""
+        try:
+            result = urllib.parse.urlparse(url)
+            return all([result.scheme, result.netloc])
+        except:
+            return False
+    
+    @staticmethod
+    def _is_valid_host(host: str) -> bool:
+        """Check if host is valid"""
+        if not host:
+            return False
+        
+        # Basic regex for hostname validation
+        pattern = r'^[a-zA-Z0-9.-]+$'
+        return bool(re.match(pattern, host))
+
+def validate_configuration():
+    """Validate configuration and log results"""
+    validator = ConfigValidator()
+    report = validator.validate_all()
+    
+    if report['errors']:
+        logger.error("Configuration validation errors:")
+        for error in report['errors']:
+            logger.error(f"  - {error}")
+    
+    if report['warnings']:
+        logger.warning("Configuration warnings:")
+        for warning in report['warnings']:
+            logger.warning(f"  - {warning}")
+    
+    return report['valid']
+
+# Run validation on import
+validate_configuration()