-
Notifications
You must be signed in to change notification settings - Fork 100
Fix RemoteConversation polling on terminal errors #1572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Coverage Report •
|
||||||||||||||||||||
|
Hi! I started running the integration tests on your PR. You will receive a comment with the results shortly. |
🧪 Integration Tests ResultsOverall Success Rate: 0.0% 📁 Detailed Logs & ArtifactsClick the links below to access detailed agent/LLM logs showing the complete reasoning process for each model. On the GitHub Actions page, scroll down to the 'Artifacts' section to download the logs.
📊 Summary
📋 Detailed Resultslitellm_proxy_vertex_ai_gemini_3_pro_preview
Failed Tests:
litellm_proxy_claude_sonnet_4_5_20250929
Failed Tests:
litellm_proxy_mistral_devstral_2512
Skipped Tests:
Failed Tests:
litellm_proxy_gpt_5.1_codex_max
Skipped Tests:
Failed Tests:
litellm_proxy_deepseek_deepseek_chat
Skipped Tests:
Failed Tests:
litellm_proxy_moonshot_kimi_k2_thinking
Skipped Tests:
Failed Tests:
|
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
|
@OpenHands read this issue #1570 |
|
I'm on it! simonrosenberg can track my progress at all-hands.dev |
Analysis SummaryDoes this PR solve the problem described in Issue #1570?Yes, the PR correctly solves the issue. The issue described that The PR fixes this by:
Logical Flaws AnalysisNo significant logical flaws found in either The code structure is sound:
The TestsThe PR adds three new tests that all pass:
All three tests pass successfully. |
xingyaoww
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Hi! I started running the integration tests on your PR. You will receive a comment with the results shortly. |
🧪 Integration Tests ResultsOverall Success Rate: 98.0% 📁 Detailed Logs & ArtifactsClick the links below to access detailed agent/LLM logs showing the complete reasoning process for each model. On the GitHub Actions page, scroll down to the 'Artifacts' section to download the logs.
📊 Summary
📋 Detailed Resultslitellm_proxy_mistral_devstral_2512
Skipped Tests:
Failed Tests:
litellm_proxy_deepseek_deepseek_chat
Skipped Tests:
litellm_proxy_vertex_ai_gemini_3_pro_preview
litellm_proxy_claude_sonnet_4_5_20250929
litellm_proxy_gpt_5.1_codex_max
Skipped Tests:
litellm_proxy_moonshot_kimi_k2_thinking
Skipped Tests:
|
| RuntimeError(f"Polling failed with HTTP {status_code} {reason}"), | ||
| ) from exc | ||
| logger.warning( | ||
| "Error polling status (will retry): HTTP %d %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I see this right, we retry on all statuses except 400’s? Even on 500 and 500+
Ah, until timeout, right… makes sense to me 🤔
enyst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! ☕️
Fixes #1570
Summary
_handle_terminal_statusto_handle_conversation_statusand move completion logging to the caller.Testing
Ran CI workflow with this branch: no more polling issues
Checklist
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:d73dfb9-pythonRun
All tags pushed for this build
About Multi-Architecture Support
d73dfb9-python) is a multi-arch manifest supporting both amd64 and arm64d73dfb9-python-amd64) are also available if needed