id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
21034	atomic.__exit__ should check exc_type rather than exc_value	Aymeric Augustin	Aymeric Augustin	"This bug was reported by Thomas Chaumeny after we hit it with our fork of `xact`, which `atomic` derives from.

Under Python 2, in a context manager's `__exit__`, `exc_value` may be `None` even though an exception occured and `exc_type` isn't `None`.

Django currently checks if `exc_value is None` to determine if an exception occured. It should check if `exc_type is None` instead.

Proof:

{{{
% python2.6

>>> class context_manager:
...     def __enter__(self):
...         pass
...     def __exit__(self, type, value, traceback):
...         print('type: %s %s' % (type, type is None))
...         print('value: %s %s' % (value, value is None))

>>> with context_manager():
...     raise ValueError('foo')
... 
type: <type 'exceptions.ValueError'> False
value: foo False
Traceback (most recent call last):
  File ""<stdin>"", line 2, in <module>
ValueError: foo

>>> with context_manager():
...      raise KeyboardInterrupt
... 
type: <type 'exceptions.KeyboardInterrupt'> False
value:  False
Traceback (most recent call last):
  File ""<stdin>"", line 2, in <module>
KeyboardInterrupt

>>> with context_manager():
...      import time; time.sleep(10)
... 
^Ctype: <type 'exceptions.KeyboardInterrupt'> False
value: None True
Traceback (most recent call last):
  File ""<stdin>"", line 2, in <module>
KeyboardInterrupt
}}}

This bug doesn't exist in Python 3:

{{{
% python3.3

>>> class context_manager:
...     def __enter__(self):
...         pass
...     def __exit__(self, type, value, traceback):
...         print('type: {} {}'.format(type, type is None))
...         print('value: {} {}'.format(value, value is None))
... 

>>> with context_manager():
...     raise ValueError('foo')
... 
type: <class 'ValueError'> False
value: foo False
Traceback (most recent call last):
  File ""<stdin>"", line 2, in <module>
ValueError: foo

>>> with context_manager():
...      raise KeyboardInterrupt
... 
type: <class 'KeyboardInterrupt'> False
value:  False
Traceback (most recent call last):
  File ""<stdin>"", line 2, in <module>
KeyboardInterrupt

>>> with context_manager():
...      import time; time.sleep(10)
... 
^Ctype: <class 'KeyboardInterrupt'> False
value:  False
Traceback (most recent call last):
  File ""<stdin>"", line 2, in <module>
KeyboardInterrupt
}}}

This is a release blocker because it's a major bug in a new feature. It might lead to data loss."	Bug	closed	Database layer (models, ORM)	1.6-beta-1	Release blocker	fixed			Accepted	0	0	0	0	0	0