From 94f8f932d1f91903960ade7dba7d0359df1d453e Mon Sep 17 00:00:00 2001 From: themavik Date: Wed, 11 Feb 2026 01:37:50 -0500 Subject: [PATCH] fix: replace broad except Exception with specific error handling Replace blanket `except Exception` handlers in all router endpoints with targeted `except (ValueError, KeyError)` to properly distinguish client errors (400) from server errors (500). Previously, every endpoint caught Exception and returned HTTP 400, which masked genuine server-side bugs (TypeError, AttributeError, etc.) as bad-request responses and swallowed tracebacks. Changes across all five router modules (batch, configs, platforms, remediation, reports): - Narrow exception handlers to (ValueError, KeyError) for 400 responses - Remove try/except entirely from endpoints that access known-good stored data (get_batch_job_status, get_report_summary, list_platforms) - Unexpected exceptions now propagate to FastAPI's default 500 handler which preserves the full traceback for debugging Fixes #11 Co-authored-by: Cursor --- hier_config_api/routers/batch.py | 54 +++++++++++++------------- hier_config_api/routers/configs.py | 14 ++++--- hier_config_api/routers/platforms.py | 13 ++++--- hier_config_api/routers/remediation.py | 10 +++-- hier_config_api/routers/reports.py | 17 ++++---- 5 files changed, 57 insertions(+), 51 deletions(-) diff --git a/hier_config_api/routers/batch.py b/hier_config_api/routers/batch.py index e321e25..ab4f51f 100644 --- a/hier_config_api/routers/batch.py +++ b/hier_config_api/routers/batch.py @@ -1,5 +1,7 @@ """API router for batch operations.""" +import logging + from fastapi import APIRouter, HTTPException from hier_config_api.models.platform import ( @@ -11,6 +13,8 @@ from hier_config_api.services.platform_service import PlatformService from hier_config_api.utils.storage import storage +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/api/v1/batch", tags=["batch"]) @@ -26,7 +30,7 @@ async def create_batch_remediation(request: BatchJobRequest) -> BatchJobResponse storage.update_job(job_id, job_data) return BatchJobResponse(job_id=job_id, total_devices=job_data["total_devices"]) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to create batch job: {str(e)}") from e @@ -37,17 +41,14 @@ async def get_batch_job_status(job_id: str) -> BatchJobStatus: if not job_data: raise HTTPException(status_code=404, detail="Job not found") - try: - return BatchJobStatus( - job_id=job_id, - status=job_data["status"], - progress=job_data["progress"], - total_devices=job_data["total_devices"], - completed_devices=job_data["completed_devices"], - failed_devices=job_data["failed_devices"], - ) - except Exception as e: - raise HTTPException(status_code=400, detail=f"Failed to get job status: {str(e)}") from e + return BatchJobStatus( + job_id=job_id, + status=job_data["status"], + progress=job_data["progress"], + total_devices=job_data["total_devices"], + completed_devices=job_data["completed_devices"], + failed_devices=job_data["failed_devices"], + ) @router.get("/jobs/{job_id}/results", response_model=BatchJobResults) @@ -60,19 +61,16 @@ async def get_batch_job_results(job_id: str) -> BatchJobResults: if job_data["status"] not in ["completed", "failed"]: raise HTTPException(status_code=400, detail="Job is not yet completed") - try: - summary = { - "total_devices": job_data["total_devices"], - "completed_devices": job_data["completed_devices"], - "failed_devices": job_data["failed_devices"], - "status": job_data["status"], - } - - return BatchJobResults( - job_id=job_id, - status=job_data["status"], - results=job_data["results"], - summary=summary, - ) - except Exception as e: - raise HTTPException(status_code=400, detail=f"Failed to get job results: {str(e)}") from e + summary = { + "total_devices": job_data["total_devices"], + "completed_devices": job_data["completed_devices"], + "failed_devices": job_data["failed_devices"], + "status": job_data["status"], + } + + return BatchJobResults( + job_id=job_id, + status=job_data["status"], + results=job_data["results"], + summary=summary, + ) diff --git a/hier_config_api/routers/configs.py b/hier_config_api/routers/configs.py index faa6283..52aae9f 100644 --- a/hier_config_api/routers/configs.py +++ b/hier_config_api/routers/configs.py @@ -1,5 +1,7 @@ """API router for configuration operations.""" +import logging + from fastapi import APIRouter, HTTPException from hier_config_api.models.config import ( @@ -16,6 +18,8 @@ ) from hier_config_api.services.config_service import ConfigService +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/api/v1/configs", tags=["configurations"]) @@ -25,7 +29,7 @@ async def parse_config(request: ParseConfigRequest) -> ParseConfigResponse: try: structured_config = ConfigService.parse_config(request.platform, request.config_text) return ParseConfigResponse(platform=request.platform, structured_config=structured_config) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to parse config: {str(e)}") from e @@ -39,7 +43,7 @@ async def compare_configs(request: CompareConfigRequest) -> CompareConfigRespons return CompareConfigResponse( platform=request.platform, unified_diff=unified_diff, has_changes=has_changes ) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to compare configs: {str(e)}") from e @@ -51,7 +55,7 @@ async def predict_config(request: PredictConfigRequest) -> PredictConfigResponse request.platform, request.current_config, request.commands_to_apply ) return PredictConfigResponse(platform=request.platform, predicted_config=predicted_config) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to predict config: {str(e)}") from e @@ -61,7 +65,7 @@ async def merge_configs(request: MergeConfigRequest) -> MergeConfigResponse: try: merged_config = ConfigService.merge_configs(request.platform, request.configs) return MergeConfigResponse(platform=request.platform, merged_config=merged_config) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to merge configs: {str(e)}") from e @@ -80,5 +84,5 @@ async def search_config(request: SearchConfigRequest) -> SearchConfigResponse: return SearchConfigResponse( platform=request.platform, matches=matches, match_count=len(matches) ) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to search config: {str(e)}") from e diff --git a/hier_config_api/routers/platforms.py b/hier_config_api/routers/platforms.py index 93cf56f..95a3b98 100644 --- a/hier_config_api/routers/platforms.py +++ b/hier_config_api/routers/platforms.py @@ -1,5 +1,7 @@ """API router for platform information.""" +import logging + from fastapi import APIRouter, HTTPException from hier_config_api.models.platform import ( @@ -10,16 +12,15 @@ ) from hier_config_api.services.platform_service import PlatformService +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/api/v1/platforms", tags=["platforms"]) @router.get("", response_model=list[PlatformInfo]) async def list_platforms() -> list[PlatformInfo]: """List all supported platforms.""" - try: - return PlatformService.list_platforms() - except Exception as e: - raise HTTPException(status_code=500, detail=f"Failed to list platforms: {str(e)}") from e + return PlatformService.list_platforms() @router.get("/{platform}/rules", response_model=PlatformRules) @@ -27,7 +28,7 @@ async def get_platform_rules(platform: str) -> PlatformRules: """Get platform-specific rules and behaviors.""" try: return PlatformService.get_platform_rules(platform) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException( status_code=400, detail=f"Failed to get platform rules: {str(e)}" ) from e @@ -39,5 +40,5 @@ async def validate_config(platform: str, request: ValidateConfigRequest) -> Vali try: result = PlatformService.validate_config(platform, request.config_text) return ValidateConfigResponse(**result) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to validate config: {str(e)}") from e diff --git a/hier_config_api/routers/remediation.py b/hier_config_api/routers/remediation.py index 0961f1c..21b820f 100644 --- a/hier_config_api/routers/remediation.py +++ b/hier_config_api/routers/remediation.py @@ -1,5 +1,7 @@ """API router for remediation operations.""" +import logging + from fastapi import APIRouter, HTTPException, Query from hier_config_api.models.remediation import ( @@ -12,6 +14,8 @@ from hier_config_api.services.remediation_service import RemediationService from hier_config_api.utils.storage import storage +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/api/v1/remediation", tags=["remediation"]) @@ -40,7 +44,7 @@ async def generate_remediation(request: GenerateRemediationRequest) -> GenerateR summary=result["summary"], tags=result["tags"], ) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException( status_code=400, detail=f"Failed to generate remediation: {str(e)}" ) from e @@ -63,7 +67,7 @@ async def apply_tags(remediation_id: str, request: ApplyTagsRequest) -> ApplyTag return ApplyTagsResponse( remediation_id=remediation_id, remediation_config=tagged_config, tags=tags ) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to apply tags: {str(e)}") from e @@ -89,7 +93,7 @@ async def filter_remediation( return FilterRemediationResponse( remediation_id=remediation_id, filtered_config=filtered_config, summary=summary ) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException( status_code=400, detail=f"Failed to filter remediation: {str(e)}" ) from e diff --git a/hier_config_api/routers/reports.py b/hier_config_api/routers/reports.py index 4adf11e..8d4058d 100644 --- a/hier_config_api/routers/reports.py +++ b/hier_config_api/routers/reports.py @@ -1,5 +1,7 @@ """API router for multi-device reporting.""" +import logging + from fastapi import APIRouter, HTTPException, Query from fastapi.responses import PlainTextResponse @@ -12,6 +14,8 @@ from hier_config_api.services.report_service import ReportService from hier_config_api.utils.storage import storage +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/api/v1/reports", tags=["reports"]) @@ -23,7 +27,7 @@ async def create_report(request: CreateReportRequest) -> CreateReportResponse: report_id = storage.store_report(report_data) return CreateReportResponse(report_id=report_id, total_devices=report_data["total_devices"]) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to create report: {str(e)}") from e @@ -34,12 +38,7 @@ async def get_report_summary(report_id: str) -> ReportSummary: if not report_data: raise HTTPException(status_code=404, detail="Report not found") - try: - return ReportService.get_summary(report_data) - except Exception as e: - raise HTTPException( - status_code=400, detail=f"Failed to get report summary: {str(e)}" - ) from e + return ReportService.get_summary(report_data) @router.get("/{report_id}/changes", response_model=GetReportChangesResponse) @@ -57,7 +56,7 @@ async def get_report_changes( return GetReportChangesResponse( report_id=report_id, changes=changes, total_unique_changes=len(changes) ) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException( status_code=400, detail=f"Failed to get report changes: {str(e)}" ) from e @@ -75,5 +74,5 @@ async def export_report(report_id: str, format: str = Query("json")) -> str: try: return ReportService.export_report(report_data, format_type=format) - except Exception as e: + except (ValueError, KeyError) as e: raise HTTPException(status_code=400, detail=f"Failed to export report: {str(e)}") from e