Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

Potential improvements to error reporting #27

Merged
merged 2 commits into from
Mar 9, 2022
Merged

Potential improvements to error reporting #27

merged 2 commits into from
Mar 9, 2022

Conversation

AndrewRadev
Copy link
Contributor

I've started experimenting with LSP (I'm new to using and interacting with language servers) and ran into some difficulties understanding the errors. I tried launching the example with cargo run --example goto_def and just copy-pasted some jsonrpc messages and it didn't work out. Here's how an interaction like this might look on master:

% cargo run --example goto_def  
starting generic LSP server
foo
Error: ProtocolError("expected initialize request, got Err(RecvError)")

After doing some digging I can see that the RecvError should have some more information, but the way that it's being printed doesn't show me a lot of it. Here's how the same interaction is rendered in this branch:

% cargo run --example goto_def
starting generic LSP server
foo
Error: ProtocolError("expected initialize request, got error: receiving on an empty and disconnected channel")

So now I can at least guess that the problem doesn't have to do with parsing, but with the way the input is sent, and it would have to be the first thing to fix (like piping the commands by using a file).

I think this is an improvement, but there might be different ways to make it better -- let me know. I'd also consider using unreachable! in cases where a Message::Request is expected, but a Message::Response is given, for example.

@AndrewRadev AndrewRadev changed the title Potential error handling improvements Potential error message improvements Dec 26, 2020
@AndrewRadev AndrewRadev changed the title Potential error message improvements Potential error reporting improvements Dec 26, 2020
@AndrewRadev AndrewRadev changed the title Potential error reporting improvements Potential improvements to error reporting Dec 26, 2020
@AndrewRadev
Copy link
Contributor Author

AndrewRadev commented Dec 26, 2020

Not sure what's up with the build, seems to be failing on an attempt to push a release (with --dry-run):

$ git push --tags --dry-run
remote: Permission to rust-analyzer/lsp-server.git denied to github-actions[bot].
fatal: unable to access 'https://github.com/rust-analyzer/lsp-server/': The requested URL returned error: 403

Copy link
Contributor

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Could you please rebase @AndrewRadev?

@AndrewRadev
Copy link
Contributor Author

@bjorn3 I've rebased my changes on top of master. Wasn't 100% sure if I'd gotten everything right, but it compiles, tests pass and the example I've left in the PR description seems to work as before.

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 9, 2022

Sorry for the long wait! It must have slipped through. I found this PR while going through all open PR's in the rust-analyzer organization.

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 9, 2022

Build succeeded:

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

Successfully merging this pull request may close these issues.

2 participants