Fail requests that are searching for a non existing position #2938
Conversation
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
vinistock
added
server
Graphite App
andyw8
approved these changes
Nov 29, 2024
vinistock
force-pushed
the
11-29-fail_requests_that_are_searching_for_a_non_existing_position
branch
from
fe13536 to
57dc835
Compare
Member
Author
Merge activity
|
vinistock
deleted the
11-29-fail_requests_that_are_searching_for_a_non_existing_position
branch
vinistock
added a commit
that referenced
this pull request
Nov 29, 2024
### Motivation While working on #2938, I noticed that our request cancellation response was incorrect. The spec determines that requests that got cancelled by the client need to return an error using the code for request cancelled ([see here](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#cancelRequest) and see `RequestCancelled` [here](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#errorCodes)). ### Implementation Started returning an error with the right code, rather than a response with a `nil` body. I also started running cancel notifications directly in the main thread without pushing it to the queue, which was another mistake. If we put that notification in the queue, then we're guaranteed to only process them **after** we finish running requests, which is useless. We need the ability to process them as soon as we receive the notification, so that we have enough time to cancel the requests. ### Automated Tests Added a test to verify this.
This was referenced Jan 8, 2025
vinistock
added a commit
that referenced
this pull request
Jun 4, 2025
* Fixing a bug where UTF-8 encodings resulted in incorrect char offset calculations. * Revert "Fail requests that are searching for a non existing position (#2938)" This reverts commit 0d421a6. * Calculate character length based on encountered bytes --------- Co-authored-by: Vinicius Stock <vinistock@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation
This is an attempt to handle and better understand #2446
We're seeing the server get stuck sometimes and I suspect that what's happening is the following:
I propose we start raising when we fail to locate a position in the document and then we fail the request. This will hopefully help us better understand what's happening.
Implementation
Started raising if the scanner position is already past the document range, which may happen if we modify the document exactly as we're searching for the position.
Automated Tests
Added tests.