Skip to content

Commit

Permalink
Report progress even when initialization fails (#381)
Browse files Browse the repository at this point in the history
  • Loading branch information
syphar committed Jun 29, 2023
1 parent 05ecbdc commit fd4a0df
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 25 deletions.
4 changes: 3 additions & 1 deletion pylsp/plugins/_rope_task_handle.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ def __init__(self, workspace: Workspace):
self.observers = []

def create_jobset(self, name="JobSet", count: Optional[int] = None):
report_iter = self.workspace.report_progress(name, None, None)
report_iter = self.workspace.report_progress(
name, None, None, skip_token_initialization=True
)
result = PylspJobSet(count, report_iter)
self.job_sets.append(result)
self._inform_observers()
Expand Down
57 changes: 37 additions & 20 deletions pylsp/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,20 @@ def report_progress(
title: str,
message: Optional[str] = None,
percentage: Optional[int] = None,
skip_token_initialization: bool = False,
) -> Generator[Callable[[str, Optional[int]], None], None, None]:
"""
Report progress to the editor / client.
``skip_token_initialization` is necessary due to some current
limitations of our LSP implementation. When `report_progress`
is used from a synchronous LSP handler, the token initialization
will time out because we can't receive the response.
Many editors will still correctly show the progress messages though, which
is why we are giving progress users the option to skip the initialization
of the progress token.
"""
if self._config:
client_supports_progress_reporting = (
self._config.capabilities.get("window", {}).get("workDoneProgress", False)
Expand All @@ -144,30 +157,21 @@ def report_progress(
client_supports_progress_reporting = False

if client_supports_progress_reporting:
try:
token = self._progress_begin(title, message, percentage)
except Exception: # pylint: disable=broad-exception-caught
log.warning(
"There was an error while trying to initialize progress reporting."
"Likely progress reporting was used in a synchronous LSP handler, "
"which is not supported by progress reporting yet.",
exc_info=True
)
token = self._progress_begin(title, message, percentage, skip_token_initialization)

else:
def progress_message(message: str, percentage: Optional[int] = None) -> None:
self._progress_report(token, message, percentage)
def progress_message(message: str, percentage: Optional[int] = None) -> None:
self._progress_report(token, message, percentage)

try:
yield progress_message
finally:
self._progress_end(token)
try:
yield progress_message
finally:
self._progress_end(token)

return
return

# FALLBACK:
# If the client doesn't support progress reporting, or if we failed to
# initialize it, we have a dummy method for the caller to use.
# If the client doesn't support progress reporting, we have a dummy method
# for the caller to use.
def dummy_progress_message(message: str, percentage: Optional[int] = None) -> None:
# pylint: disable=unused-argument
pass
Expand All @@ -179,10 +183,23 @@ def _progress_begin(
title: str,
message: Optional[str] = None,
percentage: Optional[int] = None,
skip_token_initialization: bool = False,
) -> str:
token = str(uuid.uuid4())

self._endpoint.request(self.M_INITIALIZE_PROGRESS, {'token': token}).result(timeout=1.0)
if not skip_token_initialization:
try:
self._endpoint.request(self.M_INITIALIZE_PROGRESS, {'token': token}).result(timeout=1.0)
except Exception: # pylint: disable=broad-exception-caught
log.warning(
"There was an error while trying to initialize progress reporting."
"Likely progress reporting was used in a synchronous LSP handler, "
"which is not supported by progress reporting yet. "
"To prevent waiting for the timeout you can set "
"`skip_token_initialization=True`. "
"Not every editor will show progress then, but many will.",
exc_info=True
)

value = {
"kind": "begin",
Expand Down
13 changes: 9 additions & 4 deletions test/test_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,19 +327,24 @@ def test_progress_simple(workspace, consumer):


@pytest.mark.parametrize("exc", [Exception("something"), TimeoutError()])
def test_progress_initialization_fails(workspace, consumer, endpoint, exc):
def test_progress_initialization_fails_but_is_skipped(workspace, consumer, endpoint, exc):
def failing_token_initialization(self, *_args, **_kwargs):
raise exc
endpoint._dispatcher.m_window__work_done_progress__create = failing_token_initialization

workspace._config.capabilities['window'] = {"workDoneProgress": True}

with workspace.report_progress("some_title"):
with workspace.report_progress("some_title", skip_token_initialization=True):
pass

# we only see the failing token initialization call, no other calls
init_call, = consumer.call_args_list
assert init_call[0][0]['method'] == 'window/workDoneProgress/create'
progress_calls = consumer.call_args_list
assert all(call[0][0]["method"] == "$/progress" for call in progress_calls)
assert len({call[0][0]["params"]["token"] for call in progress_calls}) == 1
assert [call[0][0]["params"]["value"] for call in progress_calls] == [
{"kind": "begin", "title": "some_title"},
{"kind": "end"},
]


def test_progress_with_percent(workspace, consumer):
Expand Down

0 comments on commit fd4a0df

Please sign in to comment.