Opened 10 years ago
Closed 10 years ago
#23002 closed Cleanup/optimization (wontfix)
handle_uncaught_exception: add support for debug=True kwarg / factor request.error logging out
Reported by: | Daniel Hahler | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
It would be useful if you could call handle_uncaught_exception
with debug=True
explicitly (which would default to settings.DEBUG
).
Additionally, the he logging part in handle_uncaught_exception
could be moved into a separate function:
logger.error('Internal Server Error: %s', request.path, exc_info=exc_info, extra={ 'status_code': 500, 'request': request } )
(Source: https://github.com/django/django/blob/master/django/core/handlers/base.py#L232-238)
This would make it easier to provide consistent behavior with e.g. middlewares that want to provide technical_500_response
(e.g. "Technical 500 by group membership", https://djangosnippets.org/snippets/1719/#c5852).
While they could copy the code to call the logger
, it seems to be better having a consistent method that can be called.
The following code might work just as well, but is more likely to break (and changing settings
in the code should not get done IIRC):
def get_technical_500_response_with_debug(): from django.conf import settings debug_orig = settings.DEBUG settings.DEBUG = True r = technical_500_response(request, *exc_info) settings.DEBUG = debug_orig return r
Change History (3)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
replace
return technical_500_response(request, *exc_info)
with something likereturn BaseHandler().handle_uncaught_exception(request, resolver, exc_info, debug=True)
?
Yes.
That still wouldn't achieve the same thing as your last snippet as SafeExceptionReporterFilter checks settings.DEBUG.
I see.
From what I understand it would apply filtering, which does not happen with settings.DEBUG
?
I would consider this to be a good thing in this context, i.e. the sensitive information in the technical info should be filtered.
I think you should instead use a custom
settings.DEFAULT_EXCEPTION_REPORTER_FILTER
and subclassdjango.views.debug.SafeExceptionReporterFilter
overriding theis_safe
method with the same condition as in your middleware.
Without settings.DEBUG
this won't return debug.technical_500_response
though?
Or did you mean "additionally" instead of "instead", i.e. to get the same behavior (filtering) as with settings.DEBUG
?
comment:3 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Oh, I didn't know you *did* want filtering.
Well, I don't think we should make BaseHandler()
a public API and modify it as you suggest. Just copy the lines you want into your middleware. It's easy and much simpler.
So in the djangosnippets middleware you want to replace
return technical_500_response(request, *exc_info)
with something likereturn BaseHandler().handle_uncaught_exception(request, resolver, exc_info, debug=True)
? That still wouldn't achieve the same thing as your last snippet asSafeExceptionReporterFilter
checkssettings.DEBUG
. I think you should instead use a customsettings.DEFAULT_EXCEPTION_REPORTER_FILTER
and subclassdjango.views.debug.SafeExceptionReporterFilter
overriding theis_safe
method with the same condition as in your middleware.Summary: I think there are enough hooks now to accomplish what you want. If you want to write a patch, I will look at it though.