Django-rest-framework: Support for sentry/raven

Created on 14 Apr 2016  路  10Comments  路  Source: encode/django-rest-framework

Checklist

  • [x] I have verified that that issue exists against the master branch of Django REST framework.
  • [x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • [x] This is not a usage question. (Those should be directed to the discussion group instead.)
  • [x] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • [x] I have reduced the issue to the simplest possible case.
  • [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

    Steps to reproduce

  1. Configure sentry
  2. Create an API endpoint with POST as required method.
  3. Raise exception in the implementation.
  4. Hit the endpoint with some POST data.
  5. Check sentry for the raised exception.
  6. Check body of the request.

    Expected behavior

It should contain the data that was sent in the request by the user, i.e., the complete POST data.

Actual behavior

It shows there.

This is due to the fact that sentry/raven tries to extract the data from either body or raw_post_data or POST as shown here. In all the cases, its either not implemented or it returns blank dict.

Kindly add support for third-party apps which run on django's implementation model.

Most helpful comment

This might work:

from rest_framework.request import is_form_media_type

class RequestBodyMiddleware(object):
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        """..."""
        if not is_form_media_type(request.content_type):
            request.body  # forces stream to be read into memory
        return self.get_response(request)

In short,

  • multipart/form-data and application/x-www-form-urlencoded requests already have valid POST data, so are compatible with Sentry/Raven.
  • This otherwise forces request.body evaluation for non-form data requests, which are more likely to be smaller in size. This does increase the size of the request, but it's more likely to be acceptable.

All 10 comments

Hello,

Is this being looked into?

Just in case, adding a link to (currently) rather short discussion over at sentry/raven: https://github.com/getsentry/raven-python/issues/591

There is no obvious way to resolve this. REST framework parsers read the request.stream, and the data is then not subsequently available.

Feasible that 'request.data' might end up being a supported interface in Django core, at which point sentry could use that to make the _parsed_ data visible.

Closing due to lack of an actionable outcome at present.

@khazidhea - I was able to work around the issue by adding middleware - https://gist.github.com/eprikazc/32bf9f182d030985cbca80c75aaf6589
The trick is to access request.body before request is handed to REST framework. Then afterwards Sentry is able to retrieve request.body. Seems to be working ok so far.

@tomchristie do you see any possibility to make a proper fix based on idea from the workaround? I'd be happy to prepare pull request, if you provide some hints.
BTW,

REST framework parsers read the request.stream

Not 100% correct - there is no request.stream, there is request._stream, which is not part of HTTPRequest's public API.

Hi @eprikazc.

do you see any possibility to make a proper fix based on idea from the workaround?

Unfortunately this isn't a complete fix, as evaluating request.body reads the entire stream into memory. This is acceptable for a lot of cases, but isn't for file uploads where memory use is a concern. Large files are usually written to a temporary file instead of into memory - this change would subvert that. That said, this middleware works if you don't need to handle file uploads in your API.

Not 100% correct - there is no request.stream, there is request._stream, which is not part of HTTPRequest's public API.

btw, the DRF parser would operate on a DRF Request, not a Django HttpRequest.

Yes, I see. Then what if we evaluate request.body only if request.FILES is empty?

@rpkilby - Nevermind, I see that it does not work. Evaluating request.FILES reads (streams) the request, and after that request.body again throws exception.

This might work:

from rest_framework.request import is_form_media_type

class RequestBodyMiddleware(object):
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        """..."""
        if not is_form_media_type(request.content_type):
            request.body  # forces stream to be read into memory
        return self.get_response(request)

In short,

  • multipart/form-data and application/x-www-form-urlencoded requests already have valid POST data, so are compatible with Sentry/Raven.
  • This otherwise forces request.body evaluation for non-form data requests, which are more likely to be smaller in size. This does increase the size of the request, but it's more likely to be acceptable.

That timing 馃槃

This would work, however I doubt whether it needs to be included in DRF core just for the sake of compatibility with Sentry...

Agreed. This is a very small amount of code that can reasonably exist in user land. Anyone searching for Sentry compat will run across this issue.

Yep, thanks!

Was this page helpful?
0 / 5 - 0 ratings