Merge lp:~mvo/software-center/recoverable-errors-lp1051829 into lp:software-center

[Michael Vogt]

This branch adds a custom DuplicateSignature to help erorrs.ubuntu.com
sort the various types of transaction faiures into different buckets so
that its easier to idenitify common problem types.

It also skips dpkg errors as those are recorded seperately (and are
usually not easily recoverable anyway).

[Evan (ev)]

This will require a change to lp:daisy, which I'll make now.

[Evan (ev)]

The change is in lp:daisy r172, but we're waiting for it to be deployed in RT 56667.

=== modified file 'softwarecenter/backend/installbackend_impl/aptd.py'
--- softwarecenter/backend/installbackend_impl/aptd.py	2012-10-08 08:04:43 +0000
+++ softwarecenter/backend/installbackend_impl/aptd.py	2012-10-11 08:58:21 +0000
@@ -1008,6 +1008,10 @@
         if pkgname and pkgname in self.pending_transactions:
             del self.pending_transactions[pkgname]
 
+        # calculate a dupes_signature here to have different buckets
+        # on errors.ubuntu.com for the different crash types
+        dupes_signature = "software-center:trans-failed:%s" % trans.error_code
+
         self.emit("transaction-stopped", result)
         if isinstance(error, dbus.DBusException):
             # ignore errors that the user knows about already (like
@@ -1025,21 +1029,27 @@
                          "the authentication service is not available. "
                          "(%s") % error
                 # send to apport for reporting
-                self._call_apport_recoverable_error(text, error)
+                self._call_apport_recoverable_error(
+                    text, error, dupes_signature)
                 # ... and display as a dialog
                 self.ui.error(None, summary, text)
                 return
 
         # lintian errors are ignored and not send to apport_recoverable_error
-        if trans.error_code == enums.ERROR_INVALID_PACKAGE_FILE:
+        # and dpkg errors as well as they will already be recorded seperately
+        # by apt itself
+        if trans.error_code in (enums.ERROR_INVALID_PACKAGE_FILE,
+                                enums.ERROR_PACKAGE_MANAGER_FAILED):
             return
 
         # show a apport recoverable error dialog to the user as we want
         # to know about these issues
         self._call_apport_recoverable_error(
-            _("There was an error submitting the transaction"), error)
+            _("There was an error submitting the transaction"),
+            error,
+            dupes_signature)
 
-    def _call_apport_recoverable_error(self, text, error):
+    def _call_apport_recoverable_error(self, text, error, dupes_signature):
         """Call apports recoverable_problem dialog """
 
         # ensure we have a proper expection string in the report
@@ -1048,10 +1058,13 @@
 
         # mvo: I don't think we need to send "Package\0software-center",
         #      apport should figure this out itself
-        data = "DialogBody\0%(text)s\0Traceback\0%(error)s" % {
+        data = ("DialogBody\0%(text)s\0"
+                "Traceback\0%(error)s\0"
+                "DuplicateSignature\0%(dupes_signature)s" % {
                 'text': text,
                 'error': error,
-                }
+                'dupes_signature': dupes_signature,
+                })
         # This will be quick as it just writes the report file. Then
         # the report gets picked up asynchronously by a inotify watch
         # and displayed to the user in a seperate process.

=== modified file 'tests/test_aptd.py'
--- tests/test_aptd.py	2012-10-05 08:39:29 +0000
+++ tests/test_aptd.py	2012-10-11 08:58:21 +0000
@@ -159,7 +159,8 @@
         mock_popen_instance.communicate.return_value = ("stdout", "stderr")
         mock_popen_instance.returncode = 0
         mock_popen.return_value = mock_popen_instance
-        self.aptd._call_apport_recoverable_error("msg", "traceback-error")
+        self.aptd._call_apport_recoverable_error(
+            "msg", "traceback-error", "custom:dupes:signature")
         mock_popen.assert_called_with(
             [APPORT_RECOVERABLE_ERROR], stdin=subprocess.PIPE, 
             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -167,7 +168,10 @@
         args, kwargs = mock_popen_instance.communicate.call_args
         self.assertEqual(
             kwargs["input"].split("\0"),
-            [ 'DialogBody', 'msg','Traceback', 'traceback-error'])
+            [ 'DialogBody', 'msg',
+              'Traceback', 'traceback-error',
+              'DuplicateSignature', 'custom:dupes:signature',
+            ])
 
     def test_ignore_bad_packages(self):
         mock_trans = Mock(aptdaemon.client.AptTransaction)
@@ -176,6 +180,13 @@
             self.aptd._on_trans_error("some error", mock_trans)
             self.assertFalse(m.called)
 
+    def test_ignore_dpkg_errors(self):
+        mock_trans = Mock(aptdaemon.client.AptTransaction)
+        mock_trans.error_code = aptdaemon.enums.ERROR_PACKAGE_MANAGER_FAILED
+        with patch.object(self.aptd, "_call_apport_recoverable_error") as m:
+            self.aptd._on_trans_error("some error", mock_trans)
+            self.assertFalse(m.called)
+
 
 if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)