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 Tim Graham, 10 years ago

So in the djangosnippets middleware you want to replace return technical_500_response(request, *exc_info) with something like return BaseHandler().handle_uncaught_exception(request, resolver, exc_info, debug=True)? That still wouldn't achieve the same thing as your last snippet as SafeExceptionReporterFilter checks settings.DEBUG. I think you should instead use a custom settings.DEFAULT_EXCEPTION_REPORTER_FILTER and subclass django.views.debug.SafeExceptionReporterFilter overriding the is_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.

comment:2 by Daniel Hahler, 10 years ago

replace return technical_500_response(request, *exc_info) with something like return 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 subclass django.views.debug.SafeExceptionReporterFilter overriding the is_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 Tim Graham, 10 years ago

Resolution: wontfix
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top