Merge pull request #96 from freedomofpress/95-exit-gracefully

Always communicate error messages to clients

securedrop_export/export.py

@@ -102,18 +102,19 @@ def exit_gracefully(self, msg, e=False):
         solutions for mimetype handling, which we want to avoid.
         """
         logger.info("Exiting with message: {}".format(msg))
-        if not e:
+        if e:
+            logger.error("Captured exception output: {}".format(e.output))
+        try:
+            # If the file archive was extracted, delete before returning
+            if os.path.isdir(self.tmpdir):
+                shutil.rmtree(self.tmpdir)
+            # Do this after deletion to avoid giving the client two error messages in case of the
+            # block above failing
             sys.stderr.write(msg)
             sys.stderr.write("\n")
-        else:
-            try:
-                # If the file archive was extracted, delete before returning
-                if os.path.isdir(self.tmpdir):
-                    shutil.rmtree(self.tmpdir)
-                logger.error("{}:{}".format(msg, e.output))
-            except Exception as ex:
-                logger.error("Unhandled exception: {}".format(ex))
-                sys.stderr.write(ExportStatus.ERROR_GENERIC.value)
+        except Exception as ex:
+            logger.error("Unhandled exception: {}".format(ex))
+            sys.stderr.write(ExportStatus.ERROR_GENERIC.value)
         # exit with 0 return code otherwise the os will attempt to open
         # the file with another application
         sys.exit(0)

tests/test_export.py

@@ -2,6 +2,8 @@
 import subprocess  # noqa: F401
 import tempfile
 
+from unittest import mock
+
 import json
 import pytest
 import tarfile
@@ -426,16 +428,18 @@ def test_exit_gracefully_no_exception(capsys):
 
 def test_exit_gracefully_exception(capsys):
     submission = export.SDExport("testfile", TEST_CONFIG)
-    test_msg = "test"
+    test_msg = "ERROR_GENERIC"
 
     with pytest.raises(SystemExit) as sysexit:
-        submission.exit_gracefully(test_msg, e=Exception("BANG!"))
+        exception = mock.MagicMock()
+        exception.output = "BANG!"
+        submission.exit_gracefully(test_msg, e=exception)
 
     # A graceful exit means a return code of 0
     assert sysexit.value.code == 0
 
     captured = capsys.readouterr()
-    assert captured.err == export.ExportStatus.ERROR_GENERIC.value
+    assert captured.err.rstrip() == export.ExportStatus.ERROR_GENERIC.value
     assert captured.out == ""