Hide exceptions that occur during session.close() (#4891)

* Hide exceptions that occur during session.close()

This fixes #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
This commit is contained in:
Brad Warren
2017-07-05 09:25:44 -04:00
committed by GitHub
parent 97b22da1b6
commit 5318945267
2 changed files with 14 additions and 2 deletions
+6 -1
View File
@@ -519,7 +519,12 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes
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.
+8 -1
View File
@@ -600,12 +600,19 @@ class ClientNetworkTest(unittest.TestCase):
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