From 30073d36d8df58a4df8b5362c21e0a258295b618 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Tue, 21 Jul 2026 09:24:32 +0700 Subject: [PATCH] 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 --- CHANGELOG.md | 7 ++ src/solidlsp/ls_process.py | 52 ++++++++---- test/solidlsp/test_content_modified_retry.py | 85 ++++++++++++++++++++ 3 files changed, 128 insertions(+), 16 deletions(-) create mode 100644 test/solidlsp/test_content_modified_retry.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 83a46a53..63f411ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 "") to be diff --git a/src/solidlsp/ls_process.py b/src/solidlsp/ls_process.py index 9fc84a8e..a7b97cde 100644 --- a/src/solidlsp/ls_process.py +++ b/src/solidlsp/ls_process.py @@ -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: diff --git a/test/solidlsp/test_content_modified_retry.py b/test/solidlsp/test_content_modified_retry.py new file mode 100644 index 00000000..27e05d10 --- /dev/null +++ b/test/solidlsp/test_content_modified_retry.py @@ -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