From 18d3df78e8cc7ae498ba411fea155a227cb91642 Mon Sep 17 00:00:00 2001
From: Bob Strecansky <bob.strecansky@gmail.com>
Date: Wed, 26 Jul 2017 20:37:13 -0400
Subject: [PATCH] [#4535] - Unwrap max retries exceeded errors

---
 acme/acme/client.py      | 28 +++++++++++++++++++++++++++-
 acme/acme/client_test.py | 24 ++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/acme/acme/client.py b/acme/acme/client.py
index fa903f0e6..f781f011b 100644
--- a/acme/acme/client.py
+++ b/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,31 @@ 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',))"""
+
+            err_regex = r".*host='(\S*)'.*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}{3}".format(host, path, err_no, 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:
diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py
index 54652b46c..c800f476c 100644
--- a/acme/acme/client_test.py
+++ b/acme/acme/client_test.py
@@ -7,6 +7,7 @@ from six.moves import http_client  # pylint: disable=import-error
 
 import mock
 import requests
+import sys
 
 from acme import challenges
 from acme import errors
@@ -621,6 +622,29 @@ 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")
+
+        # Python 2
+        except ValueError as y:
+            if "linux" in sys.platform:
+                self.assertEqual("Requesting localhost/nonexistent: "
+                                 "[Errno 111] Connection refused", str(y))
+            else: #pragma: no cover
+                self.assertEqual("Requesting localhost/nonexistent: "
+                                 "[Errno 61] Connection refused", str(y))
+
+        # Python 3
+        except requests.exceptions.ConnectionError as z: #pragma: no cover
+            if "linux" in sys.platform:
+                self.assertEqual("('Connection aborted.', "
+                                 "error(111, 'Connection refused'))", str(z))
+            else: #pragma: no cover
+                self.assertEqual("('Connection aborted.', "
+                                 "error(61, 'Connection refused'))", str(z))
 
 class ClientNetworkWithMockedResponseTest(unittest.TestCase):
     """Tests for acme.client.ClientNetwork which mock out response."""