Repository: palantir/python-jsonrpc-server

Issue:
Tests failing with Python 3.8
```
[   11s] =================================== FAILURES ===================================
[   11s] ______________________________ test_request_error ______________________________
[   11s] 
[   11s] endpoint = <pyls_jsonrpc.endpoint.Endpoint object at 0x7f8e1ab39ee0>
[   11s] consumer = <MagicMock id='140248310062672'>
[   11s] 
[   11s]     def test_request_error(endpoint, consumer):
[   11s]         future = endpoint.request('methodName', {'key': 'value'})
[   11s]         assert not future.done()
[   11s]     
[   11s]         consumer.assert_called_once_with({
[   11s]             'jsonrpc': '2.0',
[   11s]             'id': MSG_ID,
[   11s]             'method': 'methodName',
[   11s]             'params': {'key': 'value'}
[   11s]         })
[   11s]     
[   11s]         # Send an error back from the client
[   11s]         error = exceptions.JsonRpcInvalidRequest(data=1234)
[   11s] >       endpoint.consume({
[   11s]             'jsonrpc': '2.0',
[   11s]             'id': MSG_ID,
[   11s]             'error': error.to_dict()
[   11s]         })
[   11s] 
[   11s] test/test_endpoint.py:86: 
[   11s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
[   11s] pyls_jsonrpc/endpoint.py:109: in consume
[   11s]     self._handle_response(message['id'], message.get('result'), message.get('error'))
[   11s] pyls_jsonrpc/endpoint.py:241: in _handle_response
[   11s]     request_future.set_result(result)
[   11s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
[   11s] 
[   11s] self = <Future at 0x7f8e1ab3fb20 state=finished raised JsonRpcInvalidRequest>
[   11s] result = None
[   11s] 
[   11s]     def set_result(self, result):
[   11s]         """Sets the return value of work associated with the future.
[   11s]     
[   11s]         Should only be used by Executor implementations and unit tests.
[   11s]         """
[   11s]         with self._condition:
[   11s]             if self._state in {CANCELLED, CANCELLED_AND_NOTIFIED, FINISHED}:
[   11s] >               raise InvalidStateError('{}: {!r}'.format(self._state, self))
[   11s] E               concurrent.futures._base.InvalidStateError: FINISHED: <Future at 0x7f8e1ab3fb20 state=finished raised JsonRpcInvalidRequest>
[   11s] 
[   11s] /usr/lib64/python3.8/concurrent/futures/_base.py:524: InvalidStateError
[   11s] _____________________________ test_request_cancel ______________________________
[   11s] 
[   11s] endpoint = <pyls_jsonrpc.endpoint.Endpoint object at 0x7f8e1ad6f670>
[   11s] consumer = <MagicMock id='140248312379232'>
[   11s] 
[   11s]     def test_request_cancel(endpoint, consumer):
[   11s]         future = endpoint.request('methodName', {'key': 'value'})
[   11s]         assert not future.done()
[   11s]     
[   11s]         consumer.assert_called_once_with({
[   11s]             'jsonrpc': '2.0',
[   11s]             'id': MSG_ID,
[   11s]             'method': 'methodName',
[   11s]             'params': {'key': 'value'}
[   11s]         })
[   11s]     
[   11s]         # Cancel the request
[   11s]         future.cancel()
[   11s]         consumer.assert_any_call({
[   11s]             'jsonrpc': '2.0',
[   11s]             'method': '$/cancelRequest',
[   11s]             'params': {'id': MSG_ID}
[   11s]         })
[   11s]     
[   11s]         with pytest.raises(exceptions.JsonRpcException) as exc_info:
[   11s] >           assert future.result(timeout=2)
[   11s] 
[   11s] test/test_endpoint.py:119: 
[   11s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
[   11s] 
[   11s] self = <Future at 0x7f8e1ab372b0 state=cancelled>, timeout = 2
[   11s] 
[   11s]     def result(self, timeout=None):
[   11s]         """Return the result of the call that the future represents.
[   11s]     
[   11s]         Args:
[   11s]             timeout: The number of seconds to wait for the result if the future
[   11s]                 isn't done. If None, then there is no limit on the wait time.
[   11s]     
[   11s]         Returns:
[   11s]             The result of the call that the future represents.
[   11s]     
[   11s]         Raises:
[   11s]             CancelledError: If the future was cancelled.
[   11s]             TimeoutError: If the future didn't finish executing before the given
[   11s]                 timeout.
[   11s]             Exception: If the call raised then that exception will be raised.
[   11s]         """
[   11s]         with self._condition:
[   11s]             if self._state in [CANCELLED, CANCELLED_AND_NOTIFIED]:
[   11s] >               raise CancelledError()
[   11s] E               concurrent.futures._base.CancelledError
[   11s] 
[   11s] /usr/lib64/python3.8/concurrent/futures/_base.py:430: CancelledError
[   11s] ------------------------------ Captured log call -------------------------------
[   11s] ERROR    concurrent.futures:_base.py:330 exception calling callback for <Future at 0x7f8e1ab372b0 state=cancelled>
[   11s] Traceback (most recent call last):
[   11s]   File "/usr/lib64/python3.8/concurrent/futures/_base.py", line 328, in _invoke_callbacks
[   11s]     callback(self)
[   11s]   File "/home/abuild/rpmbuild/BUILD/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py", line 91, in callback
[   11s]     future.set_exception(JsonRpcRequestCancelled())
[   11s]   File "/usr/lib64/python3.8/concurrent/futures/_base.py", line 539, in set_exception
[   11s]     raise InvalidStateError('{}: {!r}'.format(self._state, self))
[   11s] concurrent.futures._base.InvalidStateError: CANCELLED: <Future at 0x7f8e1ab372b0 state=cancelled>
[   11s] ___________________________ test_writer_bad_message ____________________________
[   11s] 
[   11s] wfile = <_io.BytesIO object at 0x7f8e1ab2d5e0>
[   11s] writer = <pyls_jsonrpc.streams.JsonRpcStreamWriter object at 0x7f8e1aab2580>
[   11s] 
[   11s]     def test_writer_bad_message(wfile, writer):
[   11s]         # A datetime isn't serializable(or poorly serializable),
[   11s]         # ensure the write method doesn't throw
[   11s]         import datetime
[   11s]         writer.write(datetime.datetime(
[   11s]             year=2019,
[   11s]             month=1,
[   11s]             day=1,
[   11s]             hour=1,
[   11s]             minute=1,
[   11s]             second=1,
[   11s]         ))
[   11s]     
[   11s]         if os.name == 'nt':
[   11s]             assert wfile.getvalue() == b''
[   11s]         else:
[   11s] >           assert wfile.getvalue() == (
[   11s]                 b'Content-Length: 10\r\n'
[   11s]                 b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
[   11s]                 b'\r\n'
[   11s]                 b'1546304461'
[   11s]             )
[   11s] E           AssertionError: assert b'' == b'Content-Len...r\n1546304461'
[   11s] E             Full diff:
[   11s] E             - b''
[   11s] E             + (
[   11s] E             +  b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=ut'
[   11s] E             +  b'f8\r\n\r\n1546304461',
[   11s] E             + )
[   11s] 
[   11s] test/test_streams.py:114: AssertionError
[   11s] ------------------------------ Captured log call -------------------------------
[   11s] ERROR    pyls_jsonrpc.streams:streams.py:112 Failed to write message to output file 2019-01-01 01:01:01
[   11s] Traceback (most recent call last):
[   11s]   File "/home/abuild/rpmbuild/BUILD/python-jsonrpc-server-0.3.4/pyls_jsonrpc/streams.py", line 98, in write
[   11s]     body = json.dumps(message, **self._json_dumps_args)
[   11s] TypeError: ï¿½ is not JSON serializable
[   11s] =============================== warnings summary ===============================
[   11s] test/test_endpoint.py::test_bad_message
[   11s]   /home/abuild/rpmbuild/BUILD/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py:101: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
[   11s]     log.warn("Unknown message type %s", message)
[   11s] 
[   11s] test/test_endpoint.py::test_consume_notification_method_not_found
[   11s]   /home/abuild/rpmbuild/BUILD/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py:138: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
[   11s]     log.warn("Ignoring notification for unknown method %s", method)
[   11s] 
[   11s] test/test_endpoint.py::test_consume_request_cancel_unknown
[   11s]   /home/abuild/rpmbuild/BUILD/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py:168: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
[   11s]     log.warn("Received cancel notification for unknown message id %s", msg_id)
[   11s] 
[   11s] -- Docs: https://docs.pytest.org/en/latest/warnings.html
[   11s] =================== 3 failed, 24 passed, 3 warnings in 0.20s ===================
```

[Full build log with all details](https://github.com/palantir/python-jsonrpc-server/files/4328630/_log.txt)


Discussion:
ccordoba12: @delta003, I'm sorry to bother you again, but I'm unable to merge this and other PRs.

This is what I'm seeing on Github:

![Selección_012](https://user-images.githubusercontent.com/365293/90038894-906f4e80-dc8b-11ea-9b5a-e860b233e542.png)

I don't know if that's caused because the `policy-bot` check is not passing. Could you help me to solve this?
ccordoba12: Pinging @delta003 and @uschi2000 about this one again. Could you grant me the right permissions to merge PRs here please?
bnavigator: When I click on "policy-bot ... Details" the policy-bot wants to access by Github account:

> Verify your GitHub identity (bnavigator)
> Know which resources you can access
> Act on your behalf

Why would I need to do that? @palantirtech, known for creating law-enforcement and intelligence software compromising human rights is not exactly the company I want to give those permissions!

Is this the reason why merging this PR is not possible? Tough luck. Then close and commit the changes indepently. I forfeit the "copyright".
ccordoba12: @bnavigator, as far as I can tell, contributors have nothing to do with the Policy bot. Don't worry about it.
delta003: Should be good now
ccordoba12: Thanks @delta003 for your help! Now the `policy-bot` entry doesn't appear, but it seems I'm still missing the right permissions to merge PRs here. That's what I'm seeing now

![Selección_024](https://user-images.githubusercontent.com/365293/90275033-4622d480-de27-11ea-95bb-795ba2589082.png)

As you can at the end, according to Github I'm not authorized to merge this PR.

Patch:
diff --git a/pyls_jsonrpc/endpoint.py b/pyls_jsonrpc/endpoint.py
index e8bfb5b..0caf612 100644
--- a/pyls_jsonrpc/endpoint.py
+++ b/pyls_jsonrpc/endpoint.py
@@ -236,6 +236,7 @@ class Endpoint(object):
         if error is not None:
             log.debug("Received error response to message %s: %s", msg_id, error)
             request_future.set_exception(JsonRpcException.from_dict(error))
+            return
 
         log.debug("Received result for message %s: %s", msg_id, result)
         request_future.set_result(result)


Test patch:
diff --git a/test/test_endpoint.py b/test/test_endpoint.py
index 47a038b..b954732 100644
--- a/test/test_endpoint.py
+++ b/test/test_endpoint.py
@@ -115,9 +115,9 @@ def test_request_cancel(endpoint, consumer):
         'params': {'id': MSG_ID}
     })
 
-    with pytest.raises(exceptions.JsonRpcException) as exc_info:
+    with pytest.raises((exceptions.JsonRpcException, futures.CancelledError)) as exc_info:
         assert future.result(timeout=2)
-    assert exc_info.type == exceptions.JsonRpcRequestCancelled
+    assert exc_info.type in (exceptions.JsonRpcRequestCancelled, futures.CancelledError)
 
 
 def test_consume_notification(endpoint, dispatcher):
diff --git a/test/test_streams.py b/test/test_streams.py
index 8c2e93e..480a73b 100644
--- a/test/test_streams.py
+++ b/test/test_streams.py
@@ -97,7 +97,8 @@ def test_writer(wfile, writer):
 
 def test_writer_bad_message(wfile, writer):
     # A datetime isn't serializable(or poorly serializable),
-    # ensure the write method doesn't throw
+    # ensure the write method doesn't throw, but the result could be empty
+    # or the correct datetime
     import datetime
     writer.write(datetime.datetime(
         year=2019,
@@ -108,12 +109,10 @@ def test_writer_bad_message(wfile, writer):
         second=1,
     ))
 
-    if os.name == 'nt':
-        assert wfile.getvalue() == b''
-    else:
-        assert wfile.getvalue() == (
-            b'Content-Length: 10\r\n'
-            b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
-            b'\r\n'
-            b'1546304461'
-        )
+    assert wfile.getvalue() in [
+        b'',
+        b'Content-Length: 10\r\n'
+        b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
+        b'\r\n'
+        b'1546304461'
+    ]