Code

Opened 7 years ago

Closed 6 years ago

#4701 closed (fixed)

If a view raises a SystemExit exception, the BaseHandler should allow the exception to propagate up the stack (I think)

Reported by: Chris Wagner <cw264701@…> Owned by: nobody
Component: django-admin.py runserver Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

What the BaseHandler does at present, is this:

except SystemExit:
    pass # See http://code.djangoproject.com/ticket/1023

What I expect it to do is this:

except SystemExit:
    raise

I found this to be a problem when testing. I, at one point, placed a "raise SystemExit()" in one of my views, for debugging purposes. Later on, (well after forgetting about that "raise SystemExit()" I placed earlier) I was having a test failure, like this:

...
  File "/usr/local/lib/python2.5/site-packages/django/test/client.py", line 44, in __call__
    response = middleware_method(request, response)
  File "/usr/local/lib/python2.5/site-packages/django/contrib/sessions/middleware.py", line 87, in process_response
    patch_vary_headers(response, ('Cookie',))
  File "/usr/local/lib/python2.5/site-packages/django/utils/cache.py", line 105, in patch_vary_headers
    if response.has_header('Vary'):
AttributeError: 'NoneType' object has no attribute 'has_header'

Although the problem was a bit more clear once I had a look at the view that was under test (duh), the problem was not at all clear from the failing test's error message. The raised SystemExit exception should have caused the test to halt execution, however, since the BaseHandler essentially ignores the SystemExit exception, the BaseHandler.get_response() method ends up returning None (implicitly, because nothing else is returned).

I suspect this could cause pain elsewhere (if only when debugging); it just doesn't seem right to "pass" on such an exception

Browsing over #1023, it doesn't look as if there was ever a clear answer as to why using pass was chosen over using raise, to deal with this special-case exception.

Attachments (1)

0020-Don-t-hide-SystemExit-in-base-request-handler.patch (1015 bytes) - added by Bastian Kleineidam <calvin@…> 6 years ago.

Download all attachments as: .zip

Change History (3)

comment:1 Changed 6 years ago by Bastian Kleineidam <calvin@…>

  • Has patch set

+1 for this. I use it my own tree. For completeness sake, I attached a patch

Changed 6 years ago by Bastian Kleineidam <calvin@…>

comment:2 Changed 6 years ago by jacob

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

(In [7168]) Fixed #4701 -- sys.exit() will no longer get swallowed by the http handler. Slightly backwards compatible, perhaps, but you really shouldn't be sys.exit()ing in view code anyway. Thanks, Bastian Kleineidam.

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.