launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:upload-parse-error-msg into launchpad:master.

Commit message:
Adding more information to DebFile parsing when uploading deb package

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/386290
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:upload-parse-error-msg into launchpad:master.

diff --git a/lib/lp/archiveuploader/nascentuploadfile.py b/lib/lp/archiveuploader/nascentuploadfile.py
index d69f25d..52b8b58 100644
--- a/lib/lp/archiveuploader/nascentuploadfile.py
+++ b/lib/lp/archiveuploader/nascentuploadfile.py
@@ -563,10 +563,10 @@ class BaseBinaryUploadFile(PackageUploadFile):
             deb_file = apt_inst.DebFile(self.filepath)
             control_file = deb_file.control.extractdata("control")
             control_lines = apt_pkg.TagSection(control_file, bytes=True)
-        except Exception:
+        except Exception as e:
             yield UploadError(
-                "%s: extracting control file raised %s, giving up."
-                 % (self.filename, sys.exc_type))
+                "%s: extracting control file raised %s: %s. giving up."
+                 % (self.filename, sys.exc_type, e))
             return
 
         for mandatory_field in self.mandatory_fields:
diff --git a/lib/lp/archiveuploader/tests/test_nascentuploadfile.py b/lib/lp/archiveuploader/tests/test_nascentuploadfile.py
index ff63636..61ec589 100644
--- a/lib/lp/archiveuploader/tests/test_nascentuploadfile.py
+++ b/lib/lp/archiveuploader/tests/test_nascentuploadfile.py
@@ -41,7 +41,10 @@ from lp.archiveuploader.nascentuploadfile import (
 from lp.archiveuploader.tests import AbsolutelyAnythingGoesUploadPolicy
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.services.compat import lzma
+from lp.services.compat import (
+    lzma,
+    mock,
+    )
 from lp.services.log.logger import BufferLogger
 from lp.services.osutils import write_file
 from lp.soyuz.enums import (
@@ -517,6 +520,23 @@ class DebBinaryUploadFileTests(PackageUploadFileTestCase):
         uploadfile.extractAndParseControl()
         self.assertEqual([], list(uploadfile.verifyFormat()))
 
+    @mock.patch("lp.archiveuploader.nascentuploadfile.apt_inst")
+    def test_extractAndParseControl_UploadError_message(self, m_apt_inst):
+        # extractAndParseControl should yield a reasonable error message if
+        # apt_inst.DebFile raises an exception
+        m_apt_inst.DebFile.side_effect = KeyError("banana not found")
+        uploadfile = self.createDebBinaryUploadFile(
+            "empty_0.1_all.deb", "main/admin", "extra", "empty", "0.1", None,
+            members=[])
+        errors = list(uploadfile.extractAndParseControl())
+        self.assertEqual(1, len(errors))
+        error = errors[0]
+        self.assertIsInstance(error, UploadError)
+        self.assertEqual(
+            "empty_0.1_all.deb: extracting control file raised "
+            "<type 'exceptions.KeyError'>: u'banana not found'."
+            " giving up.", error.message)
+
     def test_verifyDebTimestamp_SystemError(self):
         # verifyDebTimestamp produces a reasonable error if we provoke a
         # SystemError from apt_inst.DebFile.