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

Return proper error code when server is loading #8397

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

ceronman
Copy link

@ceronman ceronman commented Apr 7, 2021

When requests are made to rust-analyzer and the server is still loading, a response error is returned with the code ContentModified and text "Rust Analyzer is still loading...". This error code doesn't seem to be the more appropriate for this situation. Using ServerNotInitialized seems better.

As this is such a small change, I have not created an issue for it.

@matklad
Copy link
Member

matklad commented Apr 7, 2021

Unsure about this. The spec doesn't specify when ServerNotInitialized should be emmited at all. I suspect it is for the cases where (an erroneous) client sends requests before initialization sequence completed.

ContentModified is used according to it's intended cases here: sereve is initialized, but it still constantly modifies it's internal model of the world, as fileds are still being rear from the disk.

Why does this matter at all? The clients should just silently ignore content modified errors.

@ceronman
Copy link
Author

ceronman commented Apr 7, 2021

@matklad This doesn't matter much. I was just surprised by the message and thought that ServerNotInitialized was more suitable. But what you say makes sense. Feel free to close the PR.

@matklad
Copy link
Member

matklad commented Apr 7, 2021

Let's just change the message to "waiting for cargo metadata or cargo check" while keeping the error code.

@ceronman
Copy link
Author

ceronman commented Apr 7, 2021

@matklad I updated the PR with your suggestion just in case. Feel free to merge or close.

@matklad
Copy link
Member

matklad commented Apr 7, 2021

Thanks!

bors r+

@matklad
Copy link
Member

matklad commented Apr 7, 2021

In case your are curious, microsoft/language-server-protocol#511 is the related protocol issue z

@bors
Copy link
Contributor

bors bot commented Apr 7, 2021

@lnicola
Copy link
Member

lnicola commented Apr 8, 2021

changelog fix (first contribution) improve "still loading" error message

@kjeremy
Copy link
Contributor

kjeremy commented Apr 9, 2021

See rust-analyzer/lsp-server#29 for clarification of ServerNotInitialized

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants