Merge chrism-fix.nonretryablecommitexceptions branch via

svn merge -r127104:127105 svn+ssh://svn.zope.org/repos/main/transaction/branches/chrism-fix.nonretryablecommitexceptions

CHANGES.txt

@@ -4,7 +4,11 @@ Changes
 1.3.1 (unreleased)
 ------------------
 
-- TBD
+- When a non-retryable exception was raised as the result of a call to
+  ``transaction.manager.commit`` within the "attempts" machinery, the
+  exception was not reraised properly.  Symptom: an unrecoverable exception
+  such as ``Unsupported: Storing blobs in <somestorage> is not supported.``
+  would be swallowed inappropriately.
 
 1.3.0 (2012-05-16)
 ------------------

transaction/_manager.py

@@ -16,6 +16,7 @@
 It coordinates application code and resource managers, so that they
 are associated with the right transaction.
 """
+import sys
 import threading
 
 from zope.interface import implementer
@@ -24,6 +25,7 @@ from transaction.weakset import WeakSet
 from transaction._transaction import Transaction
 from transaction.interfaces import ITransactionManager
 from transaction.interfaces import TransientError
+from transaction.compat import reraise
 
 
 # We have to remember sets of synch objects, especially Connections.
@@ -152,6 +154,13 @@ class Attempt(object):
     def __init__(self, manager):
         self.manager = manager
 
+    def _retry_or_raise(self, t, v, tb):
+        retry = self.manager._retryable(t, v)
+        self.manager.abort()
+        if retry:
+            return retry # suppress the exception if necessary
+        reraise(t, v, tb) # otherwise reraise the exception
+        
     def __enter__(self):
         return self.manager.__enter__()
 
@@ -159,13 +168,8 @@ class Attempt(object):
         if v is None:
             try:
                 self.manager.commit()
-            except TransientError:
-                self.manager.abort()
-                return True # swallow
             except:
-                self.manager.abort()
-                return False # don't swallow
+                return self._retry_or_raise(*sys.exc_info())
         else:
-            retry = self.manager._retryable(t, v)
-            self.manager.abort()
-            return retry # swallow exception if True, else don't swallow
+            return self._retry_or_raise(t, v, tb)
+        

transaction/tests/test_attempt.py

@@ -21,8 +21,9 @@ class TestAttempt(unittest.TestCase):
     def test___exit__no_exc_nonretryable_commit_exception(self):
         manager = DummyManager(raise_on_commit=ValueError)
         inst = self._makeOne(manager)
-        result = inst.__exit__(None, None, None)
-        self.assertFalse(result)
+        self.assertRaises(ValueError, inst.__exit__, None, None, None)
+        self.assertTrue(manager.committed)
+        self.assertTrue(manager.aborted)
 
     def test___exit__no_exc_abort_exception_after_nonretryable_commit_exc(self):
         manager = DummyManager(raise_on_abort=ValueError, 
@@ -53,8 +54,7 @@ class TestAttempt(unittest.TestCase):
     def test___exit__with_exception_value_nonretryable(self):
         manager = DummyManager()
         inst = self._makeOne(manager)
-        result = inst.__exit__(KeyError, KeyError(), None)
-        self.assertFalse(result)
+        self.assertRaises(KeyError, inst.__exit__, KeyError, KeyError(), None)
         self.assertFalse(manager.committed)
         self.assertTrue(manager.aborted)