From eca584ea8ab18c2397d03f23e3801d3a5fef6dca Mon Sep 17 00:00:00 2001 From: SyedaAnshrahGillani Date: Wed, 23 Jul 2025 15:02:36 +0500 Subject: [PATCH] fix: improve error handling, database session management, and config validation - Add missing SQLAlchemy exception handler registration in FastAPI app - Fix critical bug in database settings class initialization - Improve config validation with proper defaults and validators - Use correct HTTP status codes (404 for not found, 422 for parsing errors) - Enhance exception logging while preventing information leakage - Add automatic secure session key generation if not provided - Improve overall application robustness and security These changes address several critical issues: 1. Missing database exception handling that could cause unhandled errors 2. Improper HTTP status codes that violate REST API standards 3. Configuration validation gaps that could cause runtime failures 4. Security improvements for session management 5. Better error logging for debugging while maintaining security --- apps/backend/app/api/router/v1/resume.py | 16 ++++++------- apps/backend/app/base.py | 3 +++ apps/backend/app/core/config.py | 30 +++++++++++++++++++++--- apps/backend/app/core/database.py | 15 ++++++------ apps/backend/app/core/exceptions.py | 12 +++++++++- 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/apps/backend/app/api/router/v1/resume.py b/apps/backend/app/api/router/v1/resume.py index f1cdaacf..8e154705 100644 --- a/apps/backend/app/api/router/v1/resume.py +++ b/apps/backend/app/api/router/v1/resume.py @@ -156,27 +156,27 @@ async def score_and_improve( headers=headers, ) except ResumeNotFoundError as e: - logger.error(str(e)) + logger.warning(f"Resume not found: {str(e)}") raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_404_NOT_FOUND, detail=str(e), ) except JobNotFoundError as e: - logger.error(str(e)) + logger.warning(f"Job not found: {str(e)}") raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_404_NOT_FOUND, detail=str(e), ) except ResumeParsingError as e: - logger.error(str(e)) + logger.error(f"Resume parsing error: {str(e)}") raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e), ) except JobParsingError as e: - logger.error(str(e)) + logger.error(f"Job parsing error: {str(e)}") raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e), ) except ResumeKeywordExtractionError as e: diff --git a/apps/backend/app/base.py b/apps/backend/app/base.py index eed071fa..55dd30c9 100644 --- a/apps/backend/app/base.py +++ b/apps/backend/app/base.py @@ -6,6 +6,7 @@ from fastapi.staticfiles import StaticFiles from fastapi.middleware.cors import CORSMiddleware from starlette.middleware.sessions import SessionMiddleware +from sqlalchemy.exc import SQLAlchemyError from .api import health_check, v1_router, RequestIDMiddleware from .core import ( @@ -15,6 +16,7 @@ custom_http_exception_handler, validation_exception_handler, unhandled_exception_handler, + sqlalchemy_exception_handler, ) from .models import Base @@ -54,6 +56,7 @@ def create_app() -> FastAPI: app.add_exception_handler(HTTPException, custom_http_exception_handler) app.add_exception_handler(RequestValidationError, validation_exception_handler) + app.add_exception_handler(SQLAlchemyError, sqlalchemy_exception_handler) app.add_exception_handler(Exception, unhandled_exception_handler) if os.path.exists(settings.FRONTEND_PATH): diff --git a/apps/backend/app/core/config.py b/apps/backend/app/core/config.py index 1fc49e36..2ab58950 100644 --- a/apps/backend/app/core/config.py +++ b/apps/backend/app/core/config.py @@ -1,6 +1,8 @@ import os import sys import logging +import secrets +from pydantic import Field, validator from pydantic_settings import BaseSettings, SettingsConfigDict from typing import List, Optional, Literal @@ -9,17 +11,39 @@ class Settings(BaseSettings): PROJECT_NAME: str = "Resume Matcher" FRONTEND_PATH: str = os.path.join(os.path.dirname(__file__), "frontend", "assets") ALLOWED_ORIGINS: List[str] = ["http://localhost:3000", "http://127.0.0.1:3000"] - SYNC_DATABASE_URL: Optional[str] - ASYNC_DATABASE_URL: Optional[str] - SESSION_SECRET_KEY: Optional[str] + SYNC_DATABASE_URL: Optional[str] = Field(default="sqlite:///./resume_matcher.db") + ASYNC_DATABASE_URL: Optional[str] = Field(default="sqlite+aiosqlite:///./resume_matcher.db") + SESSION_SECRET_KEY: Optional[str] = Field(default=None) DB_ECHO: bool = False PYTHONDONTWRITEBYTECODE: int = 1 + ENV: str = Field(default="production") model_config = SettingsConfigDict( env_file=os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, ".env"), env_file_encoding="utf-8", ) + @validator('SESSION_SECRET_KEY') + def validate_secret_key(cls, v): + if not v: + # Generate a secure random secret key if not provided + return secrets.token_urlsafe(32) + if len(v) < 32: + raise ValueError('SESSION_SECRET_KEY must be at least 32 characters long') + return v + + @validator('SYNC_DATABASE_URL') + def validate_sync_db_url(cls, v): + if not v: + raise ValueError('SYNC_DATABASE_URL is required') + return v + + @validator('ASYNC_DATABASE_URL') + def validate_async_db_url(cls, v): + if not v: + raise ValueError('ASYNC_DATABASE_URL is required') + return v + settings = Settings() diff --git a/apps/backend/app/core/database.py b/apps/backend/app/core/database.py index 919108d3..3675c596 100644 --- a/apps/backend/app/core/database.py +++ b/apps/backend/app/core/database.py @@ -20,13 +20,14 @@ class _DatabaseSettings: """Pulled from environment once at import-time.""" - SYNC_DATABASE_URL: str = settings.SYNC_DATABASE_URL - ASYNC_DATABASE_URL: str = settings.ASYNC_DATABASE_URL - DB_ECHO: bool = settings.DB_ECHO - - DB_CONNECT_ARGS = ( - {"check_same_thread": False} if SYNC_DATABASE_URL.startswith("sqlite") else {} - ) + def __init__(self): + self.SYNC_DATABASE_URL: str = settings.SYNC_DATABASE_URL + self.ASYNC_DATABASE_URL: str = settings.ASYNC_DATABASE_URL + self.DB_ECHO: bool = settings.DB_ECHO + + self.DB_CONNECT_ARGS = ( + {"check_same_thread": False} if self.SYNC_DATABASE_URL.startswith("sqlite") else {} + ) settings = _DatabaseSettings() diff --git a/apps/backend/app/core/exceptions.py b/apps/backend/app/core/exceptions.py index 76c36859..8b08f0eb 100644 --- a/apps/backend/app/core/exceptions.py +++ b/apps/backend/app/core/exceptions.py @@ -27,9 +27,19 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE async def unhandled_exception_handler(request: Request, exc: Exception): request_id = getattr(request.state, "request_id", "") + + # Log the full exception details for debugging + logger.error( + f"Unhandled exception for request {request_id} to {request.url}: {exc.__class__.__name__}: {str(exc)}", + exc_info=True + ) + return JSONResponse( status_code=HTTP_500_INTERNAL_SERVER_ERROR, - content={"detail": "Internal Server Error", "request_id": request_id}, + content={ + "detail": "An internal error occurred. Please try again later.", + "request_id": request_id + }, )