Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make watchdog more resilient against badly behaving clients #4443

Merged
merged 6 commits into from
Jun 13, 2024

Conversation

mhuisi
Copy link
Contributor

@mhuisi mhuisi commented Jun 13, 2024

This PR addresses some non-critical but annoying issues that sometimes cause the language server to report an error:

@mhuisi mhuisi force-pushed the mhuisi/resilient-watchdog branch from b79a9e2 to aec48f4 Compare June 13, 2024 12:19
@Kha Kha requested review from Kha and kim-em as code owners June 13, 2024 12:22
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jun 13, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jun 13, 2024

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase c5120c1d0dd5a23f1432b6432e6d897659ebb63f --onto bedcbfcfeed429428c3e7f008b6984fc8c2b2b76. (2024-06-13 12:35:24)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 16cad2b45c6a77efe4dce850dcdbaafaa7c91fc3 --onto bedcbfcfeed429428c3e7f008b6984fc8c2b2b76. (2024-06-13 13:43:45)

@mhuisi mhuisi added this pull request to the merge queue Jun 13, 2024
Merged via the queue into leanprover:master with commit 3119fd0 Jun 13, 2024
12 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 13, 2024
This PR addresses some non-critical but annoying issues that sometimes
cause the language server to report an error:
- When using global search and replace in VS Code, the language client
sends `textDocument/didChange` notifications for documents that it never
told the server to open first. Instead of emitting an error and crashing
the language server when this occurs, we now instead ignore the
notification. Fixes #4435.
- When terminating the language server, VS Code sometimes still sends
request to the language server even after emitting a `shutdown` request.
The LSP spec explicitly forbids this, but instead of emitting an error
when this occurs, we now error requests and ignore all other messages
until receiving the final `exit` notification. Reported on Zulip several
times over the years but never materialized as an issue, e.g.
https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/Got.20.60shutdown.60.20request.2C.20expected.20an.20.60exit.60.20notification/near/441914289.
- Some language clients attempt to reply to the file watcher
registration request before completing the LSP initialization dance. To
fix this, we now only send this request after the initialization dance
has completed. Fixes #3904.

---------

Co-authored-by: Sebastian Ullrich <[email protected]>
(cherry picked from commit 3119fd0)
Kha pushed a commit that referenced this pull request Jun 13, 2024
…badly behaving clients (#4445)

Backport 3119fd0 from #4443.

Co-authored-by: Marc Huisinga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport releases/v4.9.0 toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

search and replace across files crashes server question: why wait for initialized is needed here?
3 participants