Skip to content

Commit

Permalink
Revert back to old behavior: reject response when both 'result' and '…
Browse files Browse the repository at this point in the history
…error' keys are present, as well as when both are missing
  • Loading branch information
rwols committed Nov 10, 2018
1 parent df4cdc2 commit 9a5262f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
17 changes: 9 additions & 8 deletions plugin/core/rpc.py
Expand Up @@ -149,21 +149,22 @@ 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 error is not None:
if "result" in response and "error" not in response:
if handler:
handler(response["result"])
else:
debug("No handler found for id", request_id)
elif "result" not in response and "error" in response:
error = response["error"]
if error_handler:
error_handler(error)
else:
self._error_display_handler(error.get("message"))
else:
if handler:
handler(result)
else:
debug("No handler found for id", request_id)
debug('invalid response payload', response)

def on_request(self, request_method: str, handler: 'Callable') -> None:
self._request_handlers[request_method] = handler
Expand Down
22 changes: 22 additions & 0 deletions plugin/core/test_rpc.py
Expand Up @@ -105,6 +105,28 @@ def test_client_request_with_none_response(self):
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()
Expand Down

0 comments on commit 9a5262f

Please sign in to comment.