mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
fix error handling
This commit is contained in:
@@ -8,6 +8,7 @@ from fastapi import FastAPI
|
||||
|
||||
from backend.api.external.middleware import add_auth_responses_to_openapi
|
||||
from backend.api.middleware.security import SecurityHeadersMiddleware
|
||||
from backend.api.utils.exceptions import add_exception_handlers
|
||||
from backend.api.utils.openapi import sort_openapi
|
||||
|
||||
from .routes import v1_router
|
||||
@@ -40,6 +41,9 @@ v1_app = FastAPI(
|
||||
v1_app.add_middleware(SecurityHeadersMiddleware)
|
||||
v1_app.include_router(v1_router)
|
||||
|
||||
# Mounted sub-apps do NOT inherit exception handlers from the parent app.
|
||||
add_exception_handlers(v1_app)
|
||||
|
||||
# Add 401 responses to authenticated endpoints in OpenAPI spec
|
||||
add_auth_responses_to_openapi(v1_app)
|
||||
# Sort OpenAPI schema to eliminate diff on refactors
|
||||
|
||||
@@ -8,6 +8,7 @@ from fastapi import FastAPI
|
||||
|
||||
from backend.api.external.middleware import add_auth_responses_to_openapi
|
||||
from backend.api.middleware.security import SecurityHeadersMiddleware
|
||||
from backend.api.utils.exceptions import add_exception_handlers
|
||||
from backend.api.utils.openapi import sort_openapi
|
||||
|
||||
from .mcp_server import create_mcp_app
|
||||
@@ -84,6 +85,10 @@ v2_app = FastAPI(
|
||||
v2_app.add_middleware(SecurityHeadersMiddleware)
|
||||
v2_app.include_router(v2_router)
|
||||
|
||||
# Mounted sub-apps do NOT inherit exception handlers from the parent app,
|
||||
# so we must register them here for the v2 API specifically.
|
||||
add_exception_handlers(v2_app)
|
||||
|
||||
# Mount MCP server (Copilot tools via Streamable HTTP)
|
||||
v2_app.mount("/mcp", create_mcp_app())
|
||||
|
||||
|
||||
276
autogpt_platform/backend/backend/api/external/v2/app_test.py
vendored
Normal file
276
autogpt_platform/backend/backend/api/external/v2/app_test.py
vendored
Normal file
@@ -0,0 +1,276 @@
|
||||
"""
|
||||
Tests for v2 API error handling behavior.
|
||||
|
||||
The v2 app registers its own exception handlers (since mounted sub-apps don't
|
||||
inherit handlers from the parent app). These tests verify that exceptions from
|
||||
the DB/service layer are correctly mapped to HTTP status codes.
|
||||
|
||||
We construct a lightweight test app rather than importing the full v2_app,
|
||||
because the latter eagerly loads the MCP server, block registry, and other
|
||||
heavy dependencies that are irrelevant for error handling tests.
|
||||
"""
|
||||
|
||||
import json
|
||||
from datetime import datetime, timezone
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import fastapi
|
||||
import fastapi.testclient
|
||||
import pytest
|
||||
import pytest_mock
|
||||
from prisma.enums import APIKeyPermission
|
||||
from pytest_snapshot.plugin import Snapshot
|
||||
|
||||
from backend.api.external.middleware import require_auth
|
||||
from backend.api.utils.exceptions import add_exception_handlers
|
||||
from backend.data.auth.base import APIAuthorizationInfo
|
||||
from backend.util.exceptions import DatabaseError, NotFoundError
|
||||
|
||||
from .library.agents import agents_router
|
||||
from .marketplace import marketplace_router
|
||||
|
||||
TEST_USER_ID = "test-user-id"
|
||||
|
||||
_mock_auth = APIAuthorizationInfo(
|
||||
user_id=TEST_USER_ID,
|
||||
scopes=list(APIKeyPermission),
|
||||
type="api_key",
|
||||
created_at=datetime.now(tz=timezone.utc),
|
||||
)
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Build a lightweight test app with the shared exception handlers
|
||||
# but only the routers we need for testing.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
app = fastapi.FastAPI()
|
||||
app.include_router(agents_router, prefix="/library")
|
||||
app.include_router(marketplace_router, prefix="/marketplace")
|
||||
add_exception_handlers(app)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _override_auth():
|
||||
"""Bypass API key / OAuth auth for all tests in this module."""
|
||||
|
||||
async def fake_auth() -> APIAuthorizationInfo:
|
||||
return _mock_auth
|
||||
|
||||
app.dependency_overrides[require_auth] = fake_auth
|
||||
yield
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
|
||||
client = fastapi.testclient.TestClient(app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# NotFoundError → 404
|
||||
# ============================================================================
|
||||
|
||||
|
||||
def test_not_found_error_returns_404(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
snapshot: Snapshot,
|
||||
) -> None:
|
||||
"""NotFoundError raised by the DB layer should become a 404 response."""
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.get_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=NotFoundError("Agent #nonexistent not found"),
|
||||
)
|
||||
|
||||
response = client.get("/library/agents/nonexistent")
|
||||
|
||||
assert response.status_code == 404
|
||||
body = response.json()
|
||||
assert body["detail"] == "Agent #nonexistent not found"
|
||||
assert "message" in body
|
||||
assert body["hint"] == "Adjust the request and retry."
|
||||
|
||||
snapshot.snapshot_dir = "snapshots"
|
||||
snapshot.assert_match(
|
||||
json.dumps(body, indent=2, sort_keys=True),
|
||||
"v2_not_found_error_404",
|
||||
)
|
||||
|
||||
|
||||
def test_not_found_error_on_delete_returns_404(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
) -> None:
|
||||
"""NotFoundError on DELETE should return 404, not 204 or 500."""
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.delete_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=NotFoundError("Agent #gone not found"),
|
||||
)
|
||||
|
||||
response = client.delete("/library/agents/gone")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json()["detail"] == "Agent #gone not found"
|
||||
assert "message" in response.json()
|
||||
|
||||
|
||||
def test_not_found_error_on_marketplace_returns_404(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
) -> None:
|
||||
"""NotFoundError from store DB layer should become a 404."""
|
||||
mocker.patch(
|
||||
"backend.api.features.store.db.get_store_agent_by_version_id",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=NotFoundError("Store listing not found"),
|
||||
)
|
||||
|
||||
response = client.get("/marketplace/agents/by-version/nonexistent")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json()["detail"] == "Store listing not found"
|
||||
assert "message" in response.json()
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# ValueError → 400
|
||||
# ============================================================================
|
||||
|
||||
|
||||
def test_value_error_returns_400(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
snapshot: Snapshot,
|
||||
) -> None:
|
||||
"""ValueError raised by the service layer should become a 400 response."""
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.update_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=ValueError("Invalid graph version: -1"),
|
||||
)
|
||||
|
||||
response = client.patch(
|
||||
"/library/agents/some-id",
|
||||
json={"graph_version": -1},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = response.json()
|
||||
assert body["detail"] == "Invalid graph version: -1"
|
||||
assert "message" in body
|
||||
assert body["hint"] == "Adjust the request and retry."
|
||||
|
||||
snapshot.snapshot_dir = "snapshots"
|
||||
snapshot.assert_match(
|
||||
json.dumps(body, indent=2, sort_keys=True),
|
||||
"v2_value_error_400",
|
||||
)
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# NotFoundError is a ValueError subclass — verify specificity wins
|
||||
# ============================================================================
|
||||
|
||||
|
||||
def test_not_found_error_takes_precedence_over_value_error(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
) -> None:
|
||||
"""
|
||||
NotFoundError(ValueError) should match the NotFoundError handler (404),
|
||||
not the ValueError handler (400).
|
||||
"""
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.get_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=NotFoundError("Specific not found"),
|
||||
)
|
||||
|
||||
response = client.get("/library/agents/test-id")
|
||||
|
||||
# Must be 404, not 400
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Unhandled Exception → 500
|
||||
# ============================================================================
|
||||
|
||||
|
||||
def test_unhandled_exception_returns_500(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
snapshot: Snapshot,
|
||||
) -> None:
|
||||
"""
|
||||
Unexpected exceptions should return a generic 500 without leaking
|
||||
internal details.
|
||||
"""
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.get_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=DatabaseError("connection refused"),
|
||||
)
|
||||
|
||||
response = client.get("/library/agents/some-id")
|
||||
|
||||
assert response.status_code == 500
|
||||
body = response.json()
|
||||
assert "message" in body
|
||||
assert "detail" in body
|
||||
assert body["hint"] == "Check server logs and dependent services."
|
||||
|
||||
snapshot.snapshot_dir = "snapshots"
|
||||
snapshot.assert_match(
|
||||
json.dumps(body, indent=2, sort_keys=True),
|
||||
"v2_unhandled_exception_500",
|
||||
)
|
||||
|
||||
|
||||
def test_runtime_error_returns_500(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
) -> None:
|
||||
"""RuntimeError (not ValueError) should hit the catch-all 500 handler."""
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.delete_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=RuntimeError("something broke"),
|
||||
)
|
||||
|
||||
response = client.delete("/library/agents/some-id")
|
||||
|
||||
assert response.status_code == 500
|
||||
assert "detail" in response.json()
|
||||
assert response.json()["hint"] == "Check server logs and dependent services."
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Response format consistency
|
||||
# ============================================================================
|
||||
|
||||
|
||||
def test_all_error_responses_have_consistent_format(
|
||||
mocker: pytest_mock.MockFixture,
|
||||
) -> None:
|
||||
"""All error responses should use {"message": ..., "detail": ..., "hint": ...} format."""
|
||||
cases = [
|
||||
(NotFoundError("not found"), 404),
|
||||
(ValueError("bad value"), 400),
|
||||
(RuntimeError("boom"), 500),
|
||||
]
|
||||
|
||||
for exc, expected_status in cases:
|
||||
mocker.patch(
|
||||
"backend.api.features.library.db.get_library_agent",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=exc,
|
||||
)
|
||||
|
||||
response = client.get("/library/agents/test-id")
|
||||
|
||||
assert response.status_code == expected_status, (
|
||||
f"Expected {expected_status} for {type(exc).__name__}, "
|
||||
f"got {response.status_code}"
|
||||
)
|
||||
body = response.json()
|
||||
assert (
|
||||
"message" in body
|
||||
), f"Missing 'message' key for {type(exc).__name__}: {body}"
|
||||
assert (
|
||||
"detail" in body
|
||||
), f"Missing 'detail' key for {type(exc).__name__}: {body}"
|
||||
assert "hint" in body, f"Missing 'hint' key for {type(exc).__name__}: {body}"
|
||||
@@ -5,16 +5,12 @@ from enum import Enum
|
||||
from typing import Any, Optional
|
||||
|
||||
import fastapi
|
||||
import fastapi.responses
|
||||
import pydantic
|
||||
import starlette.middleware.cors
|
||||
import uvicorn
|
||||
from autogpt_libs.auth import add_auth_responses_to_openapi
|
||||
from autogpt_libs.auth import verify_settings as verify_auth_settings
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi.middleware.gzip import GZipMiddleware
|
||||
from fastapi.routing import APIRoute
|
||||
from prisma.errors import PrismaError
|
||||
|
||||
import backend.api.features.admin.credit_admin_routes
|
||||
import backend.api.features.admin.execution_analytics_routes
|
||||
@@ -41,22 +37,12 @@ import backend.data.user
|
||||
import backend.integrations.webhooks.utils
|
||||
import backend.util.service
|
||||
import backend.util.settings
|
||||
from backend.api.features.library.exceptions import (
|
||||
FolderAlreadyExistsError,
|
||||
FolderValidationError,
|
||||
)
|
||||
from backend.api.utils.exceptions import add_exception_handlers
|
||||
from backend.blocks.llm import DEFAULT_LLM_MODEL
|
||||
from backend.data.model import Credentials
|
||||
from backend.integrations.providers import ProviderName
|
||||
from backend.monitoring.instrumentation import instrument_fastapi
|
||||
from backend.util import json
|
||||
from backend.util.cloud_storage import shutdown_cloud_storage_handler
|
||||
from backend.util.exceptions import (
|
||||
MissingConfigError,
|
||||
NotAuthorizedError,
|
||||
NotFoundError,
|
||||
PreconditionFailed,
|
||||
)
|
||||
from backend.util.feature_flag import initialize_launchdarkly, shutdown_launchdarkly
|
||||
from backend.util.service import UnhealthyServiceError
|
||||
from backend.util.workspace_storage import shutdown_workspace_storage
|
||||
@@ -207,77 +193,7 @@ instrument_fastapi(
|
||||
)
|
||||
|
||||
|
||||
def handle_internal_http_error(status_code: int = 500, log_error: bool = True):
|
||||
def handler(request: fastapi.Request, exc: Exception):
|
||||
if log_error:
|
||||
logger.exception(
|
||||
"%s %s failed. Investigate and resolve the underlying issue: %s",
|
||||
request.method,
|
||||
request.url.path,
|
||||
exc,
|
||||
exc_info=exc,
|
||||
)
|
||||
|
||||
hint = (
|
||||
"Adjust the request and retry."
|
||||
if status_code < 500
|
||||
else "Check server logs and dependent services."
|
||||
)
|
||||
return fastapi.responses.JSONResponse(
|
||||
content={
|
||||
"message": f"Failed to process {request.method} {request.url.path}",
|
||||
"detail": str(exc),
|
||||
"hint": hint,
|
||||
},
|
||||
status_code=status_code,
|
||||
)
|
||||
|
||||
return handler
|
||||
|
||||
|
||||
async def validation_error_handler(
|
||||
request: fastapi.Request, exc: Exception
|
||||
) -> fastapi.responses.Response:
|
||||
logger.error(
|
||||
"Validation failed for %s %s: %s. Fix the request payload and try again.",
|
||||
request.method,
|
||||
request.url.path,
|
||||
exc,
|
||||
)
|
||||
errors: list | str
|
||||
if hasattr(exc, "errors"):
|
||||
errors = exc.errors() # type: ignore[call-arg]
|
||||
else:
|
||||
errors = str(exc)
|
||||
|
||||
response_content = {
|
||||
"message": f"Invalid data for {request.method} {request.url.path}",
|
||||
"detail": errors,
|
||||
"hint": "Ensure the request matches the API schema.",
|
||||
}
|
||||
|
||||
content_json = json.dumps(response_content)
|
||||
|
||||
return fastapi.responses.Response(
|
||||
content=content_json,
|
||||
status_code=422,
|
||||
media_type="application/json",
|
||||
)
|
||||
|
||||
|
||||
app.add_exception_handler(PrismaError, handle_internal_http_error(500))
|
||||
app.add_exception_handler(
|
||||
FolderAlreadyExistsError, handle_internal_http_error(409, False)
|
||||
)
|
||||
app.add_exception_handler(FolderValidationError, handle_internal_http_error(400, False))
|
||||
app.add_exception_handler(NotFoundError, handle_internal_http_error(404, False))
|
||||
app.add_exception_handler(NotAuthorizedError, handle_internal_http_error(403, False))
|
||||
app.add_exception_handler(RequestValidationError, validation_error_handler)
|
||||
app.add_exception_handler(pydantic.ValidationError, validation_error_handler)
|
||||
app.add_exception_handler(MissingConfigError, handle_internal_http_error(503))
|
||||
app.add_exception_handler(ValueError, handle_internal_http_error(400))
|
||||
app.add_exception_handler(PreconditionFailed, handle_internal_http_error(428))
|
||||
app.add_exception_handler(Exception, handle_internal_http_error(500))
|
||||
add_exception_handlers(app)
|
||||
|
||||
app.include_router(backend.api.features.v1.v1_router, tags=["v1"], prefix="/api")
|
||||
app.include_router(
|
||||
|
||||
117
autogpt_platform/backend/backend/api/utils/exceptions.py
Normal file
117
autogpt_platform/backend/backend/api/utils/exceptions.py
Normal file
@@ -0,0 +1,117 @@
|
||||
"""
|
||||
Shared exception handlers for FastAPI applications.
|
||||
|
||||
Provides a single `add_exception_handlers` function that registers a consistent
|
||||
set of exception-to-HTTP-status mappings on any FastAPI app instance. This
|
||||
ensures that all mounted sub-apps (v1, v2, main) handle errors uniformly.
|
||||
"""
|
||||
|
||||
import json
|
||||
import logging
|
||||
|
||||
import fastapi
|
||||
import fastapi.responses
|
||||
import pydantic
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from prisma.errors import PrismaError
|
||||
from starlette import status
|
||||
|
||||
from backend.api.features.library.exceptions import (
|
||||
FolderAlreadyExistsError,
|
||||
FolderValidationError,
|
||||
)
|
||||
from backend.util.exceptions import (
|
||||
MissingConfigError,
|
||||
NotAuthorizedError,
|
||||
NotFoundError,
|
||||
PreconditionFailed,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def add_exception_handlers(app: fastapi.FastAPI) -> None:
|
||||
"""
|
||||
Register standard exception handlers on the given FastAPI app.
|
||||
|
||||
Mounted sub-apps do NOT inherit exception handlers from the parent app,
|
||||
so each app instance must register its own handlers.
|
||||
"""
|
||||
for exception, handler in {
|
||||
# It's the client's problem: HTTP 4XX
|
||||
NotFoundError: _handle_error(status.HTTP_404_NOT_FOUND, log_error=False),
|
||||
NotAuthorizedError: _handle_error(status.HTTP_403_FORBIDDEN, log_error=False),
|
||||
PreconditionFailed: _handle_error(status.HTTP_428_PRECONDITION_REQUIRED),
|
||||
RequestValidationError: _handle_validation_error,
|
||||
pydantic.ValidationError: _handle_validation_error,
|
||||
FolderAlreadyExistsError: _handle_error(
|
||||
status.HTTP_409_CONFLICT, log_error=False
|
||||
),
|
||||
FolderValidationError: _handle_error(
|
||||
status.HTTP_400_BAD_REQUEST, log_error=False
|
||||
),
|
||||
ValueError: _handle_error(status.HTTP_400_BAD_REQUEST),
|
||||
# It's the backend's problem: HTTP 5XX
|
||||
MissingConfigError: _handle_error(status.HTTP_503_SERVICE_UNAVAILABLE),
|
||||
PrismaError: _handle_error(status.HTTP_500_INTERNAL_SERVER_ERROR),
|
||||
Exception: _handle_error(status.HTTP_500_INTERNAL_SERVER_ERROR),
|
||||
}.items():
|
||||
app.add_exception_handler(exception, handler)
|
||||
|
||||
|
||||
def _handle_error(status_code: int = 500, log_error: bool = True):
|
||||
def handler(request: fastapi.Request, exc: Exception):
|
||||
if log_error:
|
||||
logger.exception(
|
||||
"%s %s failed. Investigate and resolve the underlying issue: %s",
|
||||
request.method,
|
||||
request.url.path,
|
||||
exc,
|
||||
exc_info=exc,
|
||||
)
|
||||
|
||||
hint = (
|
||||
"Adjust the request and retry."
|
||||
if status_code < 500
|
||||
else "Check server logs and dependent services."
|
||||
)
|
||||
return fastapi.responses.JSONResponse(
|
||||
content={
|
||||
"message": f"Failed to process {request.method} {request.url.path}",
|
||||
"detail": str(exc),
|
||||
"hint": hint,
|
||||
},
|
||||
status_code=status_code,
|
||||
)
|
||||
|
||||
return handler
|
||||
|
||||
|
||||
async def _handle_validation_error(
|
||||
request: fastapi.Request, exc: Exception
|
||||
) -> fastapi.responses.Response:
|
||||
logger.error(
|
||||
"Validation failed for %s %s: %s. Fix the request payload and try again.",
|
||||
request.method,
|
||||
request.url.path,
|
||||
exc,
|
||||
)
|
||||
errors: list | str
|
||||
if hasattr(exc, "errors"):
|
||||
errors = exc.errors() # type: ignore[call-arg]
|
||||
else:
|
||||
errors = str(exc)
|
||||
|
||||
response_content = {
|
||||
"message": f"Invalid data for {request.method} {request.url.path}",
|
||||
"detail": errors,
|
||||
"hint": "Ensure the request matches the API schema.",
|
||||
}
|
||||
|
||||
content_json = json.dumps(response_content)
|
||||
|
||||
return fastapi.responses.Response(
|
||||
content=content_json,
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
|
||||
media_type="application/json",
|
||||
)
|
||||
@@ -44,7 +44,7 @@ class NotFoundError(ValueError):
|
||||
"""The requested record was not found, resulting in an error condition"""
|
||||
|
||||
|
||||
class GraphNotFoundError(ValueError):
|
||||
class GraphNotFoundError(NotFoundError):
|
||||
"""The requested Agent Graph was not found, resulting in an error condition"""
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
{
|
||||
"detail": "Agent #nonexistent not found",
|
||||
"hint": "Adjust the request and retry.",
|
||||
"message": "Failed to process GET /library/agents/nonexistent"
|
||||
}
|
||||
@@ -0,0 +1,5 @@
|
||||
{
|
||||
"detail": "connection refused",
|
||||
"hint": "Check server logs and dependent services.",
|
||||
"message": "Failed to process GET /library/agents/some-id"
|
||||
}
|
||||
5
autogpt_platform/backend/snapshots/v2_value_error_400
Normal file
5
autogpt_platform/backend/snapshots/v2_value_error_400
Normal file
@@ -0,0 +1,5 @@
|
||||
{
|
||||
"detail": "Invalid graph version: -1",
|
||||
"hint": "Adjust the request and retry.",
|
||||
"message": "Failed to process PATCH /library/agents/some-id"
|
||||
}
|
||||
Reference in New Issue
Block a user