-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Address PersistentChatClient not handling streaming content errors #54197
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?
Address PersistentChatClient not handling streaming content errors #54197
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 adds error handling behavior to the PersistentChatClient to make streaming content errors from misconfigured tools more visible. By default, the client now throws exceptions when content errors occur, but can be configured to return errors as ErrorContent objects instead.
Key changes:
- Added
throwOnContentErrorsparameter toAsIChatClientextension method (defaults totrue) - Implemented error detection for failed runs with
LastErrorin the streaming response handler - Added comprehensive test coverage for both throwing and non-throwing error handling modes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| PersistentAgentsChatClient.cs | Adds error handling logic to detect failed runs and either throw exceptions or add ErrorContent based on the throwOnContentErrors setting |
| PersistentAgentsClientExtensions.cs | Updates the AsIChatClient extension method signature to include the new throwOnContentErrors parameter |
| PersistentAgentsChatClientTests.cs | Adds new test TestContentErrorHandling with mock transport to verify both error handling behaviors, plus helper methods for creating mock responses |
| assets.json | Updates test assets tag reference |
sdk/ai/Azure.AI.Agents.Persistent/src/Custom/PersistentAgentsChatClient.cs
Outdated
Show resolved
Hide resolved
sdk/ai/Azure.AI.Agents.Persistent/tests/PersistentAgentsChatClientTests.cs
Show resolved
Hide resolved
sdk/ai/Azure.AI.Agents.Persistent/tests/PersistentAgentsChatClientTests.cs
Show resolved
Hide resolved
sdk/ai/Azure.AI.Agents.Persistent/tests/PersistentAgentsChatClientTests.cs
Outdated
Show resolved
Hide resolved
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
Co-authored-by: Copilot <[email protected]>
|
Why does this need to be configurable? In the case where this fails, it sounds like there won't be any other content, so someone who wanted an ErrorContent could catch the exception and create their own. And presumably we think someone wanting to do that is rare. |
There will be metadata content, available as the |
Motivation & Context
Was not clear when a misconfigured tool was used, resulting in non conventional and unexcepted empty response.
cc: @stephentoub
Solution
This change adds a Error Handling behavior setting to the
IChatClient:ErrorContentin message/chunkContents.