Skip to content

Commit

Permalink
Hide exceptions that occur during session.close() (certbot#4891)
Browse files Browse the repository at this point in the history
* Hide exceptions that occur during session.close()

This fixes certbot#4840. Exceptions that are raised out of __del__ methods are caught
and printed to stderr. By catching any exceptions that occur, we now prevent
this from happening.

Alternative solutions to this would have been either not calling
session.close() at all or adding a close() method to acme.client.ClientNetwork,
acme.client.Client, and certbot.client.Client and using certbot.client.Client
in a context manager to ensure close() is called. The former means that users
of the ACME library never properly close their connections until their program
exits and the latter adds a lot of complexity and nesting of client code for
little benefit.

* Only catch Exceptions
  • Loading branch information
bmw authored Jul 5, 2017
1 parent 97b22da commit 5318945
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
7 changes: 6 additions & 1 deletion acme/acme/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,12 @@ def __init__(self, key, alg=jose.RS256, verify_ssl=True,
self._default_timeout = timeout

def __del__(self):
self.session.close()
# Try to close the session, but don't show exceptions to the
# user if the call to close() fails. See #4840.
try:
self.session.close()
except Exception: # pylint: disable=broad-except
pass

def _wrap_in_jws(self, obj, nonce):
"""Wrap `JSONDeSerializable` object in JWS.
Expand Down
9 changes: 8 additions & 1 deletion acme/acme/client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -600,12 +600,19 @@ def test_send_request_timeout(self):
mock.ANY, mock.ANY, verify=mock.ANY, headers=mock.ANY,
timeout=45)

def test_del(self):
def test_del(self, close_exception=None):
sess = mock.MagicMock()

if close_exception is not None:
sess.close.side_effect = close_exception

self.net.session = sess
del self.net
sess.close.assert_called_once_with()

def test_del_error(self):
self.test_del(ReferenceError)

@mock.patch('acme.client.requests')
def test_requests_error_passthrough(self, mock_requests):
mock_requests.exceptions = requests.exceptions
Expand Down

0 comments on commit 5318945

Please sign in to comment.