Don't respond to LSP requests before startup.

[rbartlensky]

A common error found while using emacs + lsp-mode + rust-analyzer is
that on startup I receive the following logs:

LSP :: Error from the Language Server: waiting for cargo metadata or
cargo check (Unknown error) [4 times]

This looks similar to #10910, and as some people mention, everything
works well once cargo metadata finishes running.

I stumbled accross a helpful comment
which made me think that it might be worth waiting for startup to
finish, before attempting to respond to LSP requests.

[petr-tik]

i would really love to see this land

[lnicola]

r? @matklad

[lnicola]

Maybe we could avoid the unwraps here by returning None?

[rbartlensky]

You mean something like:

    fn next_non_lsp_event(&self) -> Option<Event> {
        select! {
            recv(self.task_pool.receiver) -> task =>
                task.ok().map(Event::Task),

            recv(self.loader.receiver) -> task =>
                task.ok().map(Event::Vfs),

            recv(self.flycheck_receiver) -> task =>
                task.ok().map(Event::Flycheck),
        }
    }

?

[lnicola]

Yeah ^

[matklad]

cc #12337 which documents the overal flow here a bit

[matklad]

So this I think we specifically don't want to do: quiescent basically means that a project is fully loaded, and it can take a lot of time to reach that state (it requires running cargo check to compile proc macros).

This is specifically something we don't want to do -- rust-analyzer should be responsive even if the project is not fully loaded.

I'd suggest dowgrading this to

while self.workspaces.is_empty() && self.fetch_workspaces_queue.op_in_progress() {
    let task = self.task_pool.receiver.recv()
}

This way we'll wait only for cargo metadata, not for cargo check. This should also alow removing the // Avoid flashing a bunch of unresolved references during initial load. hack. This I think will be strictly better than what we have today.

But, looking at this holistically, this logic ideally should not exist at all. It's not true that we can't serve any requests witout workspaces . Things like lsp_ext::OnEnter, lsp_ext::MatchingBrace, SelectionRangeRequest, DocumentSymbolRequest should be perfectly possible, and, ideally, even WorkspaceSymbol.

[matklad]

#12340 <- issue for ws symbol

[rbartlensky]

interesting, thank you for your input! I wasn't aware that is_quiescent also waits for cargo check, and I do agree that blocking requests for so long is quite bad.

I tried to change the condition to what you suggested, but I am still getting errors in emacs. If I define a new is_quiescent function which doesn't look at the progress of cargo check, I don't get any errors in emacs (not sure if this is because after waiting for metadata we loop another time, waiting for an event, and enough time passes for other things to get loaded properly)

But, looking at this holistically, this logic ideally should not exist at all. It's not true that we can't serve any requests witout workspaces

This almost feels like it is up to the client to know what requests can be served and which can't at any point in the lifetime of rust-analyzer. Since cargo metadata and cargo check progress is being pushed back to the client (at least by the looks of it from my emacs logs), I guess this could potentially be handled in the client (either by ignoring the errors for requests we know will fail before metadata/check, or by not sending the request if we know rust-analyzer is still doing cargo-check).

If we do handle it on the client side, I worry that we'll end up defining a list of allowed requests that can be done before metadata is loaded, and so on for all startup operations. Then another interesting question is what happens if a request can now be served before cargo check is done, but this wasn't the case before?

I would like to hear your opinions on this. I have very limited experience with LSP clients and servers, so I might not know what the best solution for this would be, but I am willing to dig into it with enough context :)

[matklad]

I tried to change the condition to what you suggested, but I am still getting errors in emacs

hm, did you also remove the “ Avoid flashing” block? If you remove that block, there shouldn’t be anything returning errors I think?

[rbartlensky]

Silly me, I forgot to mention that the error is different:

LSP :: Error from the Language Server: content modified (Unknown error)

I get that even if I remove what you mentioned, since it's a different error.

[rbartlensky]

Closing for now. If I think of another solution to the bug I'll open another PR