send_robust should include traceback in response when Exception occurs

[Jim Dalton]

I have cron job that runs a task which sends a signal via send_robust. I ran into a wall recently in trying to configure better logging for this job. The basic problem is that I can't access the traceback for any exception that occurred in a receiver, which makes debugging in my situation something of a nightmare.

As it stands now, if an exception occurs in a receiver when a send_robust signal is sent, a tuple is added to the responses list which contains the receiver function and the exception.

I would like to propose that we include the traceback as a third item in the tuple. With the exception and traceback, it is possible to construct the (type, value, traceback) returned by sys.exc_info() (and used by the Python logging module), so that exceptions in receiver functions can be more effectively debugged.

A patch will require no more than a line or two in production and test code and probably a minor documentation tweak.

I will submit a patch for this shortly, unless anybody presents any objections or concerns.

[Jim Dalton]

Patch for this ticket attached.

The most significant aspect of this patch is the change in the documentation. I had to reword the signals documentation a bit to make clear that a three tuple with the traceback is appended to the list returned by send_robust() instead of the usual (receiver, response) tuple pair.

I spent a bit of time considering whether adding the traceback to the tuple was the cleanest approach. It muddies the API a bit, since previously a tuple pair was always appended no matter what. It has the advantage of not breaking any existing code since the first two items in the tuple are exactly the same as they were before the change.

In researching this patch I also learned that all this is unnecessary in Python 3.0. Per PEP 3134 (​http://www.python.org/dev/peps/pep-3134/), Exception objects in Python 3.0 have a traceback attribute which hold the traceback object for the call stack at the point where the exception occurred.

[Aymeric Augustin]

Replying to jsdalton:

I spent a bit of time considering whether adding the traceback to the tuple was the cleanest approach. It muddies the API a bit, since previously a tuple pair was always appended no matter what. It has the advantage of not breaking any existing code since the first two items in the tuple are exactly the same as they were before the change.

I don't understand; what about:

for receiver, err in my_signal.send_robust():
    ...

[anonymous]

Replying to aaugustin:

I don't understand; what about:

for receiver, err in my_signal.send_robust():
    ...

Apologies if I was not clear in my ticket description. The traceback containing the call stack at the point when the exception occurred is not available in the exception instance returned (or the receiver function).

Imagine a situation for example where you had five receiver functions being called for a given signal execution. Let's pretend that an unhandled exception was thrown in three of those. You would like to log not only what error occurred in each receiver, but also the call stack leading to the error so it can be effectively debugged. There is no way to obtain the call stack from the error message returned by send_robust(), and even something like sys.last_traceback() will only give you the previous traceback, not all three.

Does that make sense?

Of course, If I'm overlooking some completely obvious way to get at the traceback, please do let me know..

[Aymeric Augustin]

I understand the problem you're trying to solve.

What I don't understand is why you say your solution is backwards compatible. I think the code I've shown is a common pattern, and it would break after your patch, wouldn't it?

[Jim Dalton]

Replying to aaugustin:

I understand the problem you're trying to solve.

What I don't understand is why you say your solution is backwards compatible. I think the code I've shown is a common pattern, and it would break after your patch, wouldn't it?

Ah, thanks. I misunderstood your point of concern. You are indeed correct. I overlooked this usage pattern for a list of tuples, which would break with this change.

I considered two other API alternatives, though I was not at the time particularly satisfied with either:

• Rather than returning the error instance as the second argument to the tuple pair, return the three tuple (type, value, traceback) -- i.e. the same three tuple returned by sys.exc_info(), where type is the Exception class and value is the exception instance. This of course also breaks current implementations.

• Add a private (or otherwise) variable to the exception instance, e.g. __traceback__, a la Python 3. I mentioned this in my first comment above. This solution does not break current implementations. It solves the problem. It's also in keeping with a design decision that the larger Python community agreed is a "good" one, i.e. to attach call stack information about the error to the exception instance.

I'm not sure which of these alternatives I prefer. Since my original solution breaks current implementations, I'm probably least satisfied with it. The first alternative I propose (adding the three tuple in place of the error instance) is logical and also useful. For example, if you're passing error information on to a logging object (that's my particular use case), you can pass that second item in the tuple directly to the exc_infokwarg and the logging class will handle communicating all the necessary info for you. I think this is the "best" solution if it was agreed that it was acceptable to break current implementations with this change.

The third solution (adding a traceback attribute) has the advantage of being highly unlikely to break existing implementations. It's possible that it still could break them, e.g. if __traceback__ was the chosen attribute and someone was already assigning that during error handing, and this overwrote it. It's of course highly unlikely that the new traceback would be different than the one assigned, but the possibility exists. It also feels a bit weird to me to just add an arbitrary attribute to an object we know nothing about, except that it's probably an exception object. There may be stronger, more logical reasons than just "it feels weird". Anyhow, strong advantage of this is it doesn't break current implementation, weakness is it's the messiest and probably most confusing and "magical" seeming.

Not sure if this is the appropriate place to continue this discussion, and perhaps it should be moved to the developers list. But if anyone has feedback on the above I'd be interested in hearing it.

[Jim Dalton]

Another idea, after giving this a bit more thought.

Perhaps my original solution could be made backwards compatible by adding a kwarg to send_robust() as follows:

def send_robust(append_traceback=False):
   ...

If append_traceback is set to true, then the traceback is included as a third item in the appended tuple. If not it returns a tuple pair as it does at present.

Existing implementations would work as is, but if for some reason you required the traceback for debugging or logging purposes, you could set the flag and be sure you were handling the returned list properly.

This seems like the least intrusive change we could make assuming it's agreed that my ticket is worth accepting.

[Jim Dalton]

I have updated the patch to reflect my last comment. Adding the traceback as a third item the tuple response is now an option via append_traceback, the default for which is False.

Existing implementation of send_robust() will now work as is. To make use of my proposed feature, you would need to set append_traceback to True when calling send_robust().

If anyone has any objections or concerns I'd like to hear them. Accessing the traceback at the point an exception occurs in a receiver function is currently not possible. This makes it difficult if not impossible to debug if something goes wrong in one of the receiver functions.

[Jim Dalton]

Changed the API in response to good feedback from jdunck on the developers list here (​https://groups.google.com/d/topic/django-developers/iyI2avkuIpM/discussion). Basically the changes from the previous version are:

• Change name of kwarg from "append_traceback" to "exc_info"
• Instead of appending the traceback as the third item in the response, instead just return the tuple from sys.exc_info(), which contains the exception and traceback.

Updated patch (including updated tests and documentation) attached.

[Jeremy Dunck]

+1 on this patch.

[Jeremy Dunck]

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

[development@…]

Replying to jsdalton:

In researching this patch I also learned that all this is unnecessary in Python 3.0. Per PEP 3134 (​http://www.python.org/dev/peps/pep-3134/), Exception objects in Python 3.0 have a traceback attribute which hold the traceback object for the call stack at the point where the exception occurred.

If traceback is an attribute of the exception in python 3.0, then why don't we emulate that here? Add the traceback as an attribute of the returned exception.

Either way, I hope this is able to get into 1.4. Its definitely needed.

[Unai Zalakain]

+1 on the last proposed solution (and second solution given in comment:5): if not in python3, just emulate it.

[Unai Zalakain]

Mailing list discussion: ​https://groups.google.com/d/msg/django-developers/nyF8wb-GUaI/m14hBDZ49AwJ
I'm submitting a patch with the __traceback__ way.

[Unai Zalakain]

PR: ​https://github.com/django/django/pull/1831

[Tim Graham <timograham@…>]

In ebb0279f4a7a7155c44c09506bbe5b1f9acc83a2:

Fixed #16245 -- Included traceback in send_robust()'s response

Exceptions from the (receiver, exception) tuples returned by
send_robust() now have always their traceback attached as their
traceback argument.

[GitHub <noreply@…>]

In 435e4bf3:

Refs #23919 -- Removed traceback setting needed for Python 2.

Partially reverted refs #25761 and refs #16245.