Skip to content

Commit

Permalink
bpo-37035: Don't log OSError (GH-13548)
Browse files Browse the repository at this point in the history
  • Loading branch information
asvetlov authored and miss-islington committed May 27, 2019
1 parent ff6b2e6 commit 1f39c28
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 18 deletions.
7 changes: 0 additions & 7 deletions Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down
2 changes: 1 addition & 1 deletion Lib/asyncio/proactor_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion Lib/asyncio/selector_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion Lib/asyncio/sslproto.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion Lib/asyncio/unix_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
27 changes: 25 additions & 2 deletions Lib/test/test_asyncio/test_selector_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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__':
Expand Down
6 changes: 1 addition & 5 deletions Lib/test/test_asyncio/test_unix_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 1f39c28

Please sign in to comment.