-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: Catch http errors #1308
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?
fix: Catch http errors #1308
Conversation
|
@Kludex bumping this -- really important for our remote server to include this, our library is forced to depend on our fork otherwise! |
felixweinberger
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.
Hi @lorenss-m thanks for this contribution!
There seem to be a few competing approaches to handle this issue: #1194, #1008
We probably want to resolve them once in the right place, I will aim to take a look this week.
| await self._handle_resumption_request(ctx) | ||
| else: | ||
| await self._handle_post_request(ctx) | ||
| except Exception as e: |
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.
This is too broad of an error, we should be more specific about the type of Exception we handle here.
We should at least limit to HTTPStatusError if your problem is with HTTP errors specifically.
Fix: Propagate HTTP errors to client
Motivation and Context
HTTP request failures were not being properly communicated to clients, causing silent failures. This fix ensures errors are sent through the read stream so clients are notified when requests fail.
How Has This Been Tested?
Has been tested in production environments (used by https://github.com/hud-evals/hud-python with a remote server).
Breaking Changes
None - this is a backwards-compatible error handling improvement.
Types of changes
Checklist
Additional context
This change wraps the
handle_request_asyncfunction in a try-catch block and sends any exceptions toctx.read_stream_writerto ensure proper error propagation in the streamable HTTP transport layer.Example scenario where this fix is critical:
HTTP Error Codes (502, 503, 504, etc.)