Skip to content

Commit

Permalink
[certbot#4535] - Unwrap 'max retries exceeded' errors (certbot#4733)
Browse files Browse the repository at this point in the history
Fixes certbot#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.
  • Loading branch information
jsha authored Oct 19, 2017
2 parents 5d2f6eb + a8e1df6 commit a6cecd7
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
29 changes: 28 additions & 1 deletion acme/acme/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from six.moves import http_client # pylint: disable=import-error

import OpenSSL
import re
import requests
import sys

Expand Down Expand Up @@ -599,6 +600,7 @@ def _check_response(cls, response, content_type=None):
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
Expand All @@ -624,7 +626,32 @@ def _send_request(self, method, url, *args, **kwargs):
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:
Expand Down
15 changes: 15 additions & 0 deletions acme/acme/client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,21 @@ def test_requests_error_passthrough(self, mock_requests):
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."""
Expand Down

0 comments on commit a6cecd7

Please sign in to comment.