Opened 3 years ago

Last modified 18 months ago

#21777 new Cleanup/optimization

Make request exception handling more robust to subsequent exceptions

Reported by: patrakov@… Owned by: nobody
Component: HTTP handling Version: 1.6
Severity: Normal Keywords:
Cc: numerodix@…, django@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


In django/core/handlers/, there is the following piece of code:

except: # Handle everything else.

# Get the exception info now, in case another exception is thrown later.
signals.got_request_exception.send(sender=self.class, request=request)
response = self.handle_uncaught_exception(request, resolver, sys.exc_info())

Contrary to the comment, the exception info is saved too late. If, during the got_request_exception signal handling another exception occurs (and gets ignored), it will overwrite, and this subsequent exception will get logged, instead of the original one.

I hit this while playing with MySQL's SSCursor (for exporting a lot of data to CSV). Some exception happened in my application, then Django's default got_request_exception handler kicked in and attempted to rollback. This failed with a ProgrammingError, because not all rows were consumed, and this ProgrammingError went into the traceback e-mail instead of the original exception.

Change History (9)

comment:1 Changed 3 years ago by anonymous

My diagnosis was incomplete. It is not only that exc_info is saved too late. It is also needed to put a try ... except around the signals.got_request_exception.send(...) so that handle_uncaught_exception() is actually called.

comment:2 Changed 3 years ago by Martin Matusiak

Cc: numerodix@… added

comment:3 Changed 3 years ago by ANUBHAV JOSHI

It would be really nice if you could add the code where you came around with this problem....

comment:4 Changed 3 years ago by anonymous

Yes, I understand that it would be a good idea to add a reproducer. However, the original code that led me to this report is under NDA, so I have to make another reproducer. Unfortunately, I will be able to do that only after March, 14th. Sorry for that.

comment:5 Changed 3 years ago by patrakov@…

OK, here is the simplest possible example code.


from django.conf.urls import patterns, include, url

urlpatterns = patterns('',
    url(r'^$', 'bug21777.views.buggyview'),

Relevant part of bug21777/, enough to reproduce the bug:

SECRET_KEY = 'dummy'

DEBUG = False
ADMINS = (('admin', ''),)

EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'



ROOT_URLCONF = 'bug21777.urls'
WSGI_APPLICATION = 'bug21777.wsgi.application'


from django.core.signals import got_request_exception

def buggyview(request):
    raise ValueError("Let's pretend this is a bug in my application")

def handler(signal, sender, **kwargs):
    # The handler is buggy, too
    raise KeyError("Let's pretend that the handler itself is confused by the original exception")


Test with ./ runserver

Note: here it is important to understand the assumptions not expressed in the code above. In my case, the handler was in some sense confused by the original exception, not just "buggy by itself". So the original exception in the view represents the primary problem, and the exception in the handler is just a nasty consequence.

If you modify the handler so that it is non-buggy, and load the site root URL, then you'll notice that Django sends the admin a nice e-mail with a traceback pointing to the actual buggy view function, and with ValueError. This is good. I have not put it into the above example, but in production we also log all such errors to Apache logs, using logging.StreamHandler.

With the buggy handler, as written above, the e-mail is not sent, no debug output mentions the original ValueError anywhere, and the KeyError related to the confused handler gets logged to stderr. This makes it very hard to debug the original problem in the view. In my opinion, the ideal situation would be if both exceptions were properly logged. Especially since the comment mentions the possibility of "another exception" and expresses the intention to avoid overwriting the original exception info.

comment:6 Changed 3 years ago by anonymous

Triage Stage: UnreviewedAccepted

Yup, there's totally a codepath where an exception can escape get_response.

comment:7 Changed 3 years ago by Tim Graham

Type: UncategorizedCleanup/optimization

comment:8 Changed 3 years ago by Daniel Hahler

Cc: django@… added

comment:9 Changed 18 months ago by Tim Graham

Component: Core (Other)HTTP handling
Note: See TracTickets for help on using tickets.
Back to Top