-
Notifications
You must be signed in to change notification settings - Fork 561
fix: Read request body after handler so receive stream is not deprived #4832
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: master
Are you sure you want to change the base?
fix: Read request body after handler so receive stream is not deprived #4832
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4832 +/- ##
==========================================
+ Coverage 84.58% 84.83% +0.25%
==========================================
Files 158 158
Lines 16521 16595 +74
Branches 2867 2877 +10
==========================================
+ Hits 13974 14079 +105
+ Misses 1703 1670 -33
- Partials 844 846 +2
|
if should_send_default_pii(): | ||
request_info = event.setdefault("request", {}) | ||
request_info["api_target"] = "graphql" | ||
request_info["data"] = {"query": source} |
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.
May need to change because the value associated with event["request"]["data"]
was previously subject to request body size limits.
Closing for the reasons in #4764 (comment) |
Reopening because there may be the option of changing data fields on transactions in a minor version. Users with our FastAPI integration are facing empty receive streams when using |
Description
Ensure endpoint handler runs before the FastAPI or Starlette integrations read the request body.
As the integrations previously called high-level Starlette
Request
methods early, the underlying ASGI receive stream was consumed before the user's handler or middleware ran. While theRequest
object caches the request body, any handler or middleware that relies on low-level stream access still failed.Issues
Closes #4764
Closes #4827
Closes PY-1851
Closes PY-1831
Reminders
tox -e linters
.feat:
,fix:
,ref:
,meta:
)