fix: catch CancelledError explicitly in all router streaming generators#7379
fix: catch CancelledError explicitly in all router streaming generators#7379
Conversation
Align agent and workflow router streaming generators to catch BaseException instead of Exception, matching the existing pattern in the team router. This ensures asyncio.CancelledError and other BaseException subclasses are properly handled when clients disconnect during SSE streaming.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a92c2ed5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| yield format_sse_event(error_response) | ||
| except Exception as e: | ||
| except BaseException as e: |
There was a problem hiding this comment.
Catching BaseException here causes GeneratorExit (raised when StreamingResponse closes an async generator on client disconnect/teardown) to enter this handler, and the subsequent yield triggers RuntimeError: async generator ignored GeneratorExit. This turns normal disconnects into error logs and can interfere with clean cancellation/shutdown behavior. In these streamer functions (agent_response_streamer, agent_continue_response_streamer, and workflow_response_streamer), catch asyncio.CancelledError explicitly if needed, but let GeneratorExit/other termination exceptions propagate.
Useful? React with 👍 / 👎.
Replace broad BaseException/Exception catch-all with targeted asyncio.CancelledError handling in all four SSE streaming generators (agent x2, team, workflow). CancelledError gets a clean return since the client is already gone, while Exception still yields an error event. This avoids swallowing GeneratorExit/SystemExit/KeyboardInterrupt which the previous BaseException catch in the team router could hit.
ysolanky
changed the title
1 participant
Summary
Adds explicit
asyncio.CancelledErrorhandling to all SSE streaming generator functions across agent, team, and workflow routers.Related: #7320, #7323, #7335
Problem: When a client disconnects during SSE streaming,
asyncio.CancelledError(aBaseExceptionsubclass) propagates unhandled through the generators since they only catchException.Fix: Add
except asyncio.CancelledError: returnbeforeexcept Exceptionin all four streaming generators. This cleanly terminates the generator when the client is gone, without logging errors or trying to yield events to a disconnected client.The team router previously used
except BaseExceptionwhich caughtCancelledErrorbut also swallowedGeneratorExit,SystemExit, andKeyboardInterrupt— theyieldinside that handler would causeRuntimeErroronGeneratorExit. This PR fixes the team router as well.agent_response_streamerexcept Exceptionexcept CancelledError: returnagent_continue_response_streamerexcept Exceptionexcept CancelledError: returnteam_response_streamerexcept BaseExceptionexcept CancelledError: return+except Exceptionworkflow_response_streamerexcept Exceptionexcept CancelledError: returnType of change
Checklist
./scripts/format.shand./scripts/validate.sh)Duplicate and AI-Generated PR Check
Additional Notes
PRs #7323 and #7335 both address the
CancelledErrorissue in the agent router only. This PR takes a comprehensive approach: all four streaming generators now use the same pattern, and the team router's existingBaseExceptioncatch is narrowed to avoid swallowingGeneratorExit(which would causeRuntimeErroron yield).