From 2c0bfe1bdbccd3ced8ad5e6915eea36d57f0500e Mon Sep 17 00:00:00 2001 From: duongynhi000005-oss Date: Sun, 31 May 2026 14:48:24 +0000 Subject: [PATCH] fix: fix: fix: fix: fix: fix: fix: [ Bounty $3k ] [ API ] Add structured erro --- api/main.py | 43 +++++++++--- api/middleware/error_handler.py | 116 +++++++++++++++++++++++++++++++ api/middleware/ratelimit.py | 14 ++-- api/middleware/request_id.py | 17 +++++ api/models/errors.py | 43 ++++++++++++ test/conftest.py | 10 +++ test/test_api_error_responses.py | 41 +++++++++++ 7 files changed, 272 insertions(+), 12 deletions(-) create mode 100644 api/middleware/error_handler.py create mode 100644 api/middleware/request_id.py create mode 100644 api/models/errors.py create mode 100644 test/conftest.py create mode 100644 test/test_api_error_responses.py diff --git a/api/main.py b/api/main.py index a65b3c29..4428bd7f 100644 --- a/api/main.py +++ b/api/main.py @@ -1,13 +1,40 @@ from fastapi import FastAPI, HTTPException, Query +from fastapi.exceptions import RequestValidationError +from starlette.exceptions import HTTPException as StarletteHTTPException from pydantic import BaseModel from typing import Optional from datetime import datetime +try: + from .middleware.error_handler import ( + api_error_handler, + http_error_handler, + unhandled_error_handler, + validation_error_handler, + ) + from .middleware.request_id import RequestIDMiddleware + from .models.errors import APIError, NotFoundError +except ImportError: # Allows `cd api && uvicorn main:app`. + from middleware.error_handler import ( + api_error_handler, + http_error_handler, + unhandled_error_handler, + validation_error_handler, + ) + from middleware.request_id import RequestIDMiddleware + from models.errors import APIError, NotFoundError + app = FastAPI( title="OpenAgents API", description="Off-chain indexer and agent discovery API for the OpenAgents protocol", version="0.1.0", ) +app.add_middleware(RequestIDMiddleware) +app.add_exception_handler(APIError, api_error_handler) +app.add_exception_handler(HTTPException, http_error_handler) +app.add_exception_handler(StarletteHTTPException, http_error_handler) +app.add_exception_handler(RequestValidationError, validation_error_handler) +app.add_exception_handler(Exception, unhandled_error_handler) class AgentResponse(BaseModel): @@ -47,9 +74,9 @@ class LeaderboardEntry(BaseModel): @app.get("/agents", response_model=list[AgentResponse]) async def list_agents( active_only: bool = Query(True), - min_reputation: int = Query(0), - limit: int = Query(50, le=100), - offset: int = Query(0), + min_reputation: int = Query(0, ge=0), + limit: int = Query(50, ge=1, le=100), + offset: int = Query(0, ge=0), ): results = list(agents_cache.values()) if active_only: @@ -61,15 +88,15 @@ async def list_agents( @app.get("/agents/{agent_id}", response_model=AgentResponse) async def get_agent(agent_id: str): if agent_id not in agents_cache: - raise HTTPException(status_code=404, detail="Agent not found") + raise NotFoundError("Agent not found", {"agent_id": agent_id}) return agents_cache[agent_id] @app.get("/tasks", response_model=list[TaskResponse]) async def list_tasks( status: Optional[str] = Query(None), - limit: int = Query(50, le=100), - offset: int = Query(0), + limit: int = Query(50, ge=1, le=100), + offset: int = Query(0, ge=0), ): results = list(tasks_cache.values()) if status: @@ -80,12 +107,12 @@ async def list_tasks( @app.get("/tasks/{task_id}", response_model=TaskResponse) async def get_task(task_id: int): if task_id not in tasks_cache: - raise HTTPException(status_code=404, detail="Task not found") + raise NotFoundError("Task not found", {"task_id": task_id}) return tasks_cache[task_id] @app.get("/leaderboard", response_model=list[LeaderboardEntry]) -async def leaderboard(limit: int = Query(20, le=50)): +async def leaderboard(limit: int = Query(20, ge=1, le=50)): entries = [] for agent in agents_cache.values(): completed = agent.get("tasks_completed", 0) diff --git a/api/middleware/error_handler.py b/api/middleware/error_handler.py new file mode 100644 index 00000000..97394e82 --- /dev/null +++ b/api/middleware/error_handler.py @@ -0,0 +1,116 @@ +"""Exception handlers for structured API error responses.""" + +import logging +from typing import Any + +from fastapi import HTTPException, Request +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse +from starlette.exceptions import HTTPException as StarletteHTTPException + +try: + from ..models.errors import APIError, ErrorDetail, ErrorResponse +except ImportError: # Allows `cd api && uvicorn main:app`. + from models.errors import APIError, ErrorDetail, ErrorResponse + + +logger = logging.getLogger(__name__) + + +def status_to_code(status_code: int) -> str: + return { + 400: "BAD_REQUEST", + 401: "AUTHENTICATION_ERROR", + 403: "FORBIDDEN", + 404: "NOT_FOUND", + 405: "METHOD_NOT_ALLOWED", + 409: "CONFLICT", + 422: "VALIDATION_ERROR", + 429: "RATE_LIMIT_EXCEEDED", + 500: "INTERNAL_SERVER_ERROR", + 503: "SERVICE_UNAVAILABLE", + }.get(status_code, "HTTP_ERROR") + + +def build_error_response( + *, + status_code: int, + code: str, + message: str, + request_id: str | None = None, + details: dict[str, Any] | None = None, + headers: dict[str, str] | None = None, +) -> JSONResponse: + error = ErrorDetail( + code=code, + message=message, + details=details, + request_id=request_id, + ) + body = ErrorResponse( + error=error, + request_id=request_id, + code=code, + message=message, + details=details, + ).model_dump(exclude_none=True) + response = JSONResponse(status_code=status_code, content=body, headers=headers) + if request_id: + response.headers["X-Request-ID"] = request_id + return response + + +async def api_error_handler(request: Request, exc: APIError) -> JSONResponse: + return build_error_response( + status_code=exc.status_code, + code=exc.code, + message=exc.message, + details=exc.details, + request_id=getattr(request.state, "request_id", None), + ) + + +async def http_error_handler( + request: Request, exc: HTTPException | StarletteHTTPException +) -> JSONResponse: + detail = exc.detail if isinstance(exc.detail, str) else "HTTP error" + details = None if isinstance(exc.detail, str) else {"detail": exc.detail} + return build_error_response( + status_code=exc.status_code, + code=status_to_code(exc.status_code), + message=detail, + details=details, + request_id=getattr(request.state, "request_id", None), + headers=getattr(exc, "headers", None), + ) + + +async def validation_error_handler(request: Request, exc: RequestValidationError) -> JSONResponse: + details = { + "validation_errors": [ + { + "field": ".".join(str(part) for part in error["loc"]), + "message": error["msg"], + "type": error["type"], + } + for error in exc.errors() + ] + } + return build_error_response( + status_code=422, + code="VALIDATION_ERROR", + message="Request validation failed", + details=details, + request_id=getattr(request.state, "request_id", None), + ) + + +async def unhandled_error_handler(request: Request, exc: Exception) -> JSONResponse: + logger.exception("Unhandled API exception", exc_info=exc) + return build_error_response( + status_code=500, + code="INTERNAL_SERVER_ERROR", + message="Internal server error", + request_id=getattr(request.state, "request_id", None), + ) + diff --git a/api/middleware/ratelimit.py b/api/middleware/ratelimit.py index 379fb205..b6d015ba 100644 --- a/api/middleware/ratelimit.py +++ b/api/middleware/ratelimit.py @@ -4,9 +4,13 @@ from collections import defaultdict from fastapi import Request, HTTPException from starlette.middleware.base import BaseHTTPMiddleware -from starlette.responses import JSONResponse from typing import Dict, Tuple +try: + from .error_handler import build_error_response +except ImportError: # Allows `cd api && uvicorn main:app`. + from middleware.error_handler import build_error_response + class RateLimitConfig: def __init__( @@ -65,12 +69,14 @@ async def dispatch(self, request: Request, call_next): is_limited, value = self._is_rate_limited(client_ip) if is_limited: - return JSONResponse( + return build_error_response( status_code=429, - content={ - "error": "Rate limit exceeded", + code="RATE_LIMIT_EXCEEDED", + message="Rate limit exceeded", + details={ "retry_after": value, }, + request_id=getattr(request.state, "request_id", None), headers={"Retry-After": str(value)}, ) diff --git a/api/middleware/request_id.py b/api/middleware/request_id.py new file mode 100644 index 00000000..15d90f55 --- /dev/null +++ b/api/middleware/request_id.py @@ -0,0 +1,17 @@ +"""Request ID middleware for API tracing.""" + +import uuid + +from fastapi import Request +from starlette.middleware.base import BaseHTTPMiddleware + + +class RequestIDMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request: Request, call_next): + request_id = request.headers.get("X-Request-ID") or str(uuid.uuid4()) + request.state.request_id = request_id + + response = await call_next(request) + response.headers["X-Request-ID"] = request_id + return response + diff --git a/api/models/errors.py b/api/models/errors.py new file mode 100644 index 00000000..a4d98bd1 --- /dev/null +++ b/api/models/errors.py @@ -0,0 +1,43 @@ +"""Structured API error response helpers.""" + +from typing import Any, Optional + +from pydantic import BaseModel + + +class ErrorDetail(BaseModel): + code: str + message: str + details: Optional[dict[str, Any]] = None + request_id: Optional[str] = None + + +class ErrorResponse(BaseModel): + error: ErrorDetail + request_id: Optional[str] = None + code: str + message: str + details: Optional[dict[str, Any]] = None + + +class APIError(Exception): + """Application exception with a stable machine-readable code.""" + + def __init__( + self, + code: str, + message: str, + status_code: int = 500, + details: Optional[dict[str, Any]] = None, + ): + super().__init__(message) + self.code = code + self.message = message + self.status_code = status_code + self.details = details + + +class NotFoundError(APIError): + def __init__(self, message: str = "Resource not found", details: Optional[dict[str, Any]] = None): + super().__init__("NOT_FOUND", message, 404, details) + diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 00000000..97036d84 --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,10 @@ +"""Pytest path setup for repository-local Python packages.""" + +import sys +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + diff --git a/test/test_api_error_responses.py b/test/test_api_error_responses.py new file mode 100644 index 00000000..7397ed63 --- /dev/null +++ b/test/test_api_error_responses.py @@ -0,0 +1,41 @@ +"""Tests for structured API error responses.""" + +from fastapi.testclient import TestClient + +from api.main import app + + +client = TestClient(app) + + +def test_not_found_errors_are_structured(): + response = client.get("/agents/missing-agent") + + assert response.status_code == 404 + body = response.json() + assert body["error"]["code"] == "NOT_FOUND" + assert body["error"]["message"] == "Agent not found" + assert body["error"]["details"] == {"agent_id": "missing-agent"} + assert body["request_id"] == response.headers["X-Request-ID"] + assert "detail" not in body + + +def test_validation_errors_include_field_details(): + response = client.get("/agents?limit=-1&offset=-1") + + assert response.status_code == 422 + body = response.json() + assert body["error"]["code"] == "VALIDATION_ERROR" + fields = {error["field"] for error in body["error"]["details"]["validation_errors"]} + assert "query.limit" in fields + assert "query.offset" in fields + assert "detail" not in body + + +def test_custom_request_id_is_preserved_on_errors(): + response = client.get("/tasks/999", headers={"X-Request-ID": "req-test-123"}) + + assert response.status_code == 404 + assert response.headers["X-Request-ID"] == "req-test-123" + assert response.json()["error"]["request_id"] == "req-test-123" +