Code

Opened 2 years ago

Closed 13 months ago

#18003 closed Cleanup/optimization (fixed)

All naked excepts which raise an exception should include previous exception stack

Reported by: Glenn Washburn <devlopment@…> Owned by: jrothenbuhler
Component: Core (Other) Version: 1.4
Severity: Normal Keywords:
Cc: ognajd@…, hv@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

I propose that there should be a strict policy for any new code being committed and all old code as it is found to be offending that any where there is a naked except, that is an except block that catches all exceptions, and an exception is raised within this block, that the previous exceptions exception call stack be preserved in the new raised exception.

For example, in contrib/admin/views/main.py on line 319 there is a naked except and the caught exception object is wrapped by another exception class and then re-raised. This loses the call stack of the originating exception, thus making it potentially very difficult to debug the original exception.

This issue should fix the above and start a policy discussion.

Attachments (1)

18003.diff (13.8 KB) - added by jrothenbuhler 2 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 2 years ago by Glenn Washburn <devlopment@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I forgot to add. For those who don't know, there is a rarely used form of the "raise" statement as follows:

    raise ExceptionClass, exception_object, traceback

This form will use the provided traceback, as opposed to a traceback starting from the current frame. This is how I propose we preserve the originating exception's call stack. It would look something like the following:

    try:
        ....
    except Exception, e:
        raise ExceptionWrapperClass, ExceptionWrapperClass(e), sys.exc_info[2]

comment:2 Changed 2 years ago by jezdez

  • Component changed from Uncategorized to Core (Other)
  • Summary changed from NEW POLICY: All naked excepts which raise an exception should include previous exception stack. to All naked excepts which raise an exception should include previous exception stack
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/optimization

Yeah, sounds like a good idea

comment:3 Changed 2 years ago by danols

  • Cc ognajd@… added

comment:4 Changed 2 years ago by guettli

  • Cc hv@… added

comment:5 Changed 2 years ago by jrothenbuhler

  • Owner changed from nobody to jrothenbuhler

Changed 2 years ago by jrothenbuhler

comment:6 Changed 2 years ago by jrothenbuhler

  • Has patch set

comment:7 Changed 13 months ago by modi.konark@…

Issued a PR against this ticket / patch : https://github.com/django/django/pull/930

comment:8 Changed 13 months ago by Carl Meyer <carl@…>

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

In bc4111ba68e5eede1f68882a16d68441a845e30b:

Fixed #18003 -- Preserved tracebacks when re-raising errors.

Thanks jrothenbuhler for draft patch, Konark Modi for updates.

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.