[#4535] - Unwrap 'max retries exceeded' errors (#4733)

Fixes #4535 

Extracts the relevant fields using a regex. We considered catching
specific exception types, and referencing their fields, but the types
raised by `requests` are not well documented and may not be long
term stable. If the regex fails to match, for instance due to a change
in the exception message, the new exception message will just be
passed through.

acme/acme/client.py

@@ -11,6 +11,7 @@ import six
 from six.moves import http_client  # pylint: disable=import-error
 
 import OpenSSL
+import re
 import requests
 import sys
 
@@ -599,6 +600,7 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         return response
 
     def _send_request(self, method, url, *args, **kwargs):
+        # pylint: disable=too-many-locals
         """Send HTTP request.
 
         Makes sure that `verify_ssl` is respected. Logs request and
@@ -624,7 +626,32 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         kwargs.setdefault('headers', {})
         kwargs['headers'].setdefault('User-Agent', self.user_agent)
         kwargs.setdefault('timeout', self._default_timeout)
-        response = self.session.request(method, url, *args, **kwargs)
+        try:
+            response = self.session.request(method, url, *args, **kwargs)
+        except requests.exceptions.RequestException as e:
+            # pylint: disable=pointless-string-statement
+            """Requests response parsing
+
+            The requests library emits exceptions with a lot of extra text.
+            We parse them with a regexp to raise a more readable exceptions.
+
+            Example:
+            HTTPSConnectionPool(host='acme-v01.api.letsencrypt.org',
+            port=443): Max retries exceeded with url: /directory
+            (Caused by NewConnectionError('
+            <requests.packages.urllib3.connection.VerifiedHTTPSConnection
+            object at 0x108356c50>: Failed to establish a new connection:
+            [Errno 65] No route to host',))"""
+
+            # pylint: disable=line-too-long
+            err_regex = r".*host='(\S*)'.*Max retries exceeded with url\: (\/\w*).*(\[Errno \d+\])([A-Za-z ]*)"
+            m = re.match(err_regex, str(e))
+            if m is None:
+                raise # pragma: no cover
+            else:
+                host, path, _err_no, err_msg = m.groups()
+                raise ValueError("Requesting {0}{1}:{2}".format(host, path, err_msg))
+
         # If content is DER, log the base64 of it instead of raw bytes, to keep
         # binary data out of the logs.
         if response.headers.get("Content-Type") == DER_CONTENT_TYPE:

acme/acme/client_test.py

@@ -621,6 +621,21 @@ class ClientNetworkTest(unittest.TestCase):
         self.assertRaises(requests.exceptions.RequestException,
                           self.net._send_request, 'GET', 'uri')
 
+    def test_urllib_error(self):
+        # Using a connection error to test a properly formatted error message
+        try:
+            # pylint: disable=protected-access
+            self.net._send_request('GET', "http://localhost:19123/nonexistent.txt")
+
+        # Value Error Generated Exceptions
+        except ValueError as y:
+            self.assertEqual("Requesting localhost/nonexistent: "
+                             "Connection refused", str(y))
+
+        # Requests Library Exceptions
+        except requests.exceptions.ConnectionError as z: #pragma: no cover
+            self.assertEqual("('Connection aborted.', "
+                             "error(111, 'Connection refused'))", str(z))
 
 class ClientNetworkWithMockedResponseTest(unittest.TestCase):
     """Tests for acme.client.ClientNetwork which mock out response."""