Design decision for #1625 - Adding traceback to return value from send_robust when error occurs

[Jim D.]

https://code.djangoproject.com/ticket/16245

Here's the quick summary:

send_robust() as you know is a special case of send() in the signals framework, which wraps each signal trigger in a try/except block so that an exception in one signal doesn't prevent the rest from firing.

This is great, but I've run into a problem when using this in production. The problem is that send_robust() does not capture the traceback at the point of the exception. As a result, though you can see which exception was raised, you can't see the traceback assocaited with that exception. This makes debugging exceptions that occur during send_robust extremely, extremely difficult.

I've contributed a patch to resolve this. In order to preserve backwards compatibility, my patch adds a kwarg append_traceback to send_robust() which allows one to optionally ensure the traceback is included. This seemed like the most straightforward solution that steps on the fewest toes, though in my opinion the traceback should be included by default and the old way deprecated.

The patch includes full tests and documentation. Right now it's stuck in design decision needed. Hopefully someone wouldn't mind taking a moment to review the ticket and patch and move it to ready for checkin. If there are concerns or questions or the patch need work I'd be happy to do what is required.

Many thanks...

[Jeremy Dunck]

On Tue, Sep 20, 2011 at 9:00 AM, Jim D. <jim.d...@gmail.com> wrote:
> https://code.djangoproject.com/ticket/16245

I've looked at the patch and it seems good to me.

I have a suggestion:

Rather than (receiver, err, traceback), why not (receiver, exc_info)
where exc_info is the triple returned by sys.exc_info()? There's no
harm in changing the meaning of the 2nd tuple element if people are
opting into the change.

Similarly, rather than append_traceback, why not call it
exc_info=True? This parallels the logging module's use of the kwarg:
http://docs.python.org/library/logging.html#logging.Logger.debug

[Jim D.]

Awesome, great suggestions both. That's a cleaner API and the implementation itself is even slightly cleaner.

I updated the patch and uploaded it to the ticket. If you or anyone else wants to review it and ideally move it forward, that would be excellent.

Thanks!

[Jeremy Dunck]

Well, I meant to mark accepted as endorsement of the patch, but that
made me owner.

I can't own it since I'm not a core committer.

Setting it back to nobody set the status to "new".

Hopefully a core will agree this is a good approach.

[Carl Meyer]

Addressed the Trac-confusion stuff in a new thread to keep this focused.

On Tuesday, September 20, 2011 1:26:22 PM UTC-6, jdunck wrote:

Hopefully a core will agree this is a good approach.

I marked the ticket accepted as its something that should be fixed and the general approach looks fine to me - don't have time at the moment to review the patch in depth.

Carl