Fix unintentional exception clobbering when rollback fails

sruggier · sruggier · commit c089649cab7d · 2016-03-25T16:04:17.000-04:00
diff --git a/atomicwrites/__init__.py b/atomicwrites/__init__.py
@@ -116,16 +116,18 @@ def open(self):
     def _open(self, get_fileobject):
         f = None  # make sure f exists even if get_fileobject() fails
         try:
+            success = False
             with get_fileobject() as f:
                 yield f
                 self.sync(f)
             self.commit(f)
-        except:
-            try:
-                self.rollback(f)
-            except Exception:
-                pass
-            raise
+            success = True
+        finally:
+            if not success:
+                try:
+                    self.rollback(f)
+                except Exception:
+                    pass
 
     def get_fileobject(self, dir=None, **kwargs):
         '''Return the temporary file to use.'''
diff --git a/tests/test_atomicwrites.py b/tests/test_atomicwrites.py
@@ -51,3 +51,17 @@ def test_dont_remove_simultaneously_created_file(tmpdir):
     assert excinfo.value.errno == errno.EEXIST
     assert fname.read() == 'harhar'
     assert len(tmpdir.listdir()) == 1
+
+
+# Verify that nested exceptions during rollback do not overwrite the initial
+# exception that triggered a rollback.
+def test_open_reraise(tmpdir):
+    fname = tmpdir.join('ha')
+    with pytest.raises(AssertionError):
+        with atomic_write(str(fname), overwrite=False) as f:
+            # Mess with f, so rollback will trigger an OSError. We're testing
+            # that the initial AssertionError triggered below is propagated up
+            # the stack, not the second exception triggered during rollback.
+            f.name = "asdf"
+            # Now trigger our own exception.
+            assert False, "Intentional failure for testing purposes"
