Merge "Add 'save_and_reraise_exception' method when call 'session.rollback()'"

oslo_db/sqlalchemy/enginefacade.py

@@ -20,6 +20,7 @@ import warnings
 
 import debtcollector.removals as removals
 from oslo_config import cfg
+from oslo_utils import excutils
 
 from oslo_db import exception
 from oslo_db import options
@@ -636,12 +637,8 @@ class _TransactionContext(object):
                     yield self.session
                 self._end_session_transaction(self.session)
             except Exception:
-                self.session.rollback()
-                # TODO(zzzeek) do we need save_and_reraise() here,
-                # or do newer eventlets not have issues?  we are using
-                # raw "raise" in many other places in oslo.db already
-                # (and one six.reraise()).
-                raise
+                with excutils.save_and_reraise_exception():
+                    self.session.rollback()
             finally:
                 self.session.close()
                 self.session = None

oslo_db/tests/sqlalchemy/test_enginefacade.py

@@ -13,6 +13,7 @@
 import collections
 import contextlib
 import copy
+import fixtures
 import pickle
 import warnings
 
@@ -24,6 +25,7 @@ from sqlalchemy import Column
 from sqlalchemy import Integer
 from sqlalchemy import MetaData
 from sqlalchemy.orm import mapper
+from sqlalchemy.orm import Session
 from sqlalchemy import select
 from sqlalchemy import String
 from sqlalchemy import Table
@@ -1654,6 +1656,30 @@ class LiveFacadeTest(test_base.DbTestCase):
             session.query(self.User.name).scalar()
         )
 
+    @mock.patch.object(Session, 'commit')
+    @mock.patch.object(Session, 'rollback')
+    def test_save_and_reraise_when_rollback_exception(self,
+                                                      rollback_patch,
+                                                      commit_patch):
+        context = oslo_context.RequestContext()
+        log = self.useFixture(fixtures.FakeLogger())
+
+        class RollbackException(Exception):
+            pass
+
+        class CommitException(Exception):
+            pass
+
+        commit_patch.side_effect = CommitException()
+        rollback_patch.side_effect = RollbackException()
+
+        @enginefacade.writer
+        def go_session(context):
+            context.session.add(self.User(name="u1"))
+
+        self.assertRaises(RollbackException, go_session, context)
+        self.assertIn('CommitException', log.output)
+
     def test_flush_on_subtransaction(self):
         facade = enginefacade.transaction_context()
         facade.configure(