Skip to content
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

django-rest-framework: UnsupportedMediaType crashes post_exception_handler #314

Closed
KPilnacek opened this issue May 16, 2019 · 3 comments · Fixed by #335
Closed

django-rest-framework: UnsupportedMediaType crashes post_exception_handler #314

KPilnacek opened this issue May 16, 2019 · 3 comments · Fixed by #335
Assignees

Comments

@KPilnacek
Copy link

KPilnacek commented May 16, 2019

Hello,

we are happy users of rollbar but recently we came across the following problem:
When we use the following settings:

REST_FRAMEWORK = {
    'EXCEPTION_HANDLER': 'rollbar.contrib.django_rest_framework.post_exception_handler'
}

and UnsupportedMediaType error is raised, the request fails with 500 status instead of 415.

The problem is in the line 21

context['request']._request.POST = context['request'].data

where the data attribute tries to read the data but these cannot be parsed as the media type is not supported.

For now, we created the following workaround:

from rest_framework.exceptions import APIException, UnsupportedMediaType, ParseError
from rollbar.contrib.django_rest_framework import post_exception_handler, RestFrameworkExceptionHandler

def custom_exception_handler(exc: Union[Exception, APIException], context: dict) -> Response:
    # Call Rollbar's post exception handler first, to get the standard error response.
    try:
        return post_exception_handler(exc, context)
    except (UnsupportedMediaType, ParseError):
        return RestFrameworkExceptionHandler(exc, context)

But is there a proper way how to handle this?
With regards
Krystof

@ArturMoczulski
Copy link
Contributor

This will need to be addressed in the upcoming release. Thanks for sharing your temporary solution for now :)

@ArturMoczulski ArturMoczulski self-assigned this Jun 8, 2019
@KPilnacek
Copy link
Author

My pleasure :)

@terencehonles
Copy link
Contributor

It looks like this was never fixed as promised, but I fixed it with PR #335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants