Skip to content

Commit 3bdedc9

Browse files
Return 422 for malformed JSON bodies and surface invalid resolver timestamps (#46, #47)
Malformed JSON request bodies previously produced a generic Starlette 500 because the endpoints called request.json() directly without guarding json.JSONDecodeError. Now all four POST endpoints (/v1/events, /v1/events:batch, /v1/search, /v1/query) use a shared _json_body() helper that raises ValidationError (→ 422) on decode failure, consistent with the existing error-to-status mapping. Invalid resolver begin/end timestamps were silently dropped by _valid_optional_timestamp, widening the search to unbounded instead of surfacing the error. Now a malformed timestamp raises QueryError, which the query error handler catches and returns as status='error'. Tests: - Parametrized test covering all four POST endpoints with malformed JSON. - Updated test_malformed_optional_timestamp to assert the new error behavior.
1 parent a236947 commit 3bdedc9

4 files changed

Lines changed: 44 additions & 10 deletions

File tree

historian/http.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import asyncio
66
import contextvars
7+
import json
78
import time
89
import uuid
910
from dataclasses import asdict
@@ -67,6 +68,13 @@ def _bearer_token(request: Request) -> str:
6768
return token.strip()
6869

6970

71+
async def _json_body(request: Request) -> Any:
72+
try:
73+
return await request.json()
74+
except json.JSONDecodeError as exc:
75+
raise ValidationError("Request body must be valid JSON.") from exc
76+
77+
7078
def build_agent_card(base_url: str) -> AgentCard:
7179
bearer = SecurityScheme(
7280
http_auth_security_scheme=HTTPAuthSecurityScheme(
@@ -262,7 +270,7 @@ async def agent_card_alias() -> JSONResponse:
262270

263271
@app.post("/v1/events")
264272
async def ingest_event(request: Request) -> dict[str, Any]:
265-
event, duplicate = context.service.ingest(request.state.principal, await request.json())
273+
event, duplicate = context.service.ingest(request.state.principal, await _json_body(request))
266274
_LOG.info(
267275
"event_id=%s producer_app=%s type=%s duplicate=%s event_ingested",
268276
event.event_id,
@@ -274,7 +282,7 @@ async def ingest_event(request: Request) -> dict[str, Any]:
274282

275283
@app.post("/v1/events:batch")
276284
async def ingest_batch(request: Request) -> dict[str, Any]:
277-
body = await request.json()
285+
body = await _json_body(request)
278286
events = body.get("events") if isinstance(body, dict) else None
279287
if not isinstance(events, list):
280288
raise ValidationError("Batch body must contain an events array.")
@@ -294,7 +302,7 @@ async def ingest_batch(request: Request) -> dict[str, Any]:
294302

295303
@app.post("/v1/search")
296304
async def search_events(request: Request) -> dict[str, Any]:
297-
body = await request.json()
305+
body = await _json_body(request)
298306
try:
299307
spec = SearchSpec(**body)
300308
except TypeError as exc:
@@ -333,7 +341,7 @@ async def get_event(event_id: str, request: Request) -> dict[str, Any]:
333341

334342
@app.post("/v1/query")
335343
async def structured_query(request: Request) -> dict[str, Any]:
336-
body = await request.json()
344+
body = await _json_body(request)
337345
question = str(body.get("question", "")) if isinstance(body, dict) else ""
338346
return to_jsonable(context.service.query(request.state.principal, question))
339347

historian/service.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,8 +391,8 @@ def _valid_optional_timestamp(cls, value: Any) -> str | None:
391391
return None
392392
try:
393393
datetime.fromisoformat(text.replace("Z", "+00:00"))
394-
except ValueError:
395-
return None
394+
except ValueError as exc:
395+
raise QueryError(f"Resolver returned an invalid timestamp: {text!r}") from exc
396396
return text
397397

398398
def _evidence_chunks(

tests/test_http.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import asyncio
44

55
import httpx
6+
import pytest
67
from a2a.helpers import new_text_part
78
from a2a.types import Message, Role, SendMessageRequest
89
from a2a.utils.constants import PROTOCOL_VERSION_1_0, VERSION_HEADER
@@ -180,3 +181,28 @@ def test_search_endpoint_rejects_invalid_spec(context, vesper_token) -> None:
180181
)
181182
assert response.status_code == 422
182183
assert response.json()["status"] == "error"
184+
185+
186+
@pytest.mark.parametrize(
187+
"path,method",
188+
[
189+
("/v1/events", "POST"),
190+
("/v1/events:batch", "POST"),
191+
("/v1/search", "POST"),
192+
("/v1/query", "POST"),
193+
],
194+
)
195+
def test_malformed_json_returns_422(context, vesper_token, path, method) -> None:
196+
"""POST endpoints return 422 for malformed JSON bodies, not 500."""
197+
app = create_http_app(context)
198+
response = asyncio.run(
199+
_request(
200+
app,
201+
method,
202+
path,
203+
headers={"Authorization": f"Bearer {vesper_token}", "Content-Type": "application/json"},
204+
content=b"{not valid json",
205+
)
206+
)
207+
assert response.status_code == 422
208+
assert response.json()["status"] == "error"

tests/test_query.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,10 @@ def test_search_timestamps_and_text_are_optional(context, resolver, vesper_token
112112
]
113113

114114

115-
def test_malformed_optional_timestamp_does_not_abort_search(
115+
def test_malformed_optional_timestamp_raises_query_error(
116116
context, resolver, vesper_token
117117
) -> None:
118+
"""A malformed begin/end from the resolver surfaces as a QueryError, not a silent unbounded search."""
118119
principal = context.store.authenticate(vesper_token)
119120
context.service.ingest(principal, event())
120121
resolver.plans.append(
@@ -133,9 +134,8 @@ def test_malformed_optional_timestamp_does_not_abort_search(
133134
)
134135
resolver.answers.append(_answer())
135136
result = context.service.query(principal, "What has Vesper played?")
136-
assert result.status == "ok"
137-
assert result.searches[0]["begin"] is None
138-
assert result.searches[0]["end"] is None
137+
assert result.status == "error"
138+
assert "invalid timestamp" in result.message
139139

140140

141141
def test_model_plan_is_executed_faithfully(

0 commit comments

Comments
 (0)