From 1f39c28e489cca0397fc4c3675d13569318122ac Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 27 May 2019 16:28:34 +0300 Subject: [PATCH] bpo-37035: Don't log OSError (GH-13548) https://bugs.python.org/issue37035 --- Lib/asyncio/base_events.py | 7 ----- 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(+), 18 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 63b072b851..ce4f1904f9 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -59,13 +59,6 @@ _MIN_SCHEDULED_TIMER_HANDLES = 100 # 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 7dfe29579a..710f768718 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -96,7 +96,7 @@ class _ProactorBasePipeTransport(transports._FlowControlMixin, 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 f5f43a9bfe..44c380ae62 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -685,7 +685,7 @@ class _SelectorTransport(transports._FlowControlMixin, 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 8546985fe6..3eca6b4a39 100644 --- a/Lib/asyncio/sslproto.py +++ b/Lib/asyncio/sslproto.py @@ -707,7 +707,7 @@ class SSLProtocol(protocols.Protocol): 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 81d10b190c..28128d2977 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -724,7 +724,7 @@ class _UnixWritePipeTransport(transports._FlowControlMixin, 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 bf721b0005..2e52e9df5c 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -448,10 +448,23 @@ class SelectorTransportTests(test_utils.TestCase): 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 @@ class SelectorDatagramTransportTests(test_utils.TestCase): 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 31e710037f..ac84304ec9 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -977,11 +977,7 @@ class UnixWritePipeTransportTests(test_utils.TestCase): 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 0000000000..004ec2d137 --- /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. -- 2.40.0