fix(h2_connection_reset): add stream reset when client cancel the req… #944
Conversation
7455935 to
9924db4
Compare
There was a problem hiding this comment.
Thanks, seems like a good start. Make sure to run scripts/unasync.py to also apply the changes to the httpcore/_sync/http2.py module.
https://github.com/encode/httpcore/actions/runs/10464774808/job/28978912844#step:5:26
(Related... maybe we could change the build logs when unasync --check fails, so that there's clearer feedback to the developer.)
46031e9 to
5997901
Compare
af2e7e4 to
c061e4f
Compare
bd4f039 to
18422ff
Compare
|
sorry to bother but we are really in a hurry to fix this one, could you have a review again? @tomchristie |
|
approved by langchain community now, shall we have any further progress on this one? @tomchristie |
| # need send cancel frame when the exception is not from remote peer. | ||
| if not isinstance(exc, RemoteProtocolError): |
There was a problem hiding this comment.
need send cancel frame when the exception is not from remote peer.
It's not obvious to me that not isinstance(exc, RemoteProtocolError) is correct/sufficient here.
Should this instead be something like just wrapping the yield?...
try:
yield chunk
except Exception as exc:
self._connection._reset_steam(
stream_id=self._stream_id,
error_code=h2.settings.ErrorCodes.CANCEL,
)
raise exc
Thanks @MarkLux - apologies for the delay it's a bit of a complex one to review. Thoughts on my review comment? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Summary
add the missing connection reset of h2 connection when client cancel the request.
fix #943
in discussion with #941