fix: remove blocking asyncio gather after cancelling disconnect watch…#1202
Conversation
…er in sync endpoints After wait_task completes, cancel() on disconnect_task followed by asyncio.gather blocks indefinitely because the disconnect watcher is stuck in request.is_disconnected() → receive() which never returns while the client is still connected. Fire-and-forget cancel is sufficient — the completed task already holds the result. Affected: POST /v1/tasks/image/sync and OpenAI /v1/images/* sync handlers.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the asynchronous task handling and cleanup logic when waiting for image generation tasks and client disconnect watchers. The reviewer identified an issue in both modified files where removing the .result() call on disconnect_task prevents exceptions raised within the disconnect watcher from being propagated. This can lead to unretrieved task exception warnings and incorrect HTTP 499 status codes instead of proper server errors. Restoring the .result() check is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if disconnect_task in done: | ||
| if not wait_task.done(): | ||
| wait_task.cancel() | ||
| await asyncio.wait({wait_task}, timeout=0) | ||
| raise HTTPException(status_code=499, detail=f"Client disconnected, task {task_id} cancelled") |
There was a problem hiding this comment.
By checking if disconnect_task in done: without calling .result(), any exception raised inside _watch_client_disconnect (for example, if request.is_disconnected() fails) will not be propagated. This leads to two issues:
- Python will log a
Task exception was never retrievedwarning. - The endpoint will incorrectly raise a
499 Client Disconnectederror instead of propagating the actual exception (which would normally result in a 500/503 error).
Calling disconnect_task.result() ensures that any exceptions are properly propagated and handled.
| if disconnect_task in done: | |
| if not wait_task.done(): | |
| wait_task.cancel() | |
| await asyncio.wait({wait_task}, timeout=0) | |
| raise HTTPException(status_code=499, detail=f"Client disconnected, task {task_id} cancelled") | |
| if disconnect_task in done and disconnect_task.result(): | |
| if not wait_task.done(): | |
| wait_task.cancel() | |
| raise HTTPException(status_code=499, detail=f"Client disconnected, task {task_id} cancelled") |
| if disconnect_task in done: | ||
| if not wait_task.done(): | ||
| wait_task.cancel() | ||
| await asyncio.gather(wait_task, return_exceptions=True) | ||
| raise HTTPException(status_code=499, detail=f"Client disconnected, task {task_id} cancelled") |
There was a problem hiding this comment.
By checking if disconnect_task in done: without calling .result(), any exception raised inside _watch_client_disconnect (for example, if request.is_disconnected() fails) will not be propagated. This leads to two issues:
- Python will log a
Task exception was never retrievedwarning. - The endpoint will incorrectly raise a
499 Client Disconnectederror instead of propagating the actual exception (which would normally result in a 500/503 error).
Calling disconnect_task.result() ensures that any exceptions are properly propagated and handled.
| if disconnect_task in done: | |
| if not wait_task.done(): | |
| wait_task.cancel() | |
| await asyncio.gather(wait_task, return_exceptions=True) | |
| raise HTTPException(status_code=499, detail=f"Client disconnected, task {task_id} cancelled") | |
| if disconnect_task in done and disconnect_task.result(): | |
| if not wait_task.done(): | |
| wait_task.cancel() | |
| raise HTTPException(status_code=499, detail=f"Client disconnected, task {task_id} cancelled") |
2 participants
…er in sync endpoints
After wait_task completes, cancel() on disconnect_task followed by asyncio.gather
blocks indefinitely because the disconnect watcher is stuck in request.is_disconnected()
→ receive() which never returns while the client is still connected. Fire-and-forget
cancel is sufficient — the completed task already holds the result.
Affected: POST /v1/tasks/image/sync and OpenAI /v1/images/* sync handlers.