[Python-checkins] bpo-37035: Don't log OSError (GH-13548)

Miss Islington (bot) webhook-mailer at python.org
Mon May 27 09:28:38 EDT 2019


https://github.com/python/cpython/commit/1f39c28e489cca0397fc4c3675d13569318122ac
commit: 1f39c28e489cca0397fc4c3675d13569318122ac
branch: master
author: Andrew Svetlov <andrew.svetlov at gmail.com>
committer: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
date: 2019-05-27T06:28:34-07:00
summary:

bpo-37035: Don't log OSError (GH-13548)



https://bugs.python.org/issue37035

files:
A Misc/NEWS.d/next/Library/2019-05-24-18-16-07.bpo-37035.HFbJVT.rst
M Lib/asyncio/base_events.py
M Lib/asyncio/proactor_events.py
M Lib/asyncio/selector_events.py
M Lib/asyncio/sslproto.py
M Lib/asyncio/unix_events.py
M Lib/test/test_asyncio/test_selector_events.py
M Lib/test/test_asyncio/test_unix_events.py

diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py
index 63b072b851e3..ce4f1904f950 100644
--- a/Lib/asyncio/base_events.py
+++ b/Lib/asyncio/base_events.py
@@ -59,13 +59,6 @@
 # before cleanup of cancelled handles is performed.
 _MIN_CANCELLED_TIMER_HANDLES_FRACTION = 0.5
 
-# Exceptions which must not call the exception handler in fatal error
-# methods (_fatal_error())
-_FATAL_ERROR_IGNORE = (BrokenPipeError,
-                       ConnectionResetError, ConnectionAbortedError)
-
-if ssl is not None:
-    _FATAL_ERROR_IGNORE = _FATAL_ERROR_IGNORE + (ssl.SSLCertVerificationError,)
 
 _HAS_IPv6 = hasattr(socket, 'AF_INET6')
 
diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py
index 7dfe29579a0d..710f768718b4 100644
--- a/Lib/asyncio/proactor_events.py
+++ b/Lib/asyncio/proactor_events.py
@@ -96,7 +96,7 @@ def __del__(self, _warn=warnings.warn):
 
     def _fatal_error(self, exc, message='Fatal error on pipe transport'):
         try:
-            if isinstance(exc, base_events._FATAL_ERROR_IGNORE):
+            if isinstance(exc, OSError):
                 if self._loop.get_debug():
                     logger.debug("%r: %s", self, message, exc_info=True)
             else:
diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index f5f43a9bfef3..44c380ae62db 100644
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -685,7 +685,7 @@ def __del__(self, _warn=warnings.warn):
 
     def _fatal_error(self, exc, message='Fatal error on transport'):
         # Should be called from exception handler only.
-        if isinstance(exc, base_events._FATAL_ERROR_IGNORE):
+        if isinstance(exc, OSError):
             if self._loop.get_debug():
                 logger.debug("%r: %s", self, message, exc_info=True)
         else:
diff --git a/Lib/asyncio/sslproto.py b/Lib/asyncio/sslproto.py
index 8546985fe63f..3eca6b4a3912 100644
--- a/Lib/asyncio/sslproto.py
+++ b/Lib/asyncio/sslproto.py
@@ -707,7 +707,7 @@ def _process_write_backlog(self):
                 self._fatal_error(exc, 'Fatal error on SSL transport')
 
     def _fatal_error(self, exc, message='Fatal error on transport'):
-        if isinstance(exc, base_events._FATAL_ERROR_IGNORE):
+        if isinstance(exc, OSError):
             if self._loop.get_debug():
                 logger.debug("%r: %s", self, message, exc_info=True)
         else:
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
index 81d10b190cc6..28128d2977df 100644
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -724,7 +724,7 @@ def abort(self):
 
     def _fatal_error(self, exc, message='Fatal error on pipe transport'):
         # should be called by exception handler only
-        if isinstance(exc, base_events._FATAL_ERROR_IGNORE):
+        if isinstance(exc, OSError):
             if self._loop.get_debug():
                 logger.debug("%r: %s", self, message, exc_info=True)
         else:
diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py
index bf721b0005b0..2e52e9df5c3b 100644
--- a/Lib/test/test_asyncio/test_selector_events.py
+++ b/Lib/test/test_asyncio/test_selector_events.py
@@ -448,10 +448,23 @@ def test_fatal_error(self, m_exc):
         tr._force_close = mock.Mock()
         tr._fatal_error(exc)
 
+        m_exc.assert_not_called()
+
+        tr._force_close.assert_called_with(exc)
+
+    @mock.patch('asyncio.log.logger.error')
+    def test_fatal_error_custom_exception(self, m_exc):
+        class MyError(Exception):
+            pass
+        exc = MyError()
+        tr = self.create_transport()
+        tr._force_close = mock.Mock()
+        tr._fatal_error(exc)
+
         m_exc.assert_called_with(
             test_utils.MockPattern(
                 'Fatal error on transport\nprotocol:.*\ntransport:.*'),
-            exc_info=(OSError, MOCK_ANY, MOCK_ANY))
+            exc_info=(MyError, MOCK_ANY, MOCK_ANY))
 
         tr._force_close.assert_called_with(exc)
 
@@ -1338,10 +1351,20 @@ def test_fatal_error_connected(self, m_exc):
         err = ConnectionRefusedError()
         transport._fatal_error(err)
         self.assertFalse(self.protocol.error_received.called)
+        m_exc.assert_not_called()
+
+    @mock.patch('asyncio.base_events.logger.error')
+    def test_fatal_error_connected_custom_error(self, m_exc):
+        class MyException(Exception):
+            pass
+        transport = self.datagram_transport(address=('0.0.0.0', 1))
+        err = MyException()
+        transport._fatal_error(err)
+        self.assertFalse(self.protocol.error_received.called)
         m_exc.assert_called_with(
             test_utils.MockPattern(
                 'Fatal error on transport\nprotocol:.*\ntransport:.*'),
-            exc_info=(ConnectionRefusedError, MOCK_ANY, MOCK_ANY))
+            exc_info=(MyException, MOCK_ANY, MOCK_ANY))
 
 
 if __name__ == '__main__':
diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py
index 31e710037f76..ac84304ec99d 100644
--- a/Lib/test/test_asyncio/test_unix_events.py
+++ b/Lib/test/test_asyncio/test_unix_events.py
@@ -977,11 +977,7 @@ def test__write_ready_err(self, m_write, m_logexc):
         self.assertFalse(self.loop.readers)
         self.assertEqual(bytearray(), tr._buffer)
         self.assertTrue(tr.is_closing())
-        m_logexc.assert_called_with(
-            test_utils.MockPattern(
-                'Fatal write error on pipe transport'
-                '\nprotocol:.*\ntransport:.*'),
-            exc_info=(OSError, MOCK_ANY, MOCK_ANY))
+        m_logexc.assert_not_called()
         self.assertEqual(1, tr._conn_lost)
         test_utils.run_briefly(self.loop)
         self.protocol.connection_lost.assert_called_with(err)
diff --git a/Misc/NEWS.d/next/Library/2019-05-24-18-16-07.bpo-37035.HFbJVT.rst b/Misc/NEWS.d/next/Library/2019-05-24-18-16-07.bpo-37035.HFbJVT.rst
new file mode 100644
index 000000000000..004ec2d13714
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-05-24-18-16-07.bpo-37035.HFbJVT.rst
@@ -0,0 +1,5 @@
+Don't log OSError based exceptions if a fatal error has occurred in asyncio
+transport. Peer can generate almost any OSError, user cannot avoid these exceptions
+by fixing own code. 
+Errors are still propagated to user code, it's just logging them
+is pointless and pollute asyncio logs.



More information about the Python-checkins mailing list