Fix: we should be able to handle a "null" result from a server

plugin/core/rpc.py

@@ -149,17 +149,16 @@ def on_transport_closed(self) -> None:
 
     def response_handler(self, response: 'Dict[str, Any]') -> None:
         request_id = int(response["id"])
-        result = response.get("result", None)
-        error = response.get("error", None)
         if self.settings.log_payloads:
-            debug('     ' + str(result))
+            debug('     ' + str(response.get("result", None)))
         handler, error_handler = self._response_handlers.pop(request_id, (None, None))
-        if result is not None and error is None:
+        if "result" in response and "error" not in response:
             if handler:
-                handler(result)
+                handler(response["result"])
             else:
                 debug("No handler found for id", request_id)
-        elif error is not None and result is None:
+        elif "result" not in response and "error" in response:
+            error = response["error"]
             if error_handler:
                 error_handler(error)
             else:

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,41 @@ 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_should_reject_response_when_both_result_and_error_are_present(self):
+        transport = MockTransport(lambda x: '{"id": 1, "result": {"key": "value"}, "error": {"message": "oops"}}')
+        settings = MockSettings()
+        client = Client(transport, settings)
+        req = Request.initialize(dict())
+        responses = []
+        errors = []
+        client.send_request(req, lambda resp: responses.append(resp), lambda err: errors.append(err))
+        self.assertEqual(len(responses), 0)
+        self.assertEqual(len(errors), 0)
+
+    def test_client_should_reject_response_when_both_result_and_error_keys_are_not_present(self):
+        transport = MockTransport(lambda x: '{"id": 1}')
+        settings = MockSettings()
+        client = Client(transport, settings)
+        req = Request.initialize(dict())
+        responses = []
+        errors = []
+        client.send_request(req, lambda resp: responses.append(resp), lambda err: errors.append(err))
+        self.assertEqual(len(responses), 0)
+        self.assertEqual(len(errors), 0)
+
     def test_client_notification(self):
         transport = MockTransport(notify_pong)
         settings = MockSettings()
@@ -121,7 +160,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 +202,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)