testing-cabal / testtools

Testtools - tasteful testing for python
https://testtools.readthedocs.io/en/latest/
Other
95 stars 88 forks source link

AttributeError: 'IntegrityError' object has no attribute '__traceback__' #162

Closed thomasgoirand closed 8 years ago

thomasgoirand commented 8 years ago

Hi.

Could you please follow-up on the opened Debian bug here? https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=802677

Cheers,

Thomas

jml commented 8 years ago

Hi Thomas,

I don't have ready access to the Debian BTS, so I'm going to update here.

I don't 100% know what's going on here, but I think it is an issue with traceback2 which we started to exhibit after 55278e0e4a935f1d8c468a4d29a8133088806790 when we started to depend on it.

https://github.com/testing-cabal/traceback2/blob/master/traceback2/__init__.py#L449 assumes that if an exception has a __cause__, then the cause has a traceback. Clearly this assumption does not always hold.

__cause__ is normally set when an exception is re-raised using the new Python 3 raise from syntax. Python 3 correctly takes care of setting __traceback__ on __cause__.

However, it turns out that Django has special code for setting __cause__ on database exceptions:

As per PEP-3134, a __cause__ attribute is set with the original (underlying) database exception, allowing access to any additional information provided. (Note that this attribute is available under both Python 2 and Python 3, although PEP-3134 normally only applies to Python 3.)

(from exceptions.txt, some markup edits)

I would suggest that:

  1. Django should be a little bit better behaved, and at least set __cause__.__traceback__ to None
  2. traceback2 should be a bit more tolerant of poorly behaved exceptions, and wrap the access of __traceback__ in a getattr

Hope this helps, jml

rbtcollins commented 8 years ago

traceback2 is a rolling backport of Python3's traceback module. Per https://docs.python.org/3/library/exceptions.html - "raise new_exc from original_exc

The expression following from must be an exception or None. It will be set as __cause__ on the raised exception."

The expectation in Python 3 is that __cause__ is either None or a valid exception object - which implies the __traceback__ attribute. The likely thing here is that Django is setting __cause__ (non-standard in Python2) without also setting the exceptions __traceback__ - which it needs to do by hand on Python2 (just as it's setting __cause__ by hand)

rhertzog commented 8 years ago

For reference I have opened a ticket on the Django side: https://code.djangoproject.com/ticket/25761

It will likely get fixed but it's not a high priority ticket so you might still want to rework the code to not make this assumption... if Django did the mistake, it's quite possible other projects did something similar.

rbtcollins commented 8 years ago

Thanks; the place to do that would be in testing-cabal/traceback2 (which is the python stdlib code for traceback handling with only the necessary adjustments to work on Python2.