mirror of
				https://github.com/python/cpython.git
				synced 2025-10-31 05:31:20 +00:00 
			
		
		
		
	asyncio, Tulip issue 139: Improve error messages on "fatal errors"
Mention if the error was caused by a read or a write, and be more specific on the object (ex: "pipe transport" instead of "transport").
This commit is contained in:
		
							parent
							
								
									c098241342
								
							
						
					
					
						commit
						065ca25aae
					
				
					 6 changed files with 64 additions and 38 deletions
				
			
		|  | @ -53,10 +53,10 @@ def close(self): | |||
|         if self._read_fut is not None: | ||||
|             self._read_fut.cancel() | ||||
| 
 | ||||
|     def _fatal_error(self, exc): | ||||
|     def _fatal_error(self, exc, message='Fatal error on pipe transport'): | ||||
|         if not isinstance(exc, (BrokenPipeError, ConnectionResetError)): | ||||
|             self._loop.call_exception_handler({ | ||||
|                 'message': 'Fatal transport error', | ||||
|                 'message': message, | ||||
|                 'exception': exc, | ||||
|                 'transport': self, | ||||
|                 'protocol': self._protocol, | ||||
|  | @ -151,11 +151,11 @@ def _loop_reading(self, fut=None): | |||
|             self._read_fut = self._loop._proactor.recv(self._sock, 4096) | ||||
|         except ConnectionAbortedError as exc: | ||||
|             if not self._closing: | ||||
|                 self._fatal_error(exc) | ||||
|                 self._fatal_error(exc, 'Fatal read error on pipe transport') | ||||
|         except ConnectionResetError as exc: | ||||
|             self._force_close(exc) | ||||
|         except OSError as exc: | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal read error on pipe transport') | ||||
|         except futures.CancelledError: | ||||
|             if not self._closing: | ||||
|                 raise | ||||
|  | @ -246,7 +246,7 @@ def _loop_writing(self, f=None, data=None): | |||
|         except ConnectionResetError as exc: | ||||
|             self._force_close(exc) | ||||
|         except OSError as exc: | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal write error on pipe transport') | ||||
| 
 | ||||
|     def can_write_eof(self): | ||||
|         return True | ||||
|  |  | |||
|  | @ -377,11 +377,11 @@ def close(self): | |||
|             self._conn_lost += 1 | ||||
|             self._loop.call_soon(self._call_connection_lost, None) | ||||
| 
 | ||||
|     def _fatal_error(self, exc): | ||||
|     def _fatal_error(self, exc, message='Fatal error on transport'): | ||||
|         # Should be called from exception handler only. | ||||
|         if not isinstance(exc, (BrokenPipeError, ConnectionResetError)): | ||||
|             self._loop.call_exception_handler({ | ||||
|                 'message': 'Fatal transport error', | ||||
|                 'message': message, | ||||
|                 'exception': exc, | ||||
|                 'transport': self, | ||||
|                 'protocol': self._protocol, | ||||
|  | @ -452,7 +452,7 @@ def _read_ready(self): | |||
|         except (BlockingIOError, InterruptedError): | ||||
|             pass | ||||
|         except Exception as exc: | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal read error on socket transport') | ||||
|         else: | ||||
|             if data: | ||||
|                 self._protocol.data_received(data) | ||||
|  | @ -488,7 +488,7 @@ def write(self, data): | |||
|             except (BlockingIOError, InterruptedError): | ||||
|                 pass | ||||
|             except Exception as exc: | ||||
|                 self._fatal_error(exc) | ||||
|                 self._fatal_error(exc, 'Fatal write error on socket transport') | ||||
|                 return | ||||
|             else: | ||||
|                 data = data[n:] | ||||
|  | @ -511,7 +511,7 @@ def _write_ready(self): | |||
|         except Exception as exc: | ||||
|             self._loop.remove_writer(self._sock_fd) | ||||
|             self._buffer.clear() | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal write error on socket transport') | ||||
|         else: | ||||
|             if n: | ||||
|                 del self._buffer[:n] | ||||
|  | @ -678,7 +678,7 @@ def _read_ready(self): | |||
|             self._loop.remove_reader(self._sock_fd) | ||||
|             self._loop.add_writer(self._sock_fd, self._write_ready) | ||||
|         except Exception as exc: | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal read error on SSL transport') | ||||
|         else: | ||||
|             if data: | ||||
|                 self._protocol.data_received(data) | ||||
|  | @ -712,7 +712,7 @@ def _write_ready(self): | |||
|             except Exception as exc: | ||||
|                 self._loop.remove_writer(self._sock_fd) | ||||
|                 self._buffer.clear() | ||||
|                 self._fatal_error(exc) | ||||
|                 self._fatal_error(exc, 'Fatal write error on SSL transport') | ||||
|                 return | ||||
| 
 | ||||
|             if n: | ||||
|  | @ -770,7 +770,7 @@ def _read_ready(self): | |||
|         except OSError as exc: | ||||
|             self._protocol.error_received(exc) | ||||
|         except Exception as exc: | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal read error on datagram transport') | ||||
|         else: | ||||
|             self._protocol.datagram_received(data, addr) | ||||
| 
 | ||||
|  | @ -805,7 +805,8 @@ def sendto(self, data, addr=None): | |||
|                 self._protocol.error_received(exc) | ||||
|                 return | ||||
|             except Exception as exc: | ||||
|                 self._fatal_error(exc) | ||||
|                 self._fatal_error(exc, | ||||
|                                   'Fatal write error on datagram transport') | ||||
|                 return | ||||
| 
 | ||||
|         # Ensure that what we buffer is immutable. | ||||
|  | @ -827,7 +828,8 @@ def _sendto_ready(self): | |||
|                 self._protocol.error_received(exc) | ||||
|                 return | ||||
|             except Exception as exc: | ||||
|                 self._fatal_error(exc) | ||||
|                 self._fatal_error(exc, | ||||
|                                   'Fatal write error on datagram transport') | ||||
|                 return | ||||
| 
 | ||||
|         self._maybe_resume_protocol()  # May append to buffer. | ||||
|  |  | |||
|  | @ -271,7 +271,7 @@ def _read_ready(self): | |||
|         except (BlockingIOError, InterruptedError): | ||||
|             pass | ||||
|         except OSError as exc: | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal read error on pipe transport') | ||||
|         else: | ||||
|             if data: | ||||
|                 self._protocol.data_received(data) | ||||
|  | @ -291,11 +291,11 @@ def close(self): | |||
|         if not self._closing: | ||||
|             self._close(None) | ||||
| 
 | ||||
|     def _fatal_error(self, exc): | ||||
|     def _fatal_error(self, exc, message='Fatal error on pipe transport'): | ||||
|         # should be called by exception handler only | ||||
|         if not (isinstance(exc, OSError) and exc.errno == errno.EIO): | ||||
|             self._loop.call_exception_handler({ | ||||
|                 'message': 'Fatal transport error', | ||||
|                 'message': message, | ||||
|                 'exception': exc, | ||||
|                 'transport': self, | ||||
|                 'protocol': self._protocol, | ||||
|  | @ -381,7 +381,7 @@ def write(self, data): | |||
|                 n = 0 | ||||
|             except Exception as exc: | ||||
|                 self._conn_lost += 1 | ||||
|                 self._fatal_error(exc) | ||||
|                 self._fatal_error(exc, 'Fatal write error on pipe transport') | ||||
|                 return | ||||
|             if n == len(data): | ||||
|                 return | ||||
|  | @ -406,7 +406,7 @@ def _write_ready(self): | |||
|             # Remove writer here, _fatal_error() doesn't it | ||||
|             # because _buffer is empty. | ||||
|             self._loop.remove_writer(self._fileno) | ||||
|             self._fatal_error(exc) | ||||
|             self._fatal_error(exc, 'Fatal write error on pipe transport') | ||||
|         else: | ||||
|             if n == len(data): | ||||
|                 self._loop.remove_writer(self._fileno) | ||||
|  | @ -443,11 +443,11 @@ def close(self): | |||
|     def abort(self): | ||||
|         self._close(None) | ||||
| 
 | ||||
|     def _fatal_error(self, exc): | ||||
|     def _fatal_error(self, exc, message='Fatal error on pipe transport'): | ||||
|         # should be called by exception handler only | ||||
|         if not isinstance(exc, (BrokenPipeError, ConnectionResetError)): | ||||
|             self._loop.call_exception_handler({ | ||||
|                 'message': 'Fatal transport error', | ||||
|                 'message': message, | ||||
|                 'exception': exc, | ||||
|                 'transport': self, | ||||
|                 'protocol': self._protocol, | ||||
|  |  | |||
|  | @ -69,7 +69,9 @@ def test_loop_reading_aborted(self): | |||
|         tr = _ProactorSocketTransport(self.loop, self.sock, self.protocol) | ||||
|         tr._fatal_error = unittest.mock.Mock() | ||||
|         tr._loop_reading() | ||||
|         tr._fatal_error.assert_called_with(err) | ||||
|         tr._fatal_error.assert_called_with( | ||||
|                             err, | ||||
|                             'Fatal read error on pipe transport') | ||||
| 
 | ||||
|     def test_loop_reading_aborted_closing(self): | ||||
|         self.loop._proactor.recv.side_effect = ConnectionAbortedError() | ||||
|  | @ -105,7 +107,9 @@ def test_loop_reading_exception(self): | |||
|         tr = _ProactorSocketTransport(self.loop, self.sock, self.protocol) | ||||
|         tr._fatal_error = unittest.mock.Mock() | ||||
|         tr._loop_reading() | ||||
|         tr._fatal_error.assert_called_with(err) | ||||
|         tr._fatal_error.assert_called_with( | ||||
|                             err, | ||||
|                             'Fatal read error on pipe transport') | ||||
| 
 | ||||
|     def test_write(self): | ||||
|         tr = _ProactorSocketTransport(self.loop, self.sock, self.protocol) | ||||
|  | @ -142,7 +146,9 @@ def test_loop_writing_err(self, m_log): | |||
|         tr._fatal_error = unittest.mock.Mock() | ||||
|         tr._buffer = [b'da', b'ta'] | ||||
|         tr._loop_writing() | ||||
|         tr._fatal_error.assert_called_with(err) | ||||
|         tr._fatal_error.assert_called_with( | ||||
|                             err, | ||||
|                             'Fatal write error on pipe transport') | ||||
|         tr._conn_lost = 1 | ||||
| 
 | ||||
|         tr.write(b'data') | ||||
|  |  | |||
|  | @ -655,7 +655,7 @@ def test_fatal_error(self, m_exc): | |||
| 
 | ||||
|         m_exc.assert_called_with( | ||||
|             test_utils.MockPattern( | ||||
|                 'Fatal transport error\nprotocol:.*\ntransport:.*'), | ||||
|                 'Fatal error on transport\nprotocol:.*\ntransport:.*'), | ||||
|             exc_info=(OSError, MOCK_ANY, MOCK_ANY)) | ||||
| 
 | ||||
|         tr._force_close.assert_called_with(exc) | ||||
|  | @ -785,7 +785,9 @@ def test_read_ready_err(self, m_exc): | |||
|         transport._fatal_error = unittest.mock.Mock() | ||||
|         transport._read_ready() | ||||
| 
 | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal read error on socket transport') | ||||
| 
 | ||||
|     def test_write(self): | ||||
|         data = b'data' | ||||
|  | @ -898,7 +900,9 @@ def test_write_exception(self, m_log): | |||
|             self.loop, self.sock, self.protocol) | ||||
|         transport._fatal_error = unittest.mock.Mock() | ||||
|         transport.write(data) | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal write error on socket transport') | ||||
|         transport._conn_lost = 1 | ||||
| 
 | ||||
|         self.sock.reset_mock() | ||||
|  | @ -1001,7 +1005,9 @@ def test_write_ready_exception(self): | |||
|         transport._fatal_error = unittest.mock.Mock() | ||||
|         transport._buffer.extend(b'data') | ||||
|         transport._write_ready() | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal write error on socket transport') | ||||
| 
 | ||||
|     @unittest.mock.patch('asyncio.base_events.logger') | ||||
|     def test_write_ready_exception_and_close(self, m_log): | ||||
|  | @ -1237,7 +1243,9 @@ def test_read_ready_recv_exc(self): | |||
|         transport = self._make_one() | ||||
|         transport._fatal_error = unittest.mock.Mock() | ||||
|         transport._read_ready() | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal read error on SSL transport') | ||||
| 
 | ||||
|     def test_write_ready_send(self): | ||||
|         self.sslsock.send.return_value = 4 | ||||
|  | @ -1319,7 +1327,9 @@ def test_write_ready_send_exc(self): | |||
|         transport._buffer = list_to_buffer([b'data']) | ||||
|         transport._fatal_error = unittest.mock.Mock() | ||||
|         transport._write_ready() | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal write error on SSL transport') | ||||
|         self.assertEqual(list_to_buffer(), transport._buffer) | ||||
| 
 | ||||
|     def test_write_ready_read_wants_write(self): | ||||
|  | @ -1407,7 +1417,9 @@ def test_read_ready_err(self): | |||
|         transport._fatal_error = unittest.mock.Mock() | ||||
|         transport._read_ready() | ||||
| 
 | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal read error on datagram transport') | ||||
| 
 | ||||
|     def test_read_ready_oserr(self): | ||||
|         transport = _SelectorDatagramTransport( | ||||
|  | @ -1517,7 +1529,9 @@ def test_sendto_exception(self, m_log): | |||
|         transport.sendto(data, ()) | ||||
| 
 | ||||
|         self.assertTrue(transport._fatal_error.called) | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal write error on datagram transport') | ||||
|         transport._conn_lost = 1 | ||||
| 
 | ||||
|         transport._address = ('123',) | ||||
|  | @ -1633,7 +1647,9 @@ def test_sendto_ready_exception(self): | |||
|         transport._buffer.append((b'data', ())) | ||||
|         transport._sendto_ready() | ||||
| 
 | ||||
|         transport._fatal_error.assert_called_with(err) | ||||
|         transport._fatal_error.assert_called_with( | ||||
|                                    err, | ||||
|                                    'Fatal write error on datagram transport') | ||||
| 
 | ||||
|     def test_sendto_ready_error_received(self): | ||||
|         self.sock.sendto.side_effect = ConnectionRefusedError | ||||
|  | @ -1667,7 +1683,7 @@ def test_fatal_error_connected(self, m_exc): | |||
|         self.assertFalse(self.protocol.error_received.called) | ||||
|         m_exc.assert_called_with( | ||||
|             test_utils.MockPattern( | ||||
|                 'Fatal transport error\nprotocol:.*\ntransport:.*'), | ||||
|                 'Fatal error on transport\nprotocol:.*\ntransport:.*'), | ||||
|             exc_info=(ConnectionRefusedError, MOCK_ANY, MOCK_ANY)) | ||||
| 
 | ||||
| 
 | ||||
|  |  | |||
|  | @ -365,7 +365,7 @@ def test__read_ready_error(self, m_read, m_logexc): | |||
|         tr._close.assert_called_with(err) | ||||
|         m_logexc.assert_called_with( | ||||
|             test_utils.MockPattern( | ||||
|                 'Fatal transport error\nprotocol:.*\ntransport:.*'), | ||||
|                 'Fatal read error on pipe transport\nprotocol:.*\ntransport:.*'), | ||||
|             exc_info=(OSError, MOCK_ANY, MOCK_ANY)) | ||||
| 
 | ||||
|     @unittest.mock.patch('os.read') | ||||
|  | @ -558,7 +558,9 @@ def test_write_err(self, m_write, m_log): | |||
|         m_write.assert_called_with(5, b'data') | ||||
|         self.assertFalse(self.loop.writers) | ||||
|         self.assertEqual([], tr._buffer) | ||||
|         tr._fatal_error.assert_called_with(err) | ||||
|         tr._fatal_error.assert_called_with( | ||||
|                             err, | ||||
|                             'Fatal write error on pipe transport') | ||||
|         self.assertEqual(1, tr._conn_lost) | ||||
| 
 | ||||
|         tr.write(b'data') | ||||
|  | @ -660,7 +662,7 @@ def test__write_ready_err(self, m_write, m_logexc): | |||
|         self.assertTrue(tr._closing) | ||||
|         m_logexc.assert_called_with( | ||||
|             test_utils.MockPattern( | ||||
|                 'Fatal transport error\nprotocol:.*\ntransport:.*'), | ||||
|                 'Fatal write error on pipe transport\nprotocol:.*\ntransport:.*'), | ||||
|             exc_info=(OSError, MOCK_ANY, MOCK_ANY)) | ||||
|         self.assertEqual(1, tr._conn_lost) | ||||
|         test_utils.run_briefly(self.loop) | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Victor Stinner
						Victor Stinner