diff --git a/src/arduino/app_utils/bridge.py b/src/arduino/app_utils/bridge.py index 0c9ed523..3916c36c 100644 --- a/src/arduino/app_utils/bridge.py +++ b/src/arduino/app_utils/bridge.py @@ -516,10 +516,10 @@ def _handle_msg(self, msg: list): try: if msg_type == 0: # Request: [0, msgid, method, params] if len(msg) != 4: - raise ValueError("Invalid RPC request: expected length 4") + raise ValueError(f"Invalid RPC request: expected length 4, got {len(msg)}") _, msgid, method, params = msg if not isinstance(params, (list, tuple)): - raise ValueError("Invalid RPC request params: expected array/tuple") + raise ValueError("Invalid RPC request params: expected array or tuple") method_name = self._decode_method(method) @@ -538,7 +538,7 @@ def _handle_msg(self, msg: list): elif msg_type == 1: # Response: [1, msgid, error, result] if len(msg) != 4: - raise ValueError("Invalid RPC response: expected length 4") + raise ValueError(f"Invalid RPC response: expected length 4, got {len(msg)}") _, msgid, error, result = msg if error and (not isinstance(error, list) or len(error) < 2): raise ValueError("Invalid error format in RPC response") @@ -559,11 +559,11 @@ def _handle_msg(self, msg: list): else: on_result([GENERIC_ERR, "Unknown error occurred."]) else: - on_error([GENERIC_ERR, f"Response for unknown msgid {msgid} received."]) + logger.warning(f"Response for unknown msgid {msgid} received.") elif msg_type == 2: # Notification: [2, method, params] if len(msg) != 3: - raise ValueError("Invalid RPC notification: expected length 3") + raise ValueError(f"Invalid RPC notification: expected length 3, got {len(msg)}") _, method, params = msg if not isinstance(params, (list, tuple)): raise ValueError("Invalid RPC notification params: expected array or tuple") diff --git a/tests/arduino/app_utils/bridge/test_unit_common.py b/tests/arduino/app_utils/bridge/test_unit_common.py index dcd81fee..246e1c57 100644 --- a/tests/arduino/app_utils/bridge/test_unit_common.py +++ b/tests/arduino/app_utils/bridge/test_unit_common.py @@ -15,7 +15,9 @@ def setUp(self): # Patch dependencies # Mock the logger used by ClientServer - patch("arduino.app_utils.bridge.logger", MagicMock()).start() + self.mock_logger = MagicMock() + self.logger_patcher = patch("arduino.app_utils.bridge.logger", self.mock_logger) + self.logger_patcher.start() # Mock the socket instance that will be created self.mock_socket_instance = MagicMock() @@ -32,5 +34,6 @@ def setUp(self): def tearDown(self): """This method is called after each test and cleans up the patched dependencies.""" + self.logger_patcher.stop() self.socket_patcher.stop() self.threading_patcher.stop() diff --git a/tests/arduino/app_utils/bridge/test_unit_handle_msg.py b/tests/arduino/app_utils/bridge/test_unit_handle_msg.py index 8090465c..eb8e7666 100644 --- a/tests/arduino/app_utils/bridge/test_unit_handle_msg.py +++ b/tests/arduino/app_utils/bridge/test_unit_handle_msg.py @@ -9,6 +9,58 @@ class TestHandleMsg(UnitTest): + def test_empty_msg(self): + """Test handling of an empty message.""" + client = ClientServer() + client._handle_msg([]) + self.mock_logger.warning.assert_called_once_with("Invalid RPC message received (must be a non-empty list).") + self.mock_logger.error.assert_not_called() + + def test_unknown_msg_type(self): + """Test handling of an unknown message type.""" + client = ClientServer() + client._handle_msg([99, 1, None, "result"]) # Msg type 99 does not exist + self.mock_logger.warning.assert_called_once_with("Invalid RPC message type received: 99") + self.mock_logger.error.assert_not_called() + + def test_unknown_msg_id(self): + """Test handling of an unknown message id.""" + client = ClientServer() + client._handle_msg([1, 9999, None, "result"]) # Msg id 9999 does not exist + self.mock_logger.warning.assert_called_once_with("Response for unknown msgid 9999 received.") + self.mock_logger.error.assert_not_called() + + def test_malformed_messages(self): + """Test handling of malformed messages.""" + client = ClientServer() + + client._handle_msg([0, 1, "method", [0, 1], "extra field"]) # Malformed payload + self.mock_logger.warning.assert_not_called() + self.mock_logger.error.assert_called_once_with("Message validation error: Invalid RPC request: expected length 4, got 5") + self.mock_logger.reset_mock() + client._handle_msg([0, 1, "method", 1]) # Malformed params + self.mock_logger.warning.assert_not_called() + self.mock_logger.error.assert_called_once_with("Message validation error: Invalid RPC request params: expected array or tuple") + self.mock_logger.reset_mock() + + client._handle_msg([1, 1, None, "result", "extra field"]) # Malformed payload + self.mock_logger.warning.assert_not_called() + self.mock_logger.error.assert_called_once_with("Message validation error: Invalid RPC response: expected length 4, got 5") + self.mock_logger.reset_mock() + client._handle_msg([1, 1, 42, "result"]) # Malformed error + self.mock_logger.warning.assert_not_called() + self.mock_logger.error.assert_called_once_with("Message validation error: Invalid error format in RPC response") + self.mock_logger.reset_mock() + + client._handle_msg([2, 1, [0, 1], "extra field"]) # Malformed payload + self.mock_logger.warning.assert_not_called() + self.mock_logger.error.assert_called_once_with("Message validation error: Invalid RPC notification: expected length 3, got 4") + self.mock_logger.reset_mock() + client._handle_msg([2, 1, 42]) # Malformed params + self.mock_logger.warning.assert_not_called() + self.mock_logger.error.assert_called_once_with("Message validation error: Invalid RPC notification params: expected array or tuple") + self.mock_logger.reset_mock() + def test_handle_msg_request(self): """Test handling of an incoming request message.""" client = ClientServer() @@ -27,6 +79,22 @@ def test_handle_msg_request(self): handler_mock.assert_called_once_with(*params) client._send_response.assert_called_once_with(msgid, None, "handled") + def test_handle_msg_request_handler_fail(self): + """Test handling of a request for a method that fails running its handler.""" + client = ClientServer() + client._send_response = MagicMock() + + request_msg = [0, 111, "failing_method", []] + client.handlers["failing_method"] = MagicMock(side_effect=ValueError("Handler failed")) + + client._handle_msg(request_msg) + + client._send_response.assert_called_once() + args, _ = client._send_response.call_args + self.assertEqual(args[0], 111) # msgid + self.assertIsInstance(args[1], ValueError) # error + self.assertIsNone(args[2]) # result + def test_handle_msg_request_method_not_found(self): """Test handling of a request for a method that is not found.""" client = ClientServer() @@ -79,7 +147,7 @@ def test_handle_msg_response(self): on_error_mock.assert_not_called() self.assertNotIn(msgid, client.callbacks) # Callback should be removed - def test_handle_msg_response_function_not_found(self): + def test_handle_msg_generic_error_response(self): """Test handling of an incoming error response message.""" client = ClientServer() @@ -100,7 +168,7 @@ def test_handle_msg_response_function_not_found(self): on_error_mock.assert_called_once_with(result_error) self.assertNotIn(msgid, client.callbacks) # Callback should be removed - def test_handle_msg_response_already_provided_error(self): + def test_handle_msg_method_exists_error_response(self): """Test handling of an incoming error response message that signals a method is already provided.""" client = ClientServer()