diff --git a/pymodbus/exceptions.py b/pymodbus/exceptions.py index 8d8392d00..d96211e3d 100644 --- a/pymodbus/exceptions.py +++ b/pymodbus/exceptions.py @@ -36,12 +36,24 @@ def isError(self): class ModbusIOException(ModbusException): """Error resulting from data i/o.""" - def __init__(self, string="", function_code=None): + def __init__( + self, + string="", + function_code=None, + *, + transaction_id: int = 0, + dev_id: int = 0, + ): """Initialize the exception. :param string: The message to append to the error + :param function_code: Optional function code associated with the error + :param transaction_id: Optional framing transaction id (e.g. MBAP TID) + :param dev_id: Optional framing device / unit id """ self.fcode = function_code + self.transaction_id = transaction_id + self.dev_id = dev_id self.message = f"[Input/Output] {string}" ModbusException.__init__(self, self.message) diff --git a/pymodbus/framer/base.py b/pymodbus/framer/base.py index 773157797..df908d90a 100644 --- a/pymodbus/framer/base.py +++ b/pymodbus/framer/base.py @@ -88,7 +88,15 @@ def handleFrame( ) continue if (pdu := self.decoder.decode(frame_data)) is None: - raise ModbusIOException("Unable to decode request") + # 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, + dev_id=dev_id, + ) pdu.dev_id = dev_id pdu.transaction_id = tid return used_len, pdu diff --git a/pymodbus/server/requesthandler.py b/pymodbus/server/requesthandler.py index ebaf52917..170391a1b 100644 --- a/pymodbus/server/requesthandler.py +++ b/pymodbus/server/requesthandler.py @@ -62,8 +62,17 @@ def callback_data(self, data: bytes, addr: tuple | None = None) -> int: """Handle received data.""" try: used_len = super().callback_data(data, addr) - except ModbusIOException: - response = ExceptionResponse(40, exception_code=ExcCodes.ILLEGAL_FUNCTION) + 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. + response = ExceptionResponse( + 0x00, + exception_code=ExcCodes.ILLEGAL_FUNCTION, + device_id=exc.dev_id, + transaction=exc.transaction_id, + ) self.server_send(response, 0) return len(data) if self.last_pdu: diff --git a/test/server/test_requesthandler.py b/test/server/test_requesthandler.py index ebd171354..668f60c89 100755 --- a/test/server/test_requesthandler.py +++ b/test/server/test_requesthandler.py @@ -53,6 +53,59 @@ async def test_rh_callback_data(self, requesthandler): data = b"012" assert len(data) == requesthandler.callback_data(data, None) + async def test_rh_callback_data_undecodable_echoes_identity(self, requesthandler): + """Undecodable FC exception must echo framing tid/dev_id (#2990).""" + with mock.patch( + "pymodbus.transaction.TransactionManager.callback_data" + ) as cb_data: + exc = ModbusIOException( + "Unable to decode request", + transaction_id=0x000A, + dev_id=7, + ) + 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) + + 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 + + async def test_rh_callback_data_undecodable_without_framing_attrs( + self, requesthandler + ): + """Missing framing attrs fall back to zeros rather than crashing.""" + with mock.patch( + "pymodbus.transaction.TransactionManager.callback_data" + ) as cb_data: + cb_data.side_effect = ModbusIOException("Unable to decode request") + data = b"garbage" + assert len(data) == requesthandler.callback_data(data, None) + + response = requesthandler.pdu_send.call_args.args[0] + assert response.transaction_id == 0 + assert response.dev_id == 0 + assert response.function_code == 0x80 + + async def test_rh_callback_data_undecodable_real_socket_frame(self, requesthandler): + """End-to-end: undecodable socket FC echoes MBAP transaction id.""" + # MBAP: tid=0x1234, pid=0, len=6, uid=1, PDU: FC=0x09 + 4 data bytes + frame = b"\x12\x34\x00\x00\x00\x06\x01\x09\x00\x00\x00\x01" + assert len(frame) == requesthandler.callback_data(frame, None) + + requesthandler.pdu_send.assert_called_once() + response = requesthandler.pdu_send.call_args.args[0] + assert isinstance(response, ExceptionResponse) + assert response.transaction_id == 0x1234 + assert response.dev_id == 1 + assert response.function_code == 0x80 + assert response.exception_code == 0x01 + async def test_rh_handle_request(self, requesthandler): """Test __init__.""" requesthandler.last_pdu = None