From bee339d79c8ac318573fd54229d99e64fd27617a Mon Sep 17 00:00:00 2001
From: D G Lee <yqli2016@fiberhome.com>
Date: Mon, 8 May 2017 11:11:46 +0800
Subject: [PATCH] Add 'save_and_reraise_exception' method when call
 'session.rollback()'

Maybe we need to know the original exception when exception occurs on
method 'session.rollback()'.

Change-Id: Iccda37a3279d6f24de69cfccef3e29787f1ac694
Closes-Bug: #1684329
---
 oslo_db/sqlalchemy/enginefacade.py            |  9 +++----
 oslo_db/tests/sqlalchemy/test_enginefacade.py | 26 +++++++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py
index 88bca6b0..82ebbb11 100644
--- a/oslo_db/sqlalchemy/enginefacade.py
+++ b/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
@@ -627,12 +628,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
diff --git a/oslo_db/tests/sqlalchemy/test_enginefacade.py b/oslo_db/tests/sqlalchemy/test_enginefacade.py
index 566dd366..85246834 100644
--- a/oslo_db/tests/sqlalchemy/test_enginefacade.py
+++ b/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
@@ -1637,6 +1639,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(