Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make websocket server to send ping frame to client #459

Merged
merged 3 commits into from
Jan 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
## CHANGELOG

### `jupyter-lsp 1.0.1` (unreleased)

- bug fixes:

- send periodic pings on websocket channels to maintain connection ([#459], thanks @franckchen)

[#459]: https://github.com/krassowski/jupyterlab-lsp/pull/459

### `@krassowski/jupyterlab-lsp 3.0.0` (2021-01-06)

- features
Expand Down
1 change: 1 addition & 0 deletions python_packages/jupyter_lsp/jupyter_lsp/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def open(self, language_server):
self.language_server = language_server
self.manager.subscribe(self)
self.log.debug("[{}] Opened a handler".format(self.language_server))
super().open()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good catch. might we even want to super.open first, before any overloaded behavior?


async def on_message(self, message):
self.log.debug("[{}] Handling a message".format(self.language_server))
Expand Down
10 changes: 9 additions & 1 deletion python_packages/jupyter_lsp/jupyter_lsp/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

from jupyter_server.serverapp import ServerApp
from pytest import fixture
from tornado.httputil import HTTPServerRequest
from tornado.queues import Queue
from tornado.web import Application

# local imports
from jupyter_lsp import LanguageServerManager
Expand Down Expand Up @@ -87,18 +89,24 @@ def app():
# mocks
class MockWebsocketHandler(LanguageServerWebSocketHandler):
_messages_wrote = None # type: Queue
_ping_sent = None # type: bool

def __init__(self):
pass
self.request = HTTPServerRequest()
self.application = Application()

def initialize(self, manager):
super().initialize(manager)
self._messages_wrote = Queue()
self._ping_sent = False

def write_message(self, message: Text) -> None:
self.log.warning("write_message %s", message)
self._messages_wrote.put_nowait(message)

def send_ping(self):
self._ping_sent = True


class MockHandler(LanguageServersHandler):
_payload = None
Expand Down
27 changes: 27 additions & 0 deletions python_packages/jupyter_lsp/jupyter_lsp/tests/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,30 @@ async def test_start_unknown(known_unknown_server, handlers, jsonrpc_init_msg):

assert not manager.sessions.get(ws_handler.language_server)
assert_status_set(handler, {"not_started"})


@pytest.mark.asyncio
async def test_ping(handlers):
"""see https://github.com/krassowski/jupyterlab-lsp/issues/458"""
a_server = "pyls"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just do not think it is needed to run this check for all language servers (just wanted to have something to pass to ws_handler.open()) but did not want to create a mock either.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, yeah, maybe we need a real pygls mock server at some point :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with bringing pygls as a mock dependency in the future.


handler, ws_handler = handlers
manager = handler.manager

manager.initialize()

assert ws_handler.ping_interval > 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are looking good

# the default ping interval is 30 seconds, too long for a test
ws_handler.settings["ws_ping_interval"] = 0.1
assert ws_handler.ping_interval == 0.1

assert ws_handler._ping_sent is False

ws_handler.open(a_server)

assert ws_handler.ping_callback is not None and ws_handler.ping_callback.is_running
await asyncio.sleep(ws_handler.ping_interval * 3)

assert ws_handler._ping_sent is True

ws_handler.on_close()