Add HTTP disconnection detection to Agentic RL.#152
Conversation
- Re-raise exceptions that already carry an error_code instead of wrapping them, so the original code propagates to callers - Add _format_cause() to AgenticRLError, RuntimeErrorWithCode and ValueErrorWithCode so __str__ includes the root cause type and message when the exception was triggered by another error - Let OSSConnectionError escape FunctionComponentModel without being re-wrapped Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Agentic RL support cancel async call
|
Add HTTP disconnection detection to Agentic RL. |
![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 introduces client disconnect monitoring in the server endpoint to cancel processing tasks early and avoid wasting resources, increases the thread pool worker count, adds utility functions for extracting request metadata, and enhances error string formatting with root cause details. The code review feedback highlights a critical race condition where starting the disconnect listener before parsing the request body can cause concurrent reads on the request stream, leading to hung requests. Additionally, the reviewer suggests refactoring the duplicated _format_cause helper method across multiple error classes to adhere to DRY principles.
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.
| # Execute processor as a task so we can cancel it on disconnect | ||
| process_task = asyncio.create_task( | ||
| func_manager.processes(processor_input), | ||
| ) |
There was a problem hiding this comment.
Start the background disconnect listener here, after the request body has been fully read and parsed. This ensures that the listener does not compete with the body reader for ASGI messages.
# Start listening for disconnect only AFTER the request body has been fully read.
# Otherwise, the background listener will compete with the body reader for ASGI messages.
disconnect_listener = asyncio.create_task(_listen_for_disconnect())
# Execute processor as a task so we can cancel it on disconnect
process_task = asyncio.create_task(
func_manager.processes(processor_input),
)| def _format_cause(self) -> str: | ||
| """Format root cause information if available.""" | ||
| if self.__cause__ is not None and self is not self.root_cause: | ||
| root = self.root_cause | ||
| cause_msg = str(root).split("\n")[0][:100] | ||
| return f" (caused by: {type(root).__name__}: {cause_msg})" | ||
| return "" |
There was a problem hiding this comment.
The _format_cause method is duplicated identically across AgenticRLError, RuntimeErrorWithCode, and ValueErrorWithCode. To improve maintainability and adhere to DRY (Don't Repeat Yourself) principles, consider refactoring this logic into a shared module-level helper function or a mixin class.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Fix critical race condition: delay disconnect listener until after request body parsing to prevent concurrent receive() calls - Refactor error classes: extract _RootCauseMixin to eliminate duplicated _format_cause and root_cause methods - Extract helper functions to reduce function complexity - Fix pre-commit issues: mypy types, flake8 line length, pylint warnings Addresses PR dashscope#152 review feedback from Gemini Code Assist. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
Description
[Describe what this PR does and why]
Related Issue: Fixes #[issue_number] or Relates to #[issue_number]
Security Considerations: [Check if API keys or sensitive credentials are exposed in code/logs]
Type of Change
Component(s) Affected
Checklist
Testing
[How to test these changes]
Additional Notes
[Optional: any other context]