zed icon indicating copy to clipboard operation
zed copied to clipboard

Propagate errors from rust-analyzer on invalid Cargo.toml contents

Open napalmpapalam opened this issue 2 years ago • 15 comments

Check for existing issues

  • [X] Completed

Describe the bug / provide steps to reproduce it

I was working with two opened projects written in Rust. One is the lib from which the other project depends via the local path, and in one moment rust-analyzer stopped working. No analyzing, go to definition, etc.

Environment

Zed: v0.122.1 (Zed Preview) OS: macOS 14.1.1 Memory: 16 GiB Architecture: x86_64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

2024-02-08T17:19:33+02:00 [INFO] ========== starting zed ==========
2024-02-08T17:19:33+02:00 [INFO] Opening main db
2024-02-08T17:19:33+02:00 [ERROR] crates/theme/src/settings.rs:326: theme not found: Github x VSCode theme
2024-02-08T17:19:33+02:00 [INFO] Opening main db
2024-02-08T17:19:34+02:00 [INFO] set environment variables from shell:/bin/zsh, path:/usr/local/opt/libpq/bin:/Users/napalmpapalam/.nvm/versions/node/v18.15.0/bin:/Users/napalmpapalam/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/Applications/Sublime Text.app/Contents/SharedSupport/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Users/napalmpapalam/.cargo/bin://bin://Users/napalmpapalam/go/bin
2024-02-08T17:19:34+02:00 [INFO] set status on client 0: Authenticating
2024-02-08T17:19:34+02:00 [INFO] Opening main db
2024-02-08T17:19:34+02:00 [INFO] set status on client 105593: Connecting
2024-02-08T17:19:34+02:00 [INFO] Node runtime install_if_needed
2024-02-08T17:19:35+02:00 [INFO] starting language server "rust-analyzer", path: "/Users/napalmpapalam/rust/m10/banking-demo/shared/rust", id: 1
2024-02-08T17:19:35+02:00 [INFO] connected to rpc endpoint https://collab.zed.dev/rpc
2024-02-08T17:19:35+02:00 [INFO] add connection to peer
2024-02-08T17:19:35+02:00 [INFO] waiting for server hello
2024-02-08T17:19:35+02:00 [INFO] got server hello
2024-02-08T17:19:35+02:00 [INFO] set status to connected (connection id: ConnectionId { owner_id: 0, id: 0 }, peer id: PeerId { owner_id: 328, id: 203153 })
2024-02-08T17:19:35+02:00 [INFO] set status on client 105593: Connected { peer_id: PeerId { owner_id: 328, id: 203153 }, connection_id: ConnectionId { owner_id: 0, id: 0 } }
2024-02-08T17:19:35+02:00 [INFO] 1 unhandled notification experimental/serverStatus:
{
  "health": "ok",
  "quiescent": false,
  "message": null
}
2024-02-08T17:19:35+02:00 [INFO] 0 unhandled notification LogMessage:
{
  "level": 0,
  "message": "[DEBUG] [agent] [2024-02-08T15:19:35.875Z] Agent service starting",
  "metadataStr": "[DEBUG] [agent] [2024-02-08T15:19:35.875Z]",
  "extra": [
    "Agent service starting"
  ]
}
2024-02-08T17:19:35+02:00 [INFO] 0 unhandled notification client/registerCapability:
{
  "registrations": [
    {
      "id": "36d6893b-b84b-4857-960f-2c3274a40924",
      "method": "workspace/didChangeWorkspaceFolders",
      "registerOptions": {}
    }
  ]
}
2024-02-08T17:19:36+02:00 [INFO] 0 unhandled notification LogMessage:
{
  "level": 0,
  "message": "[DEBUG] [agent] [2024-02-08T15:19:36.372Z] Telemetry initialized",
  "metadataStr": "[DEBUG] [agent] [2024-02-08T15:19:36.372Z]",
  "extra": [
    "Telemetry initialized"
  ]
}
2024-02-08T17:19:37+02:00 [INFO] 1 unhandled notification experimental/serverStatus:
{
  "health": "error",
  "quiescent": false,
  "message": "Failed to load workspaces."
}
2024-02-08T17:19:39+02:00 [INFO] 1 unhandled notification experimental/serverStatus:
{
  "health": "error",
  "quiescent": true,
  "message": "Failed to load workspaces."
}

napalmpapalam avatar Feb 08 '24 15:02 napalmpapalam

Oh sorry, found an issue#6365 where a guy said that solved it with the cargo check, the same worked for me:

error: no matching package found
searched package name: `futures_core`
perhaps you meant:      futures-core

but is there a way to notify the user that something is wrong with the Cargo.toml or with the Rust LSP, coz this is a little bit not obvious?

For example, Golang with the Rust Plugin highlights such problems in the Cargo.toml:

image

napalmpapalam avatar Feb 08 '24 15:02 napalmpapalam

Great that you've found the root cause of an issue.

Zed by itself knows very little about the languages: it has a separate highlighting capabilities, but no way it's able to understand any semantics related to any language: whether a certain file belongs to Rust language and neither it knows what a "crate" is and where to fetch more info about that.

GoLand is its own world with Rust and Toml parsers tightly integrated with their IDEs, but Zed (and VSCode) can only rely on what rust-analyzer language server can provide to them.

I think we can improve it on Zed's side by handling the

2024-02-08T17:19:39+02:00 [INFO] 1 unhandled notification experimental/serverStatus:
{
  "health": "error",
  "quiescent": true,
  "message": "Failed to load workspaces."
}

LSP notification, and it's currently the only thing that rust-analyzer is able to send back on such errors. Here's what VSCode is able to show in the logs:

[ERROR rust_analyzer::main_loop] FetchWorkspaceError:
rust-analyzer failed to load workspace: Failed to load the project at /Users/someonetoignore/work/other/local_test/Cargo.toml: Failed to read Cargo metadata from Cargo.toml file /Users/someonetoignore/work/other/local_test/Cargo.toml, Some(Version { major: 1, minor: 75, patch: 0 }): Failed to run `cd "/Users/someonetoignore/work/other/local_test" && "cargo" "metadata" "--format-version" "1" "--manifest-path" "/Users/someonetoignore/work/other/local_test/Cargo.toml" "--filter-platform" "aarch64-apple-darwin"`: `cargo metadata` exited with an error:     Updating crates.io index
error: no matching package named `tracaaaaaaaaaaaaaaaaaaaing` found
location searched: registry `crates-io`
required by package `local_test v0.1.0 (/Users/someonetoignore/work/other/local_test)`

hence Zed should at least show that in the logs too (or, better, in a notification pop-up)

SomeoneToIgnore avatar Feb 08 '24 15:02 SomeoneToIgnore

Great that you've found the root cause of an issue.

Zed by itself knows very little about the languages: it has a separate highlighting capabilities, but no way it's able to understand any semantics related to any language: whether a certain file belongs to Rust language and neither it knows what a "crate" is and where to fetch more info about that.

GoLand is its own world with Rust and Toml parsers tightly integrated with their IDEs, but Zed (and VSCode) can only rely on what rust-analyzer language server can provide to them.

I think we can improve it on Zed's side by handling the

2024-02-08T17:19:39+02:00 [INFO] 1 unhandled notification experimental/serverStatus:
{
  "health": "error",
  "quiescent": true,
  "message": "Failed to load workspaces."
}

LSP notification, and it's currently the only thing that rust-analyzer is able to send back on such errors. Here's what VSCode is able to show in the logs:

[ERROR rust_analyzer::main_loop] FetchWorkspaceError:
rust-analyzer failed to load workspace: Failed to load the project at /Users/someonetoignore/work/other/local_test/Cargo.toml: Failed to read Cargo metadata from Cargo.toml file /Users/someonetoignore/work/other/local_test/Cargo.toml, Some(Version { major: 1, minor: 75, patch: 0 }): Failed to run `cd "/Users/someonetoignore/work/other/local_test" && "cargo" "metadata" "--format-version" "1" "--manifest-path" "/Users/someonetoignore/work/other/local_test/Cargo.toml" "--filter-platform" "aarch64-apple-darwin"`: `cargo metadata` exited with an error:     Updating crates.io index
error: no matching package named `tracaaaaaaaaaaaaaaaaaaaing` found
location searched: registry `crates-io`
required by package `local_test v0.1.0 (/Users/someonetoignore/work/other/local_test)`

hence Zed should at least show that in the logs too (or, better, in a notification pop-up)

Yeah I understood that Zed works with languages via LSP, but yeah +1 for the notification pop-up

napalmpapalam avatar Feb 08 '24 16:02 napalmpapalam

Hi! I can handle this issue. May I get some guidance as to how to get the output of the rust analyzer? Thank you!

zephaniahong avatar Feb 15 '24 01:02 zephaniahong

There's no direct way to handle the r-a output, but you don't seem to need it? Here https://github.com/zed-industries/zed/blob/d311e2fba87ab2dced06a2707347c1104e2f7b24/crates/project/src/project.rs#L3183-L3184 any language server is set up, and we need to add another on_nofication for the

2024-02-08T17:19:39+02:00 [INFO] 1 unhandled notification experimental/serverStatus:
{
  "health": "error",
  "quiescent": true,
  "message": "Failed to load workspaces."
}

ones.

That might require defining another struct for that notification + impl Notification for FOO, similar to lsp::notification::Progress, but I think that should be already present in the lsp package.

In addition to that, I would check Zed logs (env RUST_LOG=info if starting from the terminal, regular log file otherwise) for more unhandled notifications, related to that experimental/ case, maybe there's more info rust-analyzer sends us. Examining language server logs panel in Zed for r-a, the RPC messages tab might help with that too image

It seems that we might not get a very descriptive message as VSCode does (if there's just that Failed to load workspaces. notification only), but that seems enough to indicate that something is wrong. For starters, I'd just log that thing depending on the health level, but there's a way to spawn pop-up notifications in Zed too, let's return there when we know how many messages there are related to this issue: outputting any experimental/serverStatus message as a pop-up might be too much.

SomeoneToIgnore avatar Feb 15 '24 12:02 SomeoneToIgnore

@SomeoneToIgnore Wow! Thanks so much for your help! I'll experiment with whatever you've mentioned and come back again when I'm ready for the message pop-up.

zephaniahong avatar Feb 15 '24 13:02 zephaniahong

@SomeoneToIgnore Hi! After searching around, I think that the types for the experimental/ cases are not in the lsp package. Could I get some suggestions as to how to approach this? Thank you!

zephaniahong avatar Feb 23 '24 13:02 zephaniahong

First, we have to understand whether those expect any response in return: if they do, those are Requests, Notifications otherwise.

I think it falls under the Notification case, but even if it is not, it will work nonetheless if we implement a notification for request: we just would not be able to respond to that interim which looks fine.

Then, you have to create a new struct somewhere in Zed and implement that trait for that struct. It should be simple, since only requires a request path (experimental/..) and a struct to deserialize the parameters into.

SomeoneToIgnore avatar Feb 23 '24 13:02 SomeoneToIgnore

@SomeoneToIgnore Great! I've managed to receive the Notification whenever an experimental/ case happens. Shall I proceed with adding a message pop up? If so, which UI element should I use? Thanks!(:

zephaniahong avatar Feb 23 '24 13:02 zephaniahong

I am a bit worried about such pop-ups in general, can you paste all messages you get, related to that error?

Last thing I want is to irritate people with smth. like this: https://github.com/zed-industries/zed/issues/8229

We have a less intrusive Zed pop-up, e.g. the one showing the release notes, we can consider that if the messages are not abundant.

SomeoneToIgnore avatar Feb 23 '24 14:02 SomeoneToIgnore

If the file has no errors and you save the file, this is what you get: Screenshot 2024-02-23 at 10 42 07 PM

However, if you have some sort of error in the file then you might get something like this: Screenshot 2024-02-23 at 10 41 32 PM

Yeah I was definitely thinking of something similar to the release notes. The messages seem to repeat when quiescent goes from false to true. To reduce the number of repeated pop ups, we could just display a pop-up only when there is an error and quiescent is true.

If you do decide to proceed with the less intrusive pop up, could you point me to the file? Thank you!

zephaniahong avatar Feb 23 '24 14:02 zephaniahong

Thank you for exploring this, I think next step should be a PR. I think, the version-like is good enough for errors only, we'll give at spin in nightly and then Preview and see how much of an issue it gets.

Those pop-ups are a bit convoluted and are called *Notification, you can try to use https://github.com/zed-industries/zed/blob/c90065e6ea5506cddfd69fef954ae4d963830393/crates/notifications/src/notification_store.rs#L41 and see if it works for you, or try to reuse the code of UpdateNotification.

SomeoneToIgnore avatar Feb 23 '24 15:02 SomeoneToIgnore

I also think we would want to keep on logging those messages, but do it more clever and actually change a lot level based on the health of it — log::warn for warning, log:error for error, log::info for the rest.

SomeoneToIgnore avatar Feb 23 '24 15:02 SomeoneToIgnore

Conveniently, LanguageServerPrompt is about to land in main and I think this is what we should use. https://github.com/zed-industries/zed/pull/8312/

SomeoneToIgnore avatar Feb 23 '24 22:02 SomeoneToIgnore

Oh perfect! I'll use it once it lands in main. Just to clarify, I'll log all messages (but in a smarter way) as well as display errors using the LanguageServerPrompt.

zephaniahong avatar Feb 23 '24 23:02 zephaniahong

https://github.com/zed-industries/zed/pull/8356 propagates whatever rust-analyzer sends us back in experimental/serverStatus notification, now we get Failed to load workspace mainly and potentially some other errors as pop-ups.

SomeoneToIgnore avatar Mar 02 '24 08:03 SomeoneToIgnore