feat(lsp): allow per-server diagnostics wait timeouts - #41219
Open
Yagi-Michael wants to merge 1 commit into
Open
feat(lsp): allow per-server diagnostics wait timeouts#41219Yagi-Michael wants to merge 1 commit into
Yagi-Michael wants to merge 1 commit into
Conversation
clangd needs roughly 20 seconds to parse an Unreal Engine translation unit
(~3,700 transitive headers). The diagnostics waits are hardcoded at 5s for
document and 10s for full, so the client always gives up first and reports
no diagnostics -- which is indistinguishable from a clean file. Agents
consuming the LSP therefore assert false confidence, or stop trusting it.
Add an optional per-server `timeout: { document, full }` to the LSP config,
defaulting to the existing constants so nothing changes for anyone who does
not opt in.
Note the field must be declared in BOTH the v2 schema and the v1 schema: a
v1 config is decoded against v1's Entry before migrate.ts passes `lsp`
through untouched, so a field present only in v2 is silently stripped and
never reaches the client. Patching v2 alone looks correct and does nothing.
Measured on a 23,382-entry Unreal compile database:
before: {".clangd": []} in 11s -- requested file absent entirely
after: requested file present with the real clang error, in 13s
default (no timeout configured): unchanged, still 12s and empty
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #41218
Type of change
What does this PR do?
DIAGNOSTICS_DOCUMENT_WAIT_TIMEOUT_MS(5s) andDIAGNOSTICS_FULL_WAIT_TIMEOUT_MS(10s) inlsp/client.tsare hardcoded. If a language server takes longer than that to produce diagnostics, the wait expires and the client returns nothing — which callers cannot distinguish from "the file is clean". That's the bug: silence and success look identical.I hit this with clangd on an Unreal Engine project. One translation unit pulls in ~3,764 transitive headers and clangd needs about 20s to parse it, so the wait always expired and
debug lsp diagnosticsreturned only the.clangdconfig file, never the file I asked about. It was not a misconfiguration — the same file compiles cleanly withclang++ -fsyntax-onlyusing the flags fromcompile_commands.json, and a deliberately broken copy reports its error correctly.This adds an optional per-server
timeout: { document, full }, falling back to the existing constants when unset:Why it works:
create()inclient.tsalready receivesinput.server, andspawn()inlsp.tsalready builds that object from the user's config, so the value just needs carrying through and reading at the six places the two constants are used.One thing worth flagging for anyone adding LSP config fields later: the field has to be declared in both
config/lsp.tsandv1/config/lsp.ts. A v1 config is decoded against v1'sEntrybeforemigrate.tspasseslspthrough untouched, so a field present only in v2 gets silently stripped and never reaches the client. I patched only v2 first and the option appeared to do nothing.Five files, +35/-6. I deliberately left
DIAGNOSTICS_REQUEST_TIMEOUT_MSandINITIALIZE_TIMEOUT_MSalone — the latter is #23982 and belongs in its own change.How did you verify your code works?
Ran the same command against the same file and the same clangd, changing only the config.
Before, and after with no
timeoutset (confirming defaults are unchanged):~11s, requested file absent entirely.
After, with
timeoutconfigured, on a file containing a deliberate error:~13s.
Also checked in a real session that the automatic post-edit path works: writing a broken
.cppnow attachesError [3:13] Use of undeclared identifier 'this_symbol_does_not_exist'to the write tool result, where before it attached nothing.bun typecheckpasses (30/30 packages) — the pre-push hook ran it.Screenshots / recordings
Not a UI change.
Checklist