Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from http import HTTPStatus

from fastapi import Request
from fastapi.exceptions import RequestValidationError
from fastapi.responses import JSONResponse

# =============================================================================
Expand Down Expand Up @@ -89,6 +90,27 @@ def problem_detail_exception_handler(
)


def validation_exception_handler(
request: Request, # noqa: ARG001
exc: RequestValidationError,
) -> JSONResponse:
"""FastAPI exception handler for RequestValidationError.

Returns a RFC 9457 compliant response for input validation failures.
"""
return JSONResponse(
status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
content={
"type": "https://openml.org/problems/validation-error",
"title": "Validation Error",
"status": HTTPStatus.UNPROCESSABLE_ENTITY,
"detail": "Input validation failed.",
"errors": exc.errors(),
},
media_type="application/problem+json",
)
Comment on lines +93 to +111
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In FastAPI’s built-in RequestValidationError handler, is exc.errors()wrapped withjsonable_encoderbefore sendingJSONResponse, and can exc.errors()include non-JSON-serializable values inctx?

💡 Result:

Yes, in FastAPI's built-in RequestValidationError handler, exc.errors is wrapped with jsonable_encoder before being sent in the JSONResponse. The handler code explicitly does: content={"detail": jsonable_encoder(exc.errors)}. Yes, exc.errors can include non-JSON-serializable values in the ctx field, particularly the input value from Pydantic v2 validation errors, which may contain complex objects like bytes or other non-serializable types. This can cause jsonable_encoder to fail in some cases, as reported in user discussions and issues.

Citations:


🏁 Script executed:

# First, check if HTTPStatus is JSON-serializable in content dict
python <<EOF
import json
from http import HTTPStatus

# Test if HTTPStatus enum is directly JSON-serializable
try:
    json.dumps({"status": HTTPStatus.UNPROCESSABLE_ENTITY})
    print("HTTPStatus IS JSON-serializable")
except TypeError as e:
    print(f"HTTPStatus is NOT JSON-serializable: {e}")

# Test if int(HTTPStatus) is JSON-serializable
try:
    json.dumps({"status": int(HTTPStatus.UNPROCESSABLE_ENTITY)})
    print("int(HTTPStatus) IS JSON-serializable")
except TypeError as e:
    print(f"int(HTTPStatus) is NOT JSON-serializable: {e}")
EOF

Repository: openml/server-api

Length of output: 129


🏁 Script executed:

# Check the actual code in src/core/errors.py
if [ -f "src/core/errors.py" ]; then
    echo "=== Checking imports at top of file ==="
    head -20 src/core/errors.py
    echo ""
    echo "=== Checking lines 93-111 ==="
    sed -n '93,111p' src/core/errors.py
else
    echo "File not found, searching for errors.py"
    find . -name "errors.py" -type f 2>/dev/null
fi

Repository: openml/server-api

Length of output: 1387


🏁 Script executed:

# Also check what version of FastAPI and what the current import pattern is
rg "from fastapi" src/core/errors.py -A 2 -B 2

Repository: openml/server-api

Length of output: 295


Wrap exc.errors() with jsonable_encoder before returning it.

At line 108, exc.errors() is passed directly. Validation errors can include non-JSON-serializable values in ctx, which may raise during response rendering and convert the 422 into a 500. FastAPI's built-in handler wraps this with jsonable_encoder for this reason.

Proposed fix
 from fastapi import Request
+from fastapi.encoders import jsonable_encoder
 from fastapi.exceptions import RequestValidationError
 from fastapi.responses import JSONResponse
@@
     return JSONResponse(
         status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
         content={
             "type": "about:blank",
             "title": "Validation Error",
             "status": HTTPStatus.UNPROCESSABLE_ENTITY,
             "detail": "Input validation failed.",
-            "errors": exc.errors(),
+            "errors": jsonable_encoder(exc.errors()),
         },
         media_type="application/problem+json",
     )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def validation_exception_handler(
request: Request, # noqa: ARG001
exc: RequestValidationError,
) -> JSONResponse:
"""FastAPI exception handler for RequestValidationError.
Returns a RFC 9457 compliant response for input validation failures.
"""
return JSONResponse(
status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
content={
"type": "about:blank",
"title": "Validation Error",
"status": HTTPStatus.UNPROCESSABLE_ENTITY,
"detail": "Input validation failed.",
"errors": exc.errors(),
},
media_type="application/problem+json",
)
def validation_exception_handler(
request: Request, # noqa: ARG001
exc: RequestValidationError,
) -> JSONResponse:
"""FastAPI exception handler for RequestValidationError.
Returns a RFC 9457 compliant response for input validation failures.
"""
return JSONResponse(
status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
content={
"type": "about:blank",
"title": "Validation Error",
"status": HTTPStatus.UNPROCESSABLE_ENTITY,
"detail": "Input validation failed.",
"errors": jsonable_encoder(exc.errors()),
},
media_type="application/problem+json",
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/errors.py` around lines 93 - 111, The validation_exception_handler
currently returns exc.errors() directly which can contain non-JSON-serializable
values; wrap the errors with FastAPI's jsonable_encoder before including them in
the JSONResponse (i.e., replace "errors": exc.errors() with "errors":
jsonable_encoder(exc.errors())), and add the import for jsonable_encoder from
fastapi.encoders if not already present so the response can't raise during
rendering.



# =============================================================================
# Dataset Errors
# =============================================================================
Expand Down
8 changes: 7 additions & 1 deletion src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@

import uvicorn
from fastapi import FastAPI
from fastapi.exceptions import RequestValidationError
from loguru import logger

from config import load_configuration
from core.errors import ProblemDetailError, problem_detail_exception_handler
from core.errors import (
ProblemDetailError,
problem_detail_exception_handler,
validation_exception_handler,
)
from core.logging import (
add_request_context_to_log,
log_request_duration,
Expand Down Expand Up @@ -87,6 +92,7 @@ def create_api(configuration_file: Path | None = None) -> FastAPI:
app.middleware("http")(add_request_context_to_log)

app.add_exception_handler(ProblemDetailError, problem_detail_exception_handler) # type: ignore[arg-type]
app.add_exception_handler(RequestValidationError, validation_exception_handler) # type: ignore[arg-type]

logger.info("Adding routers to app")
app.include_router(datasets_router)
Expand Down
2 changes: 1 addition & 1 deletion tests/routers/openml/dataset_tag_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ async def test_dataset_tag_invalid_tag_is_rejected(
)

assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
assert response.json()["detail"][0]["loc"] == ["body", "tag"]
assert response.json()["errors"][0]["loc"] == ["body", "tag"]


# ── Direct call tests: tag_dataset ──
Expand Down
14 changes: 7 additions & 7 deletions tests/routers/openml/task_list_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ async def test_list_tasks_invalid_pagination_type(
)
assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
# Verify that the error points to the correct field
detail = response.json()["detail"][0]
assert detail["loc"][-2:] == ["pagination", expected_field]
assert detail["type"] in {"type_error.integer", "int_parsing", "int_type"}
error = response.json()["errors"][0]
assert error["loc"][-2:] == ["pagination", expected_field]
assert error["type"] in {"type_error.integer", "int_parsing", "int_type"}


@pytest.mark.parametrize(
Expand All @@ -150,8 +150,8 @@ async def test_list_tasks_invalid_range(value: str, py_api: httpx.AsyncClient) -
response = await py_api.post("/tasks/list", json={"number_instances": value})
assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
# Verify the error is for the correct field
detail = response.json()["detail"][0]
assert detail["loc"][-1] == "number_instances"
error = response.json()["errors"][0]
assert error["loc"][-1] == "number_instances"


@pytest.mark.parametrize(
Expand All @@ -171,9 +171,9 @@ async def test_list_tasks_invalid_inputs(
response = await py_api.post("/tasks/list", json=payload)
assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
# Ensure we are failing for the field we provided
detail = response.json()["detail"][0]
error = response.json()["errors"][0]
expected_field = next(iter(payload))
assert detail["loc"][-1] == expected_field
assert error["loc"][-1] == expected_field


async def test_list_tasks_no_results_api_mapping(py_api: httpx.AsyncClient) -> None:
Expand Down
Loading