mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-14 00:16:57 +00:00
Retry LSP ContentModified (-32801) errors instead of failing hard
Per the LSP spec, ContentModified means the server discarded a stale, in-flight computation because the workspace changed underneath it, not that the request itself is invalid, so a well-behaved client is expected to retry. LanguageServerInterface.send_request previously converted any such response straight into a hard SolidLSPException, with no retry, for every language server. This was flaking test_find_symbol[rust_add_function] on windows-latest: rust-analyzer returns ContentModified for hover requests cancelled by concurrent indexing (its internal retry set does not cover hover), and nothing on the client side retried it. send_request now retries ContentModified responses a bounded number of times with a short delay before giving up. This is generic to the base request path, so it applies to every language server, not just rust-analyzer. RequestCancelled (-32800) is left alone since the LSP spec defines it as client-initiated. Fixes #1724
This commit is contained in:
1 parent
9383bf788c
commit
30073d36d8
3 files changed
+128
-16
No files matched your search
@@ -51,6 +51,13 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
- PHP/Intelephense: expose `file_filter` via `ls_specific_settings["php"]`, so that additional
|
||||
extensions containing PHP sources (e.g. Drupal's `.module` / `.install` / `.inc` / `.theme`)
|
||||
become visible to the symbol tools and are indexed by the language server #1710
|
||||
- Fix: an LSP `ContentModified` (-32801) response was surfaced as a hard `SolidLSPException`
|
||||
instead of being retried. Per the LSP spec, `ContentModified` means the server discarded a
|
||||
stale, in-flight computation because the workspace changed underneath it, not that the request
|
||||
itself is invalid; well-behaved clients are expected to retry. `LanguageServerInterface.send_request`
|
||||
now retries such requests a bounded number of times before giving up, for all language servers
|
||||
(this was flaking `test_find_symbol[rust_add_function]` on windows-latest, where rust-analyzer
|
||||
returns `ContentModified` for hover requests cancelled by concurrent indexing). #1724
|
||||
|
||||
* JetBrains:
|
||||
- Allow external files from dependencies (specified via references like "<ext:FileUtil.class|472e0a13>") to be
|
||||
|
||||
+36
-16
@@ -18,7 +18,7 @@ from solidlsp.ls_config import Language
|
||||
from solidlsp.ls_exceptions import SolidLSPException
|
||||
from solidlsp.ls_request import LanguageServerRequest
|
||||
from solidlsp.lsp_protocol_handler.lsp_requests import LspNotification
|
||||
from solidlsp.lsp_protocol_handler.lsp_types import ErrorCodes
|
||||
from solidlsp.lsp_protocol_handler.lsp_types import ErrorCodes, LSPErrorCodes
|
||||
from solidlsp.lsp_protocol_handler.server import (
|
||||
ENCODING,
|
||||
LSPError,
|
||||
@@ -36,6 +36,13 @@ from solidlsp.util import subprocess_util
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
# Per the LSP spec, `ContentModified` (-32801) means the server discarded a stale, in-flight
|
||||
# computation because the workspace changed underneath it, not that the request itself is
|
||||
# invalid. A well-behaved client is expected to retry such requests rather than treat them as
|
||||
# a hard failure, so we do so here, generically, for all requests to any language server.
|
||||
_CONTENT_MODIFIED_MAX_ATTEMPTS = 5
|
||||
_CONTENT_MODIFIED_RETRY_DELAY = 0.2
|
||||
|
||||
|
||||
class LanguageServerTerminatedException(Exception):
|
||||
"""
|
||||
@@ -298,29 +305,42 @@ class LanguageServerInterface(ABC):
|
||||
|
||||
def send_request(self, method: str, params: dict | None = None) -> PayloadLike:
|
||||
"""
|
||||
Send request to the server, register the request id, and wait for the response
|
||||
Send request to the server, register the request id, and wait for the response.
|
||||
|
||||
Requests that fail with the LSP `ContentModified` error (-32801) are retried a bounded
|
||||
number of times with a short delay instead of being surfaced to the caller immediately;
|
||||
see `_CONTENT_MODIFIED_MAX_ATTEMPTS` above.
|
||||
"""
|
||||
with self._request_id_lock:
|
||||
request_id = self.request_id
|
||||
self.request_id += 1
|
||||
for attempt in range(1, _CONTENT_MODIFIED_MAX_ATTEMPTS + 1):
|
||||
with self._request_id_lock:
|
||||
request_id = self.request_id
|
||||
self.request_id += 1
|
||||
|
||||
request = Request(request_id=request_id, method=method)
|
||||
log.debug("Starting: %s", request)
|
||||
request = Request(request_id=request_id, method=method)
|
||||
log.debug("Starting: %s", request)
|
||||
|
||||
with self._response_handlers_lock:
|
||||
self._pending_requests[request_id] = request
|
||||
with self._response_handlers_lock:
|
||||
self._pending_requests[request_id] = request
|
||||
|
||||
self._send_payload(make_request(method, request_id, params))
|
||||
self._send_payload(make_request(method, request_id, params))
|
||||
|
||||
log.debug("Waiting for response to request %s with params:\n%s", method, params)
|
||||
result = request.get_result(timeout=self._request_timeout)
|
||||
log.debug("Completed: %s", request)
|
||||
log.debug("Waiting for response to request %s with params:\n%s", method, params)
|
||||
result = request.get_result(timeout=self._request_timeout)
|
||||
log.debug("Completed: %s", request)
|
||||
|
||||
if not result.is_error():
|
||||
log.debug("Returning result:\n%s", result.payload)
|
||||
return result.payload
|
||||
|
||||
is_content_modified = isinstance(result.error, LSPError) and result.error.code == LSPErrorCodes.ContentModified
|
||||
if attempt < _CONTENT_MODIFIED_MAX_ATTEMPTS and is_content_modified:
|
||||
log.info("Request %s got ContentModified (-32801); retrying (%d/%d)", method, attempt, _CONTENT_MODIFIED_MAX_ATTEMPTS)
|
||||
time.sleep(_CONTENT_MODIFIED_RETRY_DELAY)
|
||||
continue
|
||||
|
||||
if result.is_error():
|
||||
raise SolidLSPException(f"Error processing request {method} with params:\n{params}", cause=result.error) from result.error
|
||||
|
||||
log.debug("Returning result:\n%s", result.payload)
|
||||
return result.payload
|
||||
raise AssertionError("unreachable") # loop always returns or raises
|
||||
|
||||
@abstractmethod
|
||||
def _send_payload(self, payload: StringDict) -> None:
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
"""Unit tests: ``LanguageServerInterface.send_request`` retries LSP ``ContentModified`` errors.
|
||||
|
||||
Per the LSP spec, ``ContentModified`` (-32801) means the server discarded a stale, in-flight
|
||||
computation because the workspace changed underneath it, not that the request itself is
|
||||
invalid -- clients are expected to retry. This is what caused issue #1724 (flaky
|
||||
``test_find_symbol[rust_add_function]`` on windows-latest): rust-analyzer returns
|
||||
``ContentModified`` for cancelled hover requests, and Serena surfaced it as a hard error.
|
||||
|
||||
No language markers: these use a local test double and run in catch-all.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
|
||||
import pytest
|
||||
|
||||
from solidlsp import ls_process
|
||||
from solidlsp.ls_config import Language
|
||||
from solidlsp.ls_exceptions import SolidLSPException
|
||||
from solidlsp.ls_process import LanguageServerInterface, Request
|
||||
from solidlsp.lsp_protocol_handler.lsp_types import LSPErrorCodes
|
||||
from solidlsp.lsp_protocol_handler.server import LSPError
|
||||
|
||||
|
||||
class _ScriptedServer(LanguageServerInterface):
|
||||
"""Test double that answers each request synchronously from a scripted list of results,
|
||||
without a real language server process.
|
||||
"""
|
||||
|
||||
def __init__(self, results: list[Request.Result]) -> None:
|
||||
super().__init__(Language.PYTHON, lambda _line: logging.INFO)
|
||||
self._results = list(results)
|
||||
self.sent_payload_count = 0
|
||||
|
||||
def is_running(self) -> bool:
|
||||
return True
|
||||
|
||||
def _start(self) -> None:
|
||||
pass
|
||||
|
||||
def _stop(self, timeout: float) -> None:
|
||||
pass
|
||||
|
||||
def _send_payload(self, payload: dict) -> None:
|
||||
self.sent_payload_count += 1
|
||||
request = self._pending_requests[payload["id"]]
|
||||
result = self._results.pop(0)
|
||||
if result.is_error():
|
||||
request.on_error(result.error)
|
||||
else:
|
||||
request.on_result(result.payload)
|
||||
|
||||
|
||||
def _content_modified() -> Request.Result:
|
||||
return Request.Result(error=LSPError(LSPErrorCodes.ContentModified, "content modified"))
|
||||
|
||||
|
||||
def test_content_modified_is_retried_until_success() -> None:
|
||||
server = _ScriptedServer([_content_modified(), _content_modified(), Request.Result(payload={"contents": "ok"})])
|
||||
assert server.send_request("textDocument/hover") == {"contents": "ok"}
|
||||
assert server.sent_payload_count == 3
|
||||
|
||||
|
||||
def test_content_modified_gives_up_after_max_attempts() -> None:
|
||||
max_attempts = ls_process._CONTENT_MODIFIED_MAX_ATTEMPTS
|
||||
server = _ScriptedServer([_content_modified() for _ in range(max_attempts)])
|
||||
with pytest.raises(SolidLSPException):
|
||||
server.send_request("textDocument/hover")
|
||||
assert server.sent_payload_count == max_attempts
|
||||
|
||||
|
||||
def test_other_lsp_errors_are_not_retried() -> None:
|
||||
server = _ScriptedServer([Request.Result(error=LSPError(LSPErrorCodes.RequestFailed, "boom"))])
|
||||
with pytest.raises(SolidLSPException):
|
||||
server.send_request("textDocument/hover")
|
||||
assert server.sent_payload_count == 1
|
||||
|
||||
|
||||
def test_request_cancelled_is_not_retried() -> None:
|
||||
"""-32800 is client-initiated cancellation per the LSP spec, so it must not be retried here."""
|
||||
server = _ScriptedServer([Request.Result(error=LSPError(LSPErrorCodes.RequestCancelled, "cancelled"))])
|
||||
with pytest.raises(SolidLSPException):
|
||||
server.send_request("textDocument/hover")
|
||||
assert server.sent_payload_count == 1
|
||||
Reference in new issue
Block a user