Opened 5 years ago

Closed 4 years 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: Jake Rothenbuhler
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 Jake Rothenbuhler 5 years ago.

Download all attachments as: .zip

Change History (9)

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

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 5 years ago by Jannis Leidel

Component: UncategorizedCore (Other)
Summary: NEW POLICY: All naked excepts which raise an exception should include previous exception stack.All naked excepts which raise an exception should include previous exception stack
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Yeah, sounds like a good idea

comment:3 Changed 5 years ago by Daniel Sokolowski

Cc: ognajd@… added

comment:4 Changed 5 years ago by Thomas Güttler

Cc: hv@… added

comment:5 Changed 5 years ago by Jake Rothenbuhler

Owner: changed from nobody to Jake Rothenbuhler

Changed 5 years ago by Jake Rothenbuhler

Attachment: 18003.diff added

comment:6 Changed 5 years ago by Jake Rothenbuhler

Has patch: set

comment:7 Changed 4 years ago by modi.konark@…

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

comment:8 Changed 4 years ago by Carl Meyer <carl@…>

Resolution: fixed
Status: newclosed

In bc4111ba68e5eede1f68882a16d68441a845e30b:

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

Thanks jrothenbuhler for draft patch, Konark Modi for updates.

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