From ec53d1bb4d571bca6fb03fc4b9ea0bd813e1642d Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 27 May 2019 16:28:34 +0300 Subject: [PATCH] [3.7] bpo-37035: Don't log OSError (GH-13548) https://bugs.python.org/issue37035. (cherry picked from commit 1f39c28e489cca0397fc4c3675d13569318122ac) Co-authored-by: Andrew Svetlov --- Lib/asyncio/base_events.py | 5 ---- Lib/asyncio/proactor_events.py | 2 +- Lib/asyncio/selector_events.py | 2 +- Lib/asyncio/sslproto.py | 2 +- Lib/asyncio/unix_events.py | 2 +- Lib/test/test_asyncio/test_selector_events.py | 27 +++++++++++++++++-- Lib/test/test_asyncio/test_unix_events.py | 6 +---- .../2019-05-24-18-16-07.bpo-37035.HFbJVT.rst | 5 ++++ 8 files changed, 35 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-05-24-18-16-07.bpo-37035.HFbJVT.rst diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 7bd20bbf006bb4..a736d01d6f3784 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -56,11 +56,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) - _HAS_IPv6 = hasattr(socket, 'AF_INET6') # Maximum timeout passed to select to avoid OS limitations diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index a638cceda5e4af..0296e0f7621dc0 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -96,7 +96,7 @@ def __del__(self): 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 fa27b3bc23af84..23bd8ad8492ff2 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -660,7 +660,7 @@ def __del__(self): 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 15eb9c78443af7..02d29738e46880 100644 --- a/Lib/asyncio/sslproto.py +++ b/Lib/asyncio/sslproto.py @@ -700,7 +700,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 a05ebfd51ba291..a0fc996d23a0de 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -717,7 +717,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 2205ee204249bb..a9fb61ae99de11 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -850,10 +850,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) @@ -1740,10 +1753,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 5d16b95456f0cb..ec171fa83da39f 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -961,11 +961,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 00000000000000..004ec2d1371401 --- /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.