Attach the request to SSE errors via request_context#1073
Conversation
Wrap the SSE iteration in request_context so any SSEError raised while consuming the stream carries the originating request, matching how the rest of the client attaches request context to errors. The content-type check no longer needs to pass request explicitly.
|
Docs preview: |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ecd77cbfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Read the nullable _request attribute rather than the Response.request property, which raises when no request is attached. request_context already accepts None. Also assert the request on the async content-type test and cover a requestless EventSource.
Wrap the SSE iteration in
request_contextso anySSEErrorraised while consuming the stream carries the originating request, the same way the client attaches request context to errors elsewhere (e.g.Response.aiter_raw).Behaviour-neutral today: the only
SSEErrorcurrently raised is the content-type check, which already passedrequest=explicitly and continues to carry it - now via the wrapper rather than the explicit argument. This is groundwork for #1071, which adds size-limit errors during iteration that should carry the request too.Extended
test_content_type_mismatch_raisesto assertexc.requestis set.