Retry fetch chain errors (#4196) (#4383)

* Retry fetch chain errors (#4196)

* Trying to avoid confusing pylint

* Pylint disable

* Typo certz->certr

* Move pylint disable, log when fetch chain fails

certbot/client.py

@@ -8,6 +8,7 @@ import OpenSSL
 import zope.component
 
 from acme import client as acme_client
+from acme import errors as acme_errors
 from acme import jose
 from acme import messages
 
@@ -242,7 +243,28 @@ class Client(object):
             jose.ComparableX509(
                 OpenSSL.crypto.load_certificate_request(typ, csr.data)),
                 authzr)
-        return certr, self.acme.fetch_chain(certr)
+
+        notify = zope.component.getUtility(interfaces.IDisplay).notification
+        retries = 0
+        chain = None
+
+        while retries <= 1:
+            if retries:
+                notify('Failed to fetch chain, please check your network '
+                       'and continue', pause=True)
+            try:
+                chain = self.acme.fetch_chain(certr)
+                break
+            except acme_errors.Error:
+                logger.debug('Failed to fetch chain', exc_info=True)
+                retries += 1
+
+        if chain is None:
+            raise acme_errors.Error(
+                'Failed to fetch chain. You should not deploy the generated '
+                'certificate, please rerun the command for a new one.')
+
+        return certr, chain
 
     def obtain_certificate(self, domains):
         """Obtains a certificate from the ACME server.
@@ -269,10 +291,12 @@ class Client(object):
         key = crypto_util.init_save_key(
             self.config.rsa_key_size, self.config.key_dir)
         csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir)
+        certr, chain = self.obtain_certificate_from_csr(
+            domains, csr, authzr=authzr)
 
-        return (self.obtain_certificate_from_csr(domains, csr, authzr=authzr)
-                                                                + (key, csr))
+        return certr, chain, key, csr
 
+    # pylint: disable=no-member
     def obtain_and_enroll_certificate(self, domains, certname):
         """Obtain and enroll certificate.
 

certbot/main.py

@@ -487,7 +487,7 @@ def install(config, plugins):
     try:
         installer, _ = plug_sel.choose_configurator_plugins(config, plugins, "install")
     except errors.PluginSelectionError as e:
-        return e.message
+        return str(e)
 
     domains, _ = _find_domains_or_certname(config, installer)
     le_client = _init_le_client(config, authenticator=None, installer=installer)

certbot/tests/client_test.py

@@ -7,6 +7,7 @@ import unittest
 import OpenSSL
 import mock
 
+from acme import errors as acme_errors
 from acme import jose
 
 from certbot import account
@@ -170,7 +171,9 @@ class ClientTest(ClientTestCommon):
         self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr)
 
     @mock.patch("certbot.client.logger")
-    def test_obtain_certificate_from_csr(self, mock_logger):
+    @test_util.patch_get_utility()
+    def test_obtain_certificate_from_csr(self, unused_mock_get_utility,
+                                         mock_logger):
         self._mock_obtain_certificate()
         test_csr = util.CSR(form="der", file=None, data=CSR_SAN)
         auth_handler = self.client.auth_handler
@@ -203,8 +206,44 @@ class ClientTest(ClientTestCommon):
             test_csr)
         mock_logger.warning.assert_called_once_with(mock.ANY)
 
+    @test_util.patch_get_utility()
+    def test_obtain_certificate_from_csr_retry_succeeded(
+            self, mock_get_utility):
+        self._mock_obtain_certificate()
+        self.acme.fetch_chain.side_effect = [acme_errors.Error,
+                                             mock.sentinel.chain]
+        test_csr = util.CSR(form="der", file=None, data=CSR_SAN)
+        auth_handler = self.client.auth_handler
+
+        authzr = auth_handler.get_authorizations(self.eg_domains, False)
+        self.assertEqual(
+            (mock.sentinel.certr, mock.sentinel.chain),
+            self.client.obtain_certificate_from_csr(
+                self.eg_domains,
+                test_csr,
+                authzr=authzr))
+        self.assertEqual(1, mock_get_utility().notification.call_count)
+
+    @test_util.patch_get_utility()
+    def test_obtain_certificate_from_csr_retry_failed(self, mock_get_utility):
+        self._mock_obtain_certificate()
+        self.acme.fetch_chain.side_effect = acme_errors.Error
+        test_csr = util.CSR(form="der", file=None, data=CSR_SAN)
+        auth_handler = self.client.auth_handler
+
+        authzr = auth_handler.get_authorizations(self.eg_domains, False)
+        self.assertRaises(
+            acme_errors.Error,
+            self.client.obtain_certificate_from_csr,
+            self.eg_domains,
+            test_csr,
+            authzr=authzr)
+        self.assertEqual(1, mock_get_utility().notification.call_count)
+
     @mock.patch("certbot.client.crypto_util")
-    def test_obtain_certificate(self, mock_crypto_util):
+    @test_util.patch_get_utility()
+    def test_obtain_certificate(self, unused_mock_get_utility,
+                                mock_crypto_util):
         self._mock_obtain_certificate()
 
         csr = util.CSR(form="der", file=None, data=CSR_SAN)