Propagate known request identity on ModbusIOException

pymodbus/framer/base.py

@@ -88,10 +88,6 @@ def handleFrame(
                 )
                 continue
             if (pdu := self.decoder.decode(frame_data)) is None:
-                # Preserve framing identity so the server can echo transaction/dev
-                # ids on the undecodable-function exception path (see #2990).
-                # Do not recover a function code from garbage payloads — noise on
-                # serial lines is the common cause of this path.
                 raise ModbusIOException(
                     "Unable to decode request",
                     transaction_id=tid,

pymodbus/pdu/pdu.py

@@ -29,7 +29,11 @@ def __init__(
         """Initialize the base data for a modbus request."""
         self.dev_id: int = dev_id
         if dev_id > 255:
-            raise ModbusIOException(f"Invalid ID {dev_id}")
+            raise ModbusIOException(
+                f"Invalid ID {dev_id}",
+                transaction_id=transaction_id,
+                dev_id=dev_id,
+            )
         self.transaction_id: int = transaction_id
         self.address: int = address
         self.bits: list[bool] = bits or []

pymodbus/pdu/register_message.py

@@ -76,7 +76,8 @@ def decode(self, data: bytes) -> None:
         self.registers = []
         if (data_len := int(data[0])) >= len(data):
             raise ModbusIOException(
-                f"byte_count {data_len} > length of packet {len(data)}"
+                f"byte_count {data_len} > length of packet {len(data)}",
+                function_code=self.function_code,
             )
         for i in range(1, data_len, 2):
             self.registers.append(struct.unpack(">H", data[i : i + 2])[0])

pymodbus/server/requesthandler.py

@@ -63,17 +63,14 @@ def callback_data(self, data: bytes, addr: tuple | None = None) -> int:
         try:
             used_len = super().callback_data(data, addr)
         except ModbusIOException as exc:
-            # Undecodable function codes (and frame garbage) land here. last_pdu
-            # is cleared before framing runs, so identity comes from the framer
-            # exception attributes when available. Use function code 0x00 rather
-            # than a hardcoded unrelated value (was 40 / 0x28) — see #2990.
+            function_code = 0x00 if exc.fcode is None else exc.fcode
             response = ExceptionResponse(
-                0x00,
+                function_code,
                 exception_code=ExcCodes.ILLEGAL_FUNCTION,
                 device_id=exc.dev_id,
                 transaction=exc.transaction_id,
             )
-            self.server_send(response, 0)
+            self.server_send(response, addr)
             return len(data)
         if self.last_pdu:
             self.loop.call_soon(self.handle_later)

pymodbus/transaction/transaction.py

@@ -120,6 +120,18 @@ def sync_get_response(self, dev_id, tid) -> ModbusPDU:
             if monotonic() >= deadline:
                 raise asyncio.exceptions.TimeoutError()
 
+    @staticmethod
+    def _io_exception_from_request(
+        message: str, request: ModbusPDU
+    ) -> ModbusIOException:
+        """Build ModbusIOException from an outstanding request."""
+        return ModbusIOException(
+            message,
+            function_code=request.function_code,
+            transaction_id=request.transaction_id,
+            dev_id=request.dev_id,
+        )
+
     def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> ModbusPDU:
         """Execute requests asynchronously.
 
@@ -143,26 +155,29 @@ def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbus
                     )
                     self.count_until_disconnect = self.max_until_disconnect
                     if response.dev_id != request.dev_id:
-                        raise ModbusIOException(
-                            f"ERROR: request uses device id={request.dev_id} but received {response.dev_id}."
+                        raise self._io_exception_from_request(
+                            f"ERROR: request uses device id={request.dev_id} but received {response.dev_id}.",
+                            request,
                         )
                     if response.transaction_id != request.transaction_id:
-                        raise ModbusIOException(
-                            f"ERROR: request uses transaction id={request.transaction_id} but received {response.transaction_id}."
+                        raise self._io_exception_from_request(
+                            f"ERROR: request uses transaction id={request.transaction_id} but received {response.transaction_id}.",
+                            request,
                         )
                     response.retries = count_retries
                     return response
                 except asyncio.exceptions.TimeoutError:
                     count_retries += 1
             if self.count_until_disconnect < 0:
                 self.connection_lost(asyncio.TimeoutError("Server not responding"))
-                raise ModbusIOException(
-                    "ERROR: No response received of the last requests (default: retries+3), CLOSING CONNECTION."
+                raise self._io_exception_from_request(
+                    "ERROR: No response received of the last requests (default: retries+3), CLOSING CONNECTION.",
+                    request,
                 )
             self.count_until_disconnect -= 1
             txt = f"No response received after {self.retries} retries, continue with next request"
             Log.error(txt)
-            raise ModbusIOException(txt)
+            raise self._io_exception_from_request(txt, request)
 
     async def execute(
         self, no_response_expected: bool, request: ModbusPDU
@@ -193,33 +208,37 @@ async def execute(
                     )
                     self.count_until_disconnect = self.max_until_disconnect
                     if request.dev_id and response.dev_id != request.dev_id:
-                        raise ModbusIOException(
-                            f"ERROR: request uses device id={request.dev_id} but received {response.dev_id}."
+                        raise self._io_exception_from_request(
+                            f"ERROR: request uses device id={request.dev_id} but received {response.dev_id}.",
+                            request,
                         )
                     if (
                         response.transaction_id
                         and response.transaction_id != request.transaction_id
                     ):
-                        raise ModbusIOException(
-                            f"ERROR: request uses transaction id={request.transaction_id} but received {response.transaction_id}."
+                        raise self._io_exception_from_request(
+                            f"ERROR: request uses transaction id={request.transaction_id} but received {response.transaction_id}.",
+                            request,
                         )
                     response.retries = count_retries
                     return response
                 except asyncio.exceptions.TimeoutError:
                     count_retries += 1
                 except asyncio.exceptions.CancelledError as exc:
-                    raise ModbusIOException(
-                        "Request cancelled outside library."
+                    raise self._io_exception_from_request(
+                        "Request cancelled outside library.",
+                        request,
                     ) from exc
             if self.count_until_disconnect < 0:
                 self.connection_lost(asyncio.TimeoutError("Server not responding"))
-                raise ModbusIOException(
-                    "ERROR: No response received of the last requests (default: retries+3), CLOSING CONNECTION."
+                raise self._io_exception_from_request(
+                    "ERROR: No response received of the last requests (default: retries+3), CLOSING CONNECTION.",
+                    request,
                 )
             self.count_until_disconnect -= 1
             txt = f"No response received after {self.retries} retries, continue with next request"
             Log.error(txt)
-            raise ModbusIOException(txt)
+            raise self._io_exception_from_request(txt, request)
 
     def pdu_send(self, pdu: ModbusPDU, addr: tuple | None = None) -> None:
         """Build byte stream and send."""

test/client/test_client_faulty_response.py

@@ -34,8 +34,10 @@ def test_1917_frame(self):
     def test_faulty_frame1(self, framer):
         """Test ok frame."""
         faulty_frame = b"\x00\x04\x00\x00\x00\x05\x00\x03\x0a\x00\x04"
-        with pytest.raises(ModbusIOException):
+        with pytest.raises(ModbusIOException) as exc_info:
             framer.handleFrame(faulty_frame, 0, 0)
+        assert exc_info.value.transaction_id == 4
+        assert exc_info.value.dev_id == 0
         used_len, pdu = framer.handleFrame(self.good_frame, 0, 0)
         assert pdu
         assert used_len == len(self.good_frame)

test/framer/test_extras.py

@@ -84,8 +84,11 @@ def test_tcp_framer_transaction_wrong_tid(self):
     def test_tcp_framer_transaction_wrong_fc(self):
         """Test a half completed tcp frame transaction."""
         msg = b"\x00\x01\x00\x00\x00\x06\xff\x70\x01\x02\x00\x08"
-        with pytest.raises(ModbusIOException):
+        with pytest.raises(ModbusIOException) as exc_info:
             self._tcp.handleFrame(msg, 0, 0)
+        assert exc_info.value.transaction_id == 1
+        assert exc_info.value.dev_id == 0xFF
+        assert exc_info.value.fcode is None
 
     def test_tls_incoming_packet(self):
         """Framer tls incoming packet."""

test/pdu/test_pdu.py

@@ -37,8 +37,10 @@ async def test_get_pdu_size(self):
 
     async def test_pdu_id(self):
         """Test set illegal pdu id."""
-        with pytest.raises(ModbusIOException):
-            ModbusPDU(256)
+        with pytest.raises(ModbusIOException) as exc_info:
+            ModbusPDU(256, transaction_id=0x42)
+        assert exc_info.value.dev_id == 256
+        assert exc_info.value.transaction_id == 0x42
 
     async def test_is_error(self):
         """Test is_error."""

test/pdu/test_register_read_messages.py

@@ -80,8 +80,9 @@ def test_register_read_response_decode(self):
     def test_register_read_response_decode_error(self):
         """Test register read response."""
         reg = ReadHoldingRegistersResponse(count=5)
-        with pytest.raises(ModbusIOException):
+        with pytest.raises(ModbusIOException) as exc_info:
             reg.decode(b"\x14\x00\x03\x00\x11")
+        assert exc_info.value.fcode == reg.function_code
 
     async def test_register_read_requests_count_errors(self, mock_server_context):
         """This tests that the register request messages.

test/server/test_requesthandler.py

@@ -55,6 +55,7 @@ async def test_rh_callback_data(self, requesthandler):
 
     async def test_rh_callback_data_undecodable_echoes_identity(self, requesthandler):
         """Undecodable FC exception must echo framing tid/dev_id (#2990)."""
+        peer = ("192.0.2.1", 5020)
         with mock.patch(
             "pymodbus.transaction.TransactionManager.callback_data"
         ) as cb_data:
@@ -65,16 +66,16 @@ async def test_rh_callback_data_undecodable_echoes_identity(self, requesthandler
             )
             cb_data.side_effect = exc
             data = b"\x00\x0a\x00\x00\x00\x06\x07\x0a\x00\x00\x00\x01"
-            assert len(data) == requesthandler.callback_data(data, None)
+            assert len(data) == requesthandler.callback_data(data, peer)
 
         requesthandler.pdu_send.assert_called_once()
         response = requesthandler.pdu_send.call_args.args[0]
         assert isinstance(response, ExceptionResponse)
         assert response.transaction_id == 0x000A
         assert response.dev_id == 7
-        # 0x00 | 0x80 — not the previous hardcoded 0x28 | 0x80
         assert response.function_code == 0x80
-        assert response.exception_code == 0x01  # ILLEGAL_FUNCTION
+        assert response.exception_code == 0x01
+        assert requesthandler.pdu_send.call_args.kwargs.get("addr") == peer
 
     async def test_rh_callback_data_undecodable_without_framing_attrs(
         self, requesthandler

test/transaction/test_transaction.py

@@ -223,14 +223,19 @@ async def test_transaction_execute(self, use_clc, scenario):
         elif scenario == 3:  # wait receive,timeout, no_responses
             transact.comm_params.timeout_connect = 0.1
             transact.connection_lost = mock.Mock()  # type: ignore[method-assign]
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 await transact.execute(False, request)
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
+            assert exc_info.value.transaction_id == request.transaction_id
         elif scenario == 4:  # wait receive,timeout, disconnect
             transact.comm_params.timeout_connect = 0.1
             transact.count_until_disconnect = -1
             transact.connection_lost = mock.Mock()  # type: ignore[method-assign]
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 await transact.execute(False, request)
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.transaction_id == request.transaction_id
         elif scenario == 5:  # wait receive,timeout, no_responses pass
             transact.comm_params.timeout_connect = 0.1
             transact.connection_lost = mock.Mock()  # type: ignore[method-assign]
@@ -242,8 +247,10 @@ async def test_transaction_execute(self, use_clc, scenario):
             await asyncio.sleep(0.1)
             resp.cancel()
             await asyncio.sleep(0.1)
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 await resp
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
         elif scenario == 7:  # response
             transact.comm_params.timeout_connect = 0.2
             resp = asyncio.create_task(transact.execute(False, request))
@@ -259,8 +266,11 @@ async def test_transaction_execute(self, use_clc, scenario):
             new_resp.dev_id = 17
             transact.response_future.set_result(new_resp)
             await asyncio.sleep(0.1)
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 resp.result()
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
+            assert exc_info.value.transaction_id == request.transaction_id
         else:  # if scenario == 9: # response wrong tid
             transact.comm_params.timeout_connect = 0.2
             resp = asyncio.create_task(transact.execute(False, request))
@@ -269,8 +279,11 @@ async def test_transaction_execute(self, use_clc, scenario):
             new_resp.transaction_id = 17
             transact.response_future.set_result(new_resp)
             await asyncio.sleep(0.1)
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 resp.result()
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
+            assert exc_info.value.transaction_id == request.transaction_id
 
     async def test_transaction_receiver(self, use_clc):
         """Test tracers in disconnect."""
@@ -449,13 +462,18 @@ async def test_sync_transaction_execute(self, use_clc, scenario):
             )
         elif scenario == 3:  # wait receive,timeout, no_responses
             transact.comm_params.timeout_connect = 0.1
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 transact.sync_execute(False, request)
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
+            assert exc_info.value.transaction_id == request.transaction_id
         elif scenario == 4:  # wait receive,timeout, disconnect
             transact.comm_params.timeout_connect = 0.1
             transact.count_until_disconnect = -1
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 transact.sync_execute(False, request)
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.transaction_id == request.transaction_id
         elif scenario == 5:  # wait receive,timeout, no_responses pass
             transact.comm_params.timeout_connect = 0.1
             with pytest.raises(ModbusIOException):
@@ -475,17 +493,23 @@ async def test_sync_transaction_execute(self, use_clc, scenario):
             transact.sync_get_response = mock.Mock(return_value=pdu)  # type: ignore[method-assign]
             transact.pdu_send = mock.Mock()  # type: ignore[method-assign]
             transact.comm_params.timeout_connect = 0.2
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 transact.sync_execute(False, request)
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
+            assert exc_info.value.transaction_id == request.transaction_id
         elif scenario == 8:  # response wrong tid
             transact.transport = 1  # type: ignore[assignment]
             pdu = copy.deepcopy(response)
             pdu.transaction_id = 17
             transact.sync_get_response = mock.Mock(return_value=pdu)  # type: ignore[method-assign]
             transact.pdu_send = mock.Mock()  # type: ignore[method-assign]
             transact.comm_params.timeout_connect = 0.2
-            with pytest.raises(ModbusIOException):
+            with pytest.raises(ModbusIOException) as exc_info:
                 transact.sync_execute(False, request)
+            assert exc_info.value.fcode == request.function_code
+            assert exc_info.value.dev_id == request.dev_id
+            assert exc_info.value.transaction_id == request.transaction_id
         else:  # if scenario == 9 # pdu_send from client
             transact.transport = 1  # type: ignore[assignment]
             transact.is_server = True