Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#8136 closed (fixed)

Edge case: AttributeError from test client masks underlying exception

Reported by: danfairs Owned by: nobody
Component: Testing framework Version: master
Severity: Keywords: test client exception signal attributeerror
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Using Django svn r8223. In brief: if a 404 error occurs during a request using the Django test client, and rendering that 404 itself raises an exception (handled by handle_uncaught_exception in handlers/base.py, line 115), and there's no 500.html file, then signals.got_request_exception is never fired, meaning that exc_info in the Django test client (client.py, line 225) is not populated. This causes an AttributeError when response is referred to a few lines later, masking the actual error.

More detail:

My app has a 404.html defined, with errors, but no 500.html file. I have a 404 error in my app, which is exposed using the Django test client. The project is configured in debug mode. The code path through django.core.handlers.base passes through here (lines 106-115 of base.py):

        except http.Http404, e:
            if settings.DEBUG:
                from django.views import debug
                return debug.technical_404_response(request, e)
            else:
                try:
                    callback, param_dict = resolver.resolve404()
                    return callback(request, **param_dict)
                except:
                    return self.handle_uncaught_exception(request, resolver, sys.exc_info())

In this case, callback is a function, page_not_found. This itself ends up raising an exception, due to a syntax error in my 404.html. This is handled by self.handle_uncaught_exception. In my case, this again throws an exception, a TemplateDoesNotExist due to the missing 500.html (so the exception args is set to ('500.html',), and the exception isn't re-raised). The got_request_exception signal is never fired; this means that the handler for this signal set up by the test client to capture the exception information is never executed and, as a side effect of this, the 'response' variable is never assigned, causing an attribute error:

        # Capture exceptions created by the handler.
        got_request_exception.connect(self.store_exc_info)

        try:
            response = self.handler(environ)
        except TemplateDoesNotExist, e:
            # If the view raises an exception, Django will attempt to show
            # the 500.html template. If that template is not available,
            # we should ignore the error in favor of re-raising the
            # underlying exception that caused the 500 error. Any other
            # template found to be missing during view error handling
            # should be reported as-is.
            if e.args != ('500.html',):
                raise

        # Look for a signalled exception, clear the current context
        # exception data, then re-raise the signalled exception.
        # Also make sure that the signalled exception is cleared from
        # the local cache!
        if self.exc_info:
            exc_info = self.exc_info
            self.exc_info = None
            raise exc_info[1], None, exc_info[2]

        # Save the client and request that stimulated the response.
        response.client = self


The shortly-to-be-attached patch (seems to) fix this, but I'm not sure what other consequences it may have.

Attachments (1)

handlers-base-r8223 (820 bytes) - added by danfairs 6 years ago.
Patch that makes sure that got_request_exception is called

Download all attachments as: .zip

Change History (5)

Changed 6 years ago by danfairs

Patch that makes sure that got_request_exception is called

comment:1 Changed 6 years ago by jacob

  • milestone set to 1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by russellm

  • Component changed from Core framework to Unit test system

comment:3 Changed 6 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [8464]) Fixed #8136: Added a signal emission when an error is raised handling an error. This was required for the test client to handle missing 404.html templates and errors in the 404.html template. Thanks to danfairs for the report and fix.

comment:4 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.