opened
schneiderderek
username_0
Added processors for sanitizing the body of PUT and PATCH requests.
Add PUT and PATCH data processors.
issue
created
nateberkopec
username_1
Definitely OK with this, but rather than separate processor classes, I'd prefer we have a single RequestMethodProcessor.
comment
created
schneiderderek
username_0
@username_1 OK, just to be clear would you want all 3 (POST, PUT, and PATCH) combined into one processor or just PUT and PATCH?
comment
created
nateberkopec
username_1
@username_0 All three. Ideally you would have a constant or something that was just an array of ["POST", "PUT", "PATCH"] that you would use to show which HTTP methods you were scrubbing. Bonus points if you make it configurable.
comment
created
schneiderderek
username_0
@username_1 Ok, my intent with the current design was to provide flexibility and configurability for the end user while using the existing methods for configuring the way data is processed (using processors). IMO by combining these into one processor we lose that. I'll combine them if you disagree.

When you mention making it configurable, were you thinking something like this?:
```ruby
Raven.configure do |config|
  # Probably needs a better config name
  config.request_methods_sanitized_data = ["POST", "PUT"] 
end
```
comment
created
nateberkopec
username_1
Yup, exactly like that.
comment
created
schneiderderek
username_0
@username_1 I've updated the code to combine everything into 1 processor. Let me know what you think.
comment
created
nateberkopec
username_1
One last comment left, then I'm good on this.
comment
created
schneiderderek
username_0
@username_1 I've updated the docs to reflect your feedback.
comment
created
nateberkopec
username_1
Reverting because I want to put this off for 3.0
comment
created
schneiderderek
username_0
@username_1 Is there a 3.0 RC branch available so I could use this functionality now? Or should I just stick to using my fork?
comment
created
nateberkopec
username_1
stick to your fork, I won't have something "release-candidate-worthy" for a while
comment
created
nateberkopec
username_1
also on your fork, you can be reasonably certain that the API you created here won't change and will not be appearing on master until 3.0. so don't worry about conflicts in the meantime with 2.x.
comment
created
dhirendrx
username_2
@username_1 We still haven't included it in the `4.6` release, do we have any plan to include it in a future release.
comment
created
st0012
username_3
@username_2 hey I'm the current maintainer of the SDK 👋 
since version 4.0 (`sentry-ruby`), the concept of SDK-side sanitization (processors) has been removed (like other Sentry SDKs). and the old SDK `sentry-raven` has entered maintenance mode now. so we won't introduce this feature to either one of them in the future.
comment