Skip to content

Commit

Permalink
fix: Improved error handling
Browse files Browse the repository at this point in the history
Notable behaviour change:
All errors outside of LSP requests are now sent to the client as
`showMessage.type = MessageType.Error` messages. This may result
in existing custom LSP servers showing errors that were not seen before,
these aren't new errors, just previously undisplayed errors.

`JsonRPCProtocol.data_receieved()` now catches unhandled errors.

Fixes #277
  • Loading branch information
tombh committed Oct 25, 2022
1 parent 998cac5 commit 2a18b7f
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 7 deletions.
13 changes: 13 additions & 0 deletions docs/source/pages/advanced_usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,19 @@ And method invocation example:
server.send_notification('myCustomNotification', 'test data')
Custom Error Reporting
^^^^^^^^^^^^^^^^^^^^^^

By default Pygls notifies the client to display any occurences of uncaught exceptions in the
server. To override this behaviour define your own `report_server_error()` method like so:

.. code:: python
Class CustomLanguageServer(LanguageServer):
def report_server_error(self, message: str):
pass
Workspace
~~~~~~~~~

Expand Down
23 changes: 22 additions & 1 deletion pygls/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,11 @@ def _handle_notification(self, method_name, params):
logger.warning('Ignoring notification for unknown method "%s"', method_name)
except Exception:
logger.exception('Failed to handle notification "%s": %s', method_name, params)
try:
message = "Server failed to handle notification, see server logs for more details"
self._server.report_server_error(message)
except Exception:
logger.warning("Failed to report error to client")

def _handle_request(self, msg_id, method_name, params):
"""Handles a request from the client."""
Expand Down Expand Up @@ -377,7 +382,6 @@ def _send_data(self, data):
"""Sends data to the client."""
if not data:
return

try:
body = data.json(by_alias=True, exclude_unset=True, encoder=default_serializer)
logger.info('Sending data: %s', body)
Expand All @@ -392,8 +396,14 @@ def _send_data(self, data):
self.transport.write(header + body)
else:
self.transport.write(body.decode('utf-8'))
print(data)
except Exception:
logger.error(traceback.format_exc())
try:
message = "Couldn't send data to editor, see server logs for more details"
self._server.report_server_error(message)
except Exception:
logger.warning("Failed to report error to client")

def _send_response(self, msg_id, result=None, error=None):
"""Sends a JSON RPC response to the client.
Expand Down Expand Up @@ -427,6 +437,17 @@ def connection_made(self, transport: asyncio.BaseTransport):
self.transport = transport

def data_received(self, data: bytes):
try:
self._data_received(data)
except Exception:
logger.error(traceback.format_exc())
try:
message = "Couldn't read data from editor, see server logs for more details"
self._server.report_server_error(message)
except Exception:
logger.warning("Failed to report error to client")

def _data_received(self, data: bytes):
"""Method from base class, called when server receives the data"""
logger.debug('Received %r', data)

Expand Down
4 changes: 4 additions & 0 deletions pygls/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,10 @@ def show_message_log(self, message, msg_type=MessageType.Log) -> None:
"""Sends message to the client's output channel."""
self.lsp.show_message_log(message, msg_type)

def report_server_error(self, message: str):
"""Sends error to the client for displaying."""
self.show_message(message, msg_type=MessageType.Error)

def thread(self) -> Callable[[F], F]:
"""Decorator that mark function to execute it in a thread."""
return self.lsp.thread()
Expand Down
12 changes: 6 additions & 6 deletions tests/ls_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ class PyodideClientServer:
"""Implementation of the `client_server` fixture for use in a pyodide
environment."""

def __init__(self):
def __init__(self, LS=LanguageServer):

self.server = LanguageServer()
self.client = LanguageServer()
self.server = LS()
self.client = LS()

self.server.lsp.connection_made(PyodideTestTransportAdapter(self.client))
self.server.lsp._send_only_body = True
Expand Down Expand Up @@ -112,22 +112,22 @@ def __iter__(self):


class NativeClientServer:
def __init__(self):
def __init__(self, LS=LanguageServer):
# Client to Server pipe
csr, csw = os.pipe()
# Server to client pipe
scr, scw = os.pipe()

# Setup Server
self.server = LanguageServer()
self.server = LS()
self.server_thread = threading.Thread(
target=self.server.start_io,
args=(os.fdopen(csr, "rb"), os.fdopen(scw, "wb")),
)
self.server_thread.daemon = True

# Setup client
self.client = LanguageServer(asyncio.new_event_loop())
self.client = LS(asyncio.new_event_loop())
self.client_thread = threading.Thread(
target=self.client.start_io,
args=(os.fdopen(scr, "rb"), os.fdopen(csw, "wb")),
Expand Down
86 changes: 86 additions & 0 deletions tests/lsp/test_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
############################################################################
# Copyright(c) Open Law Library. All rights reserved. #
# See ThirdPartyNotices.txt in the project root for additional notices. #
# #
# Licensed under the Apache License, Version 2.0 (the "License") #
# you may not use this file except in compliance with the License. #
# You may obtain a copy of the License at #
# #
# http: // www.apache.org/licenses/LICENSE-2.0 #
# #
# Unless required by applicable law or agreed to in writing, software #
# distributed under the License is distributed on an "AS IS" BASIS, #
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. #
# See the License for the specific language governing permissions and #
# limitations under the License. #
############################################################################

from typing import Any, List
import time

import pytest

from pygls.exceptions import JsonRpcInternalError
from pygls.lsp.methods import WINDOW_SHOW_MESSAGE
from pygls.server import LanguageServer

from ..conftest import ClientServer

ERROR_TRIGGER = "test/triggerError"
ERROR_MESSAGE = "Testing errors"


class CustomLanguageServer(LanguageServer):
def report_server_error(self, message: str):
pass


class ConfiguredLS(ClientServer):
def __init__(self, LS=LanguageServer):
super().__init__(LS)
self.init()

def init(self):
self.client.messages: List[str] = []

@self.server.feature(ERROR_TRIGGER)
def f1(params: Any):
raise Exception(ERROR_MESSAGE)

@self.client.feature(WINDOW_SHOW_MESSAGE)
def f2(params: Any):
self.client.messages.append(params.message)


class CustomConfiguredLS(ConfiguredLS):
def __init__(self):
super().__init__(CustomLanguageServer)


@ConfiguredLS.decorate()
def test_request_error_reporting(client_server):
client, _ = client_server
with pytest.raises(JsonRpcInternalError, match=ERROR_MESSAGE):
client.lsp.send_request(ERROR_TRIGGER).result()


@ConfiguredLS.decorate()
def test_notification_error_reporting(client_server):
client, _ = client_server
client.lsp.notify(ERROR_TRIGGER)
time.sleep(0.1)

assert len(client.messages) == 1
error_report = (
"Server failed to handle notification, see server logs for more details"
)
assert client.messages[0] == error_report


@CustomConfiguredLS.decorate()
def test_overriding_error_reporting(client_server):
client, _ = client_server
client.lsp.notify(ERROR_TRIGGER)
time.sleep(0.1)

assert len(client.messages) == 0

0 comments on commit 2a18b7f

Please sign in to comment.