From 5318945267ec3a8926210ec5fec2108bb267f269 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Wed, 5 Jul 2017 09:25:44 -0400
Subject: [PATCH] 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
---
 acme/acme/client.py      | 7 ++++++-
 acme/acme/client_test.py | 9 ++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/acme/acme/client.py b/acme/acme/client.py
index 9455159de..fa903f0e6 100644
--- a/acme/acme/client.py
+++ b/acme/acme/client.py
@@ -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.
diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py
index cd1a90645..54652b46c 100644
--- a/acme/acme/client_test.py
+++ b/acme/acme/client_test.py
@@ -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