Fix: use httpx.AsyncClient in async FastAPI handlers to prevent event loop blocking (fixes #559) - #570
Conversation
…nt event loop blocking (fixes open-jarvis#559)
There was a problem hiding this comment.
Pull request overview
This PR addresses #559 by removing blocking I/O and CPU work from FastAPI async routes, primarily by switching outbound HTTP calls to httpx.AsyncClient and offloading synchronous transcription work to a thread to keep the Uvicorn event loop responsive.
Changes:
- Converted Ollama model pull/delete routes to use
httpx.AsyncClientinstead ofhttpx.Client. - Converted SendBlue verify/register/test routes to use
httpx.AsyncClientinstead ofhttpx.get/post. - Offloaded
backend.transcribe()in the speech transcription route usingasyncio.to_thread(), and added a regression test for non-blocking behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/server/test_async_handlers_nonblocking.py |
Adds a regression test intended to verify the event loop remains responsive during slow outbound requests. |
src/openjarvis/server/routes.py |
Uses httpx.AsyncClient in async model pull/delete handlers to avoid blocking the event loop. |
src/openjarvis/server/api_routes.py |
Runs synchronous speech transcription in a thread (asyncio.to_thread) to avoid blocking. |
src/openjarvis/server/agent_manager_routes.py |
Uses httpx.AsyncClient in async SendBlue routes to avoid blocking the event loop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Fire both requests concurrently | ||
| slow_req = asyncio.create_task( | ||
| client.post("/v1/models/pull", json={"name": "test-model"}) | ||
| ) |
| with pytest.MonkeyPatch.context() as m: | ||
| m.setattr("httpx.AsyncClient.post", mock_post) | ||
|
|
| import httpx as _httpx | ||
|
|
||
| host = getattr(engine, "_host", "http://localhost:11434") | ||
| client = _httpx.Client(base_url=host, timeout=600.0) | ||
| try: | ||
| resp = client.post( | ||
| "/api/pull", | ||
| json={"name": model_name, "stream": False}, | ||
| ) | ||
| resp.raise_for_status() | ||
| async with _httpx.AsyncClient(base_url=host, timeout=600.0) as client: | ||
| resp = await client.post( | ||
| "/api/pull", | ||
| json={"name": model_name, "stream": False}, | ||
| ) | ||
| resp.raise_for_status() |
Closes #219. Replace synchronous httpx calls in async SendBlue and model-management handlers with awaited httpx.AsyncClient (context-managed close); run Whisper transcription and engine.list_models via asyncio.to_thread so they don't block the event loop; and harden TelemetryStore/aggregator SQLite for concurrency (WAL, synchronous=NORMAL, busy_timeout=5000, plus a write-serializing lock on the shared connection). Adds async-usage assertions and a real 8-thread concurrent-write test. Related: #570 (async httpx, different issue #559). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Hey @ElliotSlusky @selena731! Now that #597 is merged, PR #570 is ready for review to close sub-issue #559. It refactors the remaining sync httpx handlers to AsyncClient and includes dedicated non-blocking regression tests. Fully rebased against main and all tests are passing clean! |
What does this PR do?
This PR resolves issue #559 by converting several blocking, synchronous
httpx.Client()calls insideasync defFastAPI routes into theirhttpx.AsyncClient()equivalents. It also offloads the CPU-boundbackend.transcribe()call inapi_routes.pyto a separate thread viaasyncio.to_thread(). This prevents the Uvicorn event loop from freezing and causing poor latency for other parallel requests.Changes
httpx.Clientwithasync with httpx.AsyncClientinserver/routes.py(/v1/models/pull,/v1/models/{name}).httpx.get/postwithasync with httpx.AsyncClientinserver/agent_manager_routes.py(/verify,/register-webhook,/test).server/api_routes.pytoasyncio.to_thread.tests/server/test_async_handlers_nonblocking.pyto verify event loop non-blocking behavior.Testing
Fixes Issue
Fixes #559