Skip to content

Commit

Permalink
[3.7] bpo-37035: Don't log OSError (pythonGH-13548)
Browse files Browse the repository at this point in the history
https://bugs.python.org/issue37035.
(cherry picked from commit 1f39c28)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
  • Loading branch information
asvetlov committed May 27, 2019
1 parent bfd0b77 commit ec53d1b
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 16 deletions.
5 changes: 0 additions & 5 deletions Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
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):

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 @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion Lib/asyncio/sslproto.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
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 @@ -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:
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 @@ -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)

Expand Down Expand Up @@ -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__':
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 @@ -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)
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 ec53d1b

Please sign in to comment.