Review on #1728 found two blocking issues in the previous commit:
1. pyrefly_server.py wraps send_request with its own 5x/0.2s retry for
RequestCancelled/ContentModified. The base-class retry nested inside
it, so a persistent ContentModified produced 5x5=25 round-trips.
2. The base-class retry applied to every request on every server, but
general.staleRequestSupport.retryOnContentModified in InitializeParams
is the client's contract with the server about exactly which requests
it will reissue. rust-analyzer declares only the three semanticTokens
methods and haskell-language-server declares none; blanket-retrying
e.g. textDocument/hover (or workspace/executeCommand, which is not
idempotent) contradicts that declared capability.
Fix: send_request now only retries a method if it was registered via
the new LanguageServerInterface.set_content_modified_retry_methods,
which SolidLanguageServer._create_initialize_params populates from
each server's own general.staleRequestSupport.retryOnContentModified
list right after building it. The mechanism is opt-in per method, per
server, and mirrors what we actually told the server -- so it's a
no-op wherever nothing is declared.
rust-analyzer now declares textDocument/hover in that list (the method
issue #1724's repro -- test_find_symbol[rust_add_function] with
include_info=True -- actually needs), in addition to the pre-existing
semanticTokens methods.
This also resolves the pyrefly double-retry: pyrefly never declares
general.staleRequestSupport, so the base-class retry is a no-op for
every pyrefly request and pyrefly's own wrapper remains the sole
handler for both RequestCancelled and ContentModified there, exactly
as before this PR. pyrefly_server.py is intentionally left untouched;
narrowing its retry tuple, as literally suggested, would have removed
ContentModified handling for it entirely once the base layer stopped
covering it too.
Also: reduced _CONTENT_MODIFIED_MAX_ATTEMPTS from 5 to 3 (now the max
per allowlisted method only, so worst-case added latency for hover is
2 * request_timeout + 0.4s instead of 4 * request_timeout + 0.8s), and
restructured send_request around a _send_request_once helper so the
final failed attempt raises directly instead of falling through to a
trailing `raise AssertionError("unreachable")`. Retry tests now
monkeypatch time.sleep and gained coverage for the allowlist gate
(undeclared method, executeCommand, default-empty registration).
Fixes#1724
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
Dominik Jain