Skip to content

bpo-37035: Don't log OSError #13548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -671,7 +671,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 @@ -699,7 +699,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 @@ -716,7 +716,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.