-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Python: fix(claude): handle API errors in run_stream() method #3653
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
base: main
Are you sure you want to change the base?
Python: fix(claude): handle API errors in run_stream() method #3653
Conversation
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.
Pull request overview
This PR fixes a critical bug where ClaudeAgent.run_stream() silently ignored API errors and returned empty results instead of raising exceptions. The fix adds proper error handling for both AssistantMessage and ResultMessage error conditions.
Changes:
- Added error handling for
AssistantMessage.errorfield with descriptive error messages mapped from error types - Added error checking for
ResultMessage.is_errorflag to surface API errors as exceptions - Added comprehensive test coverage for both error scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/packages/claude/agent_framework_claude/_agent.py | Added imports for AssistantMessage and TextBlock; implemented error handling logic in run_stream() to raise ServiceException for API errors |
| python/packages/claude/tests/test_claude_agent.py | Added two new test cases verifying exception raising for AssistantMessage errors and ResultMessage errors |
|
I've got a fix in #3661 that allows for the failing unit test to pass. |
- Import AssistantMessage and TextBlock from claude_agent_sdk - Check AssistantMessage.error and raise ServiceException with descriptive message - Check ResultMessage.is_error and raise ServiceException with error details - Add tests for error handling in run_stream() Fixes microsoft#3652
Address PR review feedback - add null check for message.content to prevent potential AttributeError if content is None.
a22339a to
a148b32
Compare
|
@moonbox3 - I cherry picked your changes, ready to merge. |
|
@moonbox3 - kindly have a look at this as well |
|
This one needs a refresh on the uv.lock file: |
pre-commit hooks were also failing. its fixed now. |
|
We'll get this in, but before that, we're trying to get #3379 in, which may cause some merge conflicts here. Please be aware. |
Summary
This PR fixes a bug where
ClaudeAgent.run_stream()silently ignored API errors andAssistantMessageresponses, returning empty results instead of raising exceptions.Changes
_agent.pyAssistantMessageandTextBlockfromclaude_agent_sdkAssistantMessage.errorby raisingServiceExceptionwith descriptive error messages mapped from error types (authentication_failed,billing_error,rate_limit,invalid_request,server_error,unknown)ResultMessage.is_errorand raiseServiceExceptionwith the error details fromResultMessage.resulttest_claude_agent.pytest_run_stream_raises_on_assistant_message_error- verifiesServiceExceptionis raised whenAssistantMessagecontains an errortest_run_stream_raises_on_result_message_error- verifiesServiceExceptionis raised whenResultMessage.is_errorisTrueTesting
All 46 tests pass:
Fixes #3652