Vered Volansky has uploaded a new change for review.
Change subject: misc: Fix exception re-rasing in RollbackContext
......................................................................
misc: Fix exception re-rasing in RollbackContext
Former re-rasing is faulty, as it doesn't take into consideration the
possibility of a tuple being thrown. This can occur when accessing a
dictionary with a missing key, for instance. In this case dictionary raises a
tuple, i.e,
{}['missingKey'], raises the tuple ('missingKey',).
In the former implementation, the variable firstException is assinged with
exc_value,
which, in the above case, is a tuple. When it's re-raised (raise
firstException, None, traceback),
the type of the exception is the class of the original exc_value,
which is the tuple ('missingKey',), as is (implicitly) exc_value.
This scenario is not supported by the raise statement,
which is not supposed to receive a tuple as the first expression.
This is then being interpreted as a string by python and we receive the
following:
"TypeError: exceptions must be old-style classes or derived from BaseException,
not str",
instead of the exception we were expecting.
In this patch, the first exception is saved and rethrown with the original
order and values of
exc_type, exc_value, traceback.
A test (testKeyErrorException) is added to miscTests to verify that the above
scenario is handled correctly,
a test which fails when tested on the previous RollbackContext implementation.
Change-Id: I0052717e2307ad6fec2225b3ad5f438c5a60e1c6
Signed-off-by: Vered Volansky <[email protected]>
---
M tests/miscTests.py
M vdsm/storage/misc.py
2 files changed, 20 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/22860/1
diff --git a/tests/miscTests.py b/tests/miscTests.py
index fb1191b..c726908 100644
--- a/tests/miscTests.py
+++ b/tests/miscTests.py
@@ -1141,6 +1141,21 @@
self.fail("Exception was not raised")
+ def testKeyErrorException(self):
+ """
+ KeyError is raised as a tuple and not expection. Re-rasing it
+ should be aware of this fact and handled carfully.
+ """
+ try:
+ with misc.RollbackContext():
+ {}['aKey']
+ except KeyError:
+ return
+ except Exception:
+ self.fail("Wrong exception was raised")
+
+ self.fail("Exception was not raised")
+
class FindCallerTests(TestCaseBase):
def _assertFindCaller(self, callback):
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index 48020b6..f1d8ab0 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -699,21 +699,17 @@
return self
def __exit__(self, exc_type, exc_value, traceback):
- firstException = exc_value
-
for undo, args, kwargs in self._finally:
try:
undo(*args, **kwargs)
- except Exception as e:
+ except Exception:
# keep the earliest exception info
- if not firstException:
- firstException = e
- # keep the original traceback info
- traceback = sys.exc_info()[2]
+ if exc_type is None:
+ exc_type, exc_value, traceback = sys.exc_info()
# re-raise the earliest exception
- if firstException is not None:
- raise firstException, None, traceback
+ if exc_type is not None:
+ raise exc_type, exc_value, traceback
def defer(self, func, *args, **kwargs):
self._finally.append((func, args, kwargs))
--
To view, visit http://gerrit.ovirt.org/22860
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0052717e2307ad6fec2225b3ad5f438c5a60e1c6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
