Raise exception when modbus unit unexpectedly closes connection

Raise an error when the modbus unit unexpectedly closes the connection
during a receive data operation without returning any data, and log a
warning if it does return some but not all data before closing the
connection. Detecting and handling this here ensures that
ModbusTcpClient._recv doesn't needlessly loop until it times out after
the stream is closed by the peer, and also makes it certain that a
response of b'' from ModbusTcpClient._recv means there was a timeout
and cannot mean that the peer closed the stream, as it could mean
before.

The previous behavior was to not identify the remote end closing the
socket, triggering a timeout, in turn causing the transaction manager
to treat it as the modbus unit returning a response with errors in
it (raising InvalidMessageReceivedException). This will now raise a
ConnectionException, which falls through to the client, bypassing the
retry mechanisms.

Note that https://docs.python.org/3/library/socket.html does not
contain a full description on the socket.recv method; see also pydoc
for socket.socket.recv.

Author: thormick

pymodbus/client/sync.py

@@ -239,7 +239,12 @@ def _recv(self, size):
         """ Reads data from the underlying descriptor
 
         :param size: The number of bytes to read
-        :return: The bytes read
+        :return: The bytes read if the peer sent a response, or a zero-length
+                 response if no data packets were received from the client at
+                 all.
+        :raises: ConnectionException if the socket is not initialized, or the
+                 peer either has closed the connection before this method is
+                 invoked or closes it before sending any data before timeout.
         """
         if not self.socket:
             raise ConnectionException(self.__str__())
@@ -270,6 +275,9 @@ def _recv(self, size):
             ready = select.select([self.socket], [], [], end - time_)
             if ready[0]:
                 recv_data = self.socket.recv(recv_size)
+                if recv_data == b'':
+                    return self._handle_abrupt_socket_close(
+                        size, data, time.time() - time_)
                 data.append(recv_data)
                 data_length += len(recv_data)
             time_ = time.time()
@@ -286,6 +294,35 @@ def _recv(self, size):
 
         return b"".join(data)
 
+    def _handle_abrupt_socket_close(self, size, data, duration):
+        """ Handle unexpected socket close by remote end
+
+        Intended to be invoked after determining that the remote end
+        has unexpectedly closed the connection, to clean up and handle
+        the situation appropriately.
+
+        :param size: The number of bytes that was attempted to read
+        :param data: The actual data returned
+        :param duration: Duration from the read was first attempted
+               until it was determined that the remote closed the
+               socket
+        :return: The more than zero bytes read from the remote end
+        :raises: ConnectionException If the remote end didn't send any
+                 data at all before closing the connection.
+        """
+        self.close()
+        readsize = (f"read of {size} bytes" if size
+                    else "unbounded read")
+        msg = (f'{self.__str__()}: Connection unexpectedly closed '
+               f'{duration:.6f} seconds into {readsize}')
+        if data:
+            result = b"".join(data)
+            msg += f" after returning {len(result)} bytes"
+            _logger.warning(msg)
+            return result
+        msg += " without response from unit before it closed connection"
+        raise ConnectionException(msg)
+
     def is_socket_open(self):
         return True if self.socket is not None else False