-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
Add request instead of response data in DioEventProcessor #933
Add request instead of response data in DioEventProcessor #933
Conversation
Codecov Report
@@ Coverage Diff @@
## main #933 +/- ##
=======================================
Coverage 89.81% 89.81%
=======================================
Files 104 104
Lines 3211 3211
=======================================
Hits 2884 2884
Misses 327 327
Continue to review full report at Codecov.
|
This looks good to me. On mobile it makes a lot of sense to record the response since, at least in my experience, that's a lot of the times also a source of errors (getsentry/develop#401). Regarding PII, why does it matter? I mean response and request can contain PII? |
I absolutely agree but it should be consistent in the SDK. |
Makes sense, since the request data is guarded by |
I think adding the response instead of the request was an oversight. The SentryHttpClient also adds the request and not the response. |
📜 Description
Add request instead of response data to
SentryRequest
inDioEventProcessor
💡 Motivation and Context
Response data may contain PII and should not be logged by the framework.
The
SentryRequest.data
is meant to contain request data.See discussion in #624 about PII.
💚 How did you test it?
Ran and adapted the tests
📝 Checklist
🔮 Next steps
CC @ueman