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

Show the error message from the LSP server #1490

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DavyLandman
Copy link

I've noticed that if a LSP server reports an error via ResponseError::message field, it's a bit of a hit or miss if this is reported to the end user in a popup.

For example rename reports the error in the dialog while errors in many others parts end up in the generic handler

I've changed this such that both the message from the library is kept, while also reporting the message from the LSP server.

@dbaeumer
Copy link
Member

dbaeumer commented Jun 4, 2024

Thanks for the PR. Should we use the result from data2String which is captured here: https://github.com/microsoft/vscode-languageserver-node/blob/main/client/src/common/client.ts#L1004

That covers more cases than simply an Error with an message.

@DavyLandman
Copy link
Author

Thanks for the PR. Should we use the result from data2String which is captured here: https://github.com/microsoft/vscode-languageserver-node/blob/main/client/src/common/client.ts#L1004

That covers more cases than simply an Error with an message.

I was thinking about that as well, but that might show a bit more than needed (as it also prints the jsonRPC/lsp error code)?

But if you prefer, I rewrite it to that.

@DavyLandman
Copy link
Author

Having reviewed the function again, yes I agree, it's better to reuse that function. The PR has been updated.

@DavyLandman
Copy link
Author

hi @dbaeumer is there something I can do to help get this merged? It would be a pitty if it misses an upcoming release window, as this helps us better report to users when something crashed, what is the issue, without having them dig through the output window.

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.

None yet

2 participants