3bb9069
commit 3bb9069
4 files changed
+72 -14
Showing 4 changed files+72-14
▼DECISIONS.md
+31-0
| @@ -2184,3 +2184,34 @@ body-stream error — aborting the entire test process. | ||
2184 | 2184 | **Evidence:** `pytest crates/justapi-py/python/justapi/` → 120 passed, 1 skipped |
2185 | 2185 | (previously aborted). `cargo test --workspace --features db` green; clippy |
2186 | 2186 | `-D warnings` clean; `cargo fmt --check` clean. |
2187 | + | |
2188 | +## ADR-059 — 2026-07-17 — `Request.json()/body()/form()` must not require an event loop | |
2189 | + | |
2190 | +**Context:** A framework audit (background tasks, routes, request handling) found | |
2191 | +that a **sync** handler accessing `request.json()` / `request.body()` / | |
2192 | +`request.form()` raised `RuntimeError: no running event loop`. Root cause: | |
2193 | +`Request::json`/`body`/`form` in `crates/justapi-py/src/request.rs` returned their | |
2194 | +value wrapped in `pyo3_async_runtimes::tokio::future_into_py(...)`, i.e. an | |
2195 | +awaitable coroutine. Under the real server the sync handler runs on the daemon | |
2196 | +event-loop thread (so it happened to work), but the `JustAPITestClient` path | |
2197 | +(`make_test_handler`) runs sync handlers on a plain worker thread with **no** | |
2198 | +running loop — so `request.json()` crashed every sync handler that parsed a body | |
2199 | +through the test client. `body()`/`form()` had the same latent defect. | |
2200 | + | |
2201 | +**Decision / change:** Make `json()`, `body()`, and `form()` pure synchronous | |
2202 | +returns (they are CPU-only parses — no I/O). `receive`/`close`/`is_disconnected` | |
2203 | +remain async (legitimate ASGI coroutines). Updated the two `await request.*` | |
2204 | +call sites (`system.py` `tools_call_handler`, `test_request_parity.py`) to call | |
2205 | +them without `await`. Added `test_sync_handler_body_json_no_event_loop` to lock | |
2206 | +the regression in. | |
2207 | + | |
2208 | +**Evidence:** New repro + test confirm `request.json()` works in a sync handler | |
2209 | +via `JustAPITestClient` (previously `RuntimeError: no running event loop`). Full | |
2210 | +suite 120 passed / 1 skipped; `cargo test --workspace --features db`, clippy | |
2211 | +`-D warnings`, `cargo fmt --check` all green. | |
2212 | + | |
2213 | +**Other audit observations (not changed):** (1) Registering two `GET` routes on | |
2214 | +the same path raises `ValueError` ("route conflict") from the `matchit` router — | |
2215 | +FastAPI/Starlette let the last win; this is a deliberate hard-fail, documented | |
2216 | +here. (2) Trailing-slash mismatch is not normalized: a route declared `/trail/` | |
2217 | +returns 404 for `/trail`. Consistent with the hard-fail philosophy; left as-is. | |
2187 | 2218 | |
▼crates/justapi-py/python/justapi/system.py
+1-1
| @@ -344,7 +344,7 @@ def _help_handler_factory(app): | ||
344 | 344 | return JSONResponse({"tools": app.list_tools(), "count": len(app.list_tools())}) |
345 | 345 | |
346 | 346 | async def tools_call_handler(request): |
347 | - body = await request.json() | |
347 | + body = request.json() | |
348 | 348 | name = body.get("name") |
349 | 349 | arguments = body.get("arguments", {}) |
350 | 350 | if not name: |
351 | 351 | |
▼crates/justapi-py/python/justapi/test_request_parity.py
+24-3
| @@ -62,8 +62,8 @@ def test_request_body_json_form(): | ||
62 | 62 | app = JustAPIApp() |
63 | 63 | |
64 | 64 | async def inspect(request): |
65 | - body = await request.body() | |
66 | - parsed = await request.json() | |
65 | + body = request.body() | |
66 | + parsed = request.json() | |
67 | 67 | return {"len": len(body), "parsed": parsed} |
68 | 68 | |
69 | 69 | app.post("/data", inspect) |
| @@ -77,7 +77,7 @@ def test_request_body_json_form(): | ||
77 | 77 | assert data["parsed"] == {"x": 7} |
78 | 78 | |
79 | 79 | async def form_inspect(request): |
80 | - form = await request.form() | |
80 | + form = request.form() | |
81 | 81 | return {"a": form.get("a"), "b": form.get("b")} |
82 | 82 | |
83 | 83 | app.post("/form", form_inspect) |
| @@ -135,8 +135,29 @@ def test_app_url_for_and_include_router(): | ||
135 | 135 | assert data["pong"] is True |
136 | 136 | |
137 | 137 | |
138 | +def test_sync_handler_body_json_no_event_loop(): | |
139 | + """`request.body()` / `request.json()` must work inside a *sync* handler, | |
140 | + where no asyncio event loop is running (e.g. under the test client). They are | |
141 | + pure synchronous parses and must not require `await`.""" | |
142 | + app = JustAPIApp() | |
143 | + | |
144 | + def handler(request): | |
145 | + return {"len": len(request.body()), "parsed": request.json()} | |
146 | + | |
147 | + app.post("/data", handler) | |
148 | + client = JustAPITestClient(app) | |
149 | + resp = client.post_with( | |
150 | + "/data", b'{"x": 7}', [("Content-Type", "application/json")] | |
151 | + ) | |
152 | + assert resp["status"] == 200 | |
153 | + data = json.loads(bytes(resp["body"])) | |
154 | + assert data["len"] == 8 | |
155 | + assert data["parsed"] == {"x": 7} | |
156 | + | |
157 | + | |
138 | 158 | if __name__ == "__main__": |
139 | 159 | test_request_attributes_and_mapping() |
140 | 160 | test_request_body_json_form() |
141 | 161 | test_request_state_isolation_and_url_for() |
162 | + test_sync_handler_body_json_no_event_loop() | |
142 | 163 | print("PASS: request parity") |
143 | 164 | |
▼crates/justapi-py/src/request.rs
+16-10
| @@ -344,22 +344,28 @@ impl Request { | ||
344 | 344 | |
345 | 345 | #[pyo3(signature = ())] |
346 | 346 | fn body<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> { |
347 | - let body = PyBytes::new(py, &self.body_raw).into_any().unbind(); | |
348 | - future_into_py(py, async move { Ok(body) }) | |
347 | + // Pure synchronous parse: returns the bytes directly so it works in both | |
348 | + // sync and async handlers (no running event loop required). Previously this | |
349 | + // wrapped the value in `future_into_py`, which raised "no running event | |
350 | + // loop" for sync handlers invoked without a loop (e.g. via the test client). | |
351 | + Ok(PyBytes::new(py, &self.body_raw).into_any()) | |
349 | 352 | } |
350 | 353 | |
351 | 354 | #[pyo3(signature = ())] |
352 | 355 | fn json<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> { |
353 | - let parsed = { | |
354 | - let json_module = py.import("json")?; | |
355 | - let body_bytes = PyBytes::new(py, &self.body_raw); | |
356 | - json_module.getattr("loads")?.call1((body_bytes,))?.unbind() | |
357 | - }; | |
358 | - future_into_py(py, async move { Ok(parsed) }) | |
356 | + // Pure synchronous parse: returns the parsed value directly so it works in | |
357 | + // both sync and async handlers without requiring a running event loop. | |
358 | + let json_module = py.import("json")?; | |
359 | + let body_bytes = PyBytes::new(py, &self.body_raw); | |
360 | + let loads = json_module.getattr("loads")?; | |
361 | + let parsed = loads.call1((body_bytes,))?; | |
362 | + Ok(parsed) | |
359 | 363 | } |
360 | 364 | |
361 | 365 | #[pyo3(signature = ())] |
362 | - fn form<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> { | |
366 | + fn form(&self, py: Python<'_>) -> PyResult<Py<PyAny>> { | |
367 | + // Pure synchronous parse: returns the form dict directly so it works in | |
368 | + // both sync and async handlers without requiring a running event loop. | |
363 | 369 | let result = if let Some(ref d) = self.form_data { |
364 | 370 | d.bind(py).clone().into_any().unbind() |
365 | 371 | } else { |
| @@ -388,7 +394,7 @@ impl Request { | ||
388 | 394 | PyDict::new(py).into_any().unbind() |
389 | 395 | } |
390 | 396 | }; |
391 | - future_into_py(py, async move { Ok(result) }) | |
397 | + Ok(result) | |
392 | 398 | } |
393 | 399 | |
394 | 400 | #[pyo3(signature = ())] |
395 | 401 | |