diff --git a/plugin/core/rpc.py b/plugin/core/rpc.py index 055073f54..2ba24dd5d 100644 --- a/plugin/core/rpc.py +++ b/plugin/core/rpc.py @@ -154,18 +154,16 @@ def response_handler(self, response: 'Dict[str, Any]') -> None: if self.settings.log_payloads: debug(' ' + str(result)) handler, error_handler = self._response_handlers.pop(request_id, (None, None)) - if result is not None and error is None: - if handler: - handler(result) - else: - debug("No handler found for id", request_id) - elif error is not None and result is None: + if error is not None: if error_handler: error_handler(error) else: self._error_display_handler(error.get("message")) else: - debug('invalid response payload', response) + if handler: + handler(result) + else: + debug("No handler found for id", request_id) def on_request(self, request_method: str, handler: 'Callable') -> None: self._request_handlers[request_method] = handler diff --git a/plugin/core/test_rpc.py b/plugin/core/test_rpc.py index 9b377f13c..cded0c000 100644 --- a/plugin/core/test_rpc.py +++ b/plugin/core/test_rpc.py @@ -20,10 +20,14 @@ def __init__(self): self.show_view_status = True -def return_result(message): +def return_empty_dict_result(message): return '{"id": 1, "result": {}}' +def return_null_result(message): + return '{"id": 1, "result": null}' + + def return_error(message): return '{"id": 1, "error": {"message": "oops"}}' @@ -76,7 +80,7 @@ def test_can_create_client(self): self.assertTrue(transport.has_started) def test_client_request_response(self): - transport = MockTransport(return_result) + transport = MockTransport(return_empty_dict_result) settings = MockSettings() client = Client(transport, settings) self.assertIsNotNone(client) @@ -88,6 +92,19 @@ def test_client_request_response(self): # Make sure the response handler dict does not grow. self.assertEqual(len(client._response_handlers), 0) + def test_client_request_with_none_response(self): + transport = MockTransport(return_null_result) + settings = MockSettings() + client = Client(transport, settings) + self.assertIsNotNone(client) + self.assertTrue(transport.has_started) + req = Request.shutdown() + responses = [] + errors = [] + client.send_request(req, lambda resp: responses.append(resp), lambda err: errors.append(err)) + self.assertEqual(len(responses), 1) + self.assertEqual(len(errors), 0) + def test_client_notification(self): transport = MockTransport(notify_pong) settings = MockSettings() @@ -121,7 +138,21 @@ def test_server_request(self): transport.receive('{ "id": 1, "method": "ping"}') self.assertEqual(len(pings), 1) - def test_response_error(self): + def test_error_response_handler(self): + transport = MockTransport(return_error) + settings = MockSettings() + client = Client(transport, settings) + self.assertIsNotNone(client) + self.assertTrue(transport.has_started) + req = Request.initialize(dict()) + errors = [] + responses = [] + client.send_request(req, lambda resp: responses.append(resp), lambda err: errors.append(err)) + self.assertEqual(len(responses), 0) + self.assertGreater(len(errors), 0) + self.assertEqual(len(client._response_handlers), 0) + + def test_error_display_handler(self): transport = MockTransport(return_error) settings = MockSettings() client = Client(transport, settings) @@ -149,7 +180,7 @@ def test_handles_transport_closed_unexpectedly(self): def test_survives_handler_error(self): set_exception_logging(False) - transport = MockTransport(return_result) + transport = MockTransport(return_empty_dict_result) settings = MockSettings() client = Client(transport, settings) self.assertIsNotNone(client)