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

Incorrect Error Handling in Django #418

Closed
patforna opened this issue Feb 19, 2018 · 2 comments
Closed

Incorrect Error Handling in Django #418

patforna opened this issue Feb 19, 2018 · 2 comments

Comments

@patforna
Copy link

patforna commented Feb 19, 2018

The code in contrib/django/middleware.py has an issues related to how django handles errors:

When a view throws an exception, TraceExceptionMiddleware.process_exception gets called and will set the span.error = 1. This is an issue, because other middleware might handle the exception and turn it into a non-500 response (e.g. raise Http404 will be handled by django and turned into a 404 response, which should have span.error = 0).

Potential Fix

One way to fix this would be to set span.error in process_response depending on the response's status code. For example:

 def process_response(self, request, response):
    ...
    span.error = 0 if response.status_code < 500 else 1
    span.finish()
    ...

This issue is related to a similar issue which has been reported and fixed for flask in 0.10.1 (see Issue 390)

@Kyle-Verhoog
Copy link
Member

This should be fixed in #462! Let us know if there is any issue still 😄

@palazzem
Copy link

Hello @patforna ! because the PR has been merged, do you have a way to check if it's working properly? we're about to ship the 0.12.1 next week, but want to be sure that problem can be considered closed. Thank you very much!

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

No branches or pull requests

3 participants