-
Notifications
You must be signed in to change notification settings - Fork 272
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
Regression: "data did not match any variant of untagged enum RawMessage" #892
Comments
It seems like the fix would imply this project would be stuck with a particular version of jsonrpc indefinitely. Is there any other way Sorbet can pass on this information? |
The serialization/deserialization library that Sorbet is using is a C++ library that makes it hard to work with untagged unions. Sorbet adds the Notably, VS Code ignores extra fields just fine. I can't find anywhere in the spec where it says one way or another whether extra fields are allowed or not. I'm not super familiar with the library that LanguageClient-neovim is using, but is there an option to have it ignore extra fields when it's attempting to deserialize? In general I've found that it's a nice feature to have (it means that adding a field to the API is not a backwards incompatible change) so I wouldn't be surprised if the Rust library that LanguageClient-neovim uses has a config option for that somewhere. Ultimately, it's up to you what you'd like to do. Sorbet only officially supports VS Code (for exactly reasons like this—that each LSP client is different in it's own way) so the Sorbet team isn't directly affected. |
It looks like the most recent upgrade to jsonrpc-core 15 by @martskins in #1099 doesn't have this problem. |
To be honest I'm a little surprised that it fixes it! I wasn't aware of this issue and updated that library but that version seems to include We can roll back that dependency upgrade like we did before though. But if it's confirmed that it's working even better. EDIT: In any case, I'll try to investigate whether additional fields are supported by the protocol or not, but in the meantime, let me know if this breaks again for you. |
I spoke too soon; I had not rebuilt |
@jez If removing that field from the payload is difficult from your side I suppose we could remove the field from the payload ourselves. We already have something similar for a javascript language server. This one is a little more expensive because we'd have to use a regex, but it could be done. And sorry for breaking that, wasn't aware of the issue. |
I think this should solve this issue and allow us to upgrade: #1115 |
For what it's worth, I'm experimenting with reworking this in Sorbet so that it's less trouble for LanguageClient-neovim https://github.com/sorbet/sorbet/compare/jez-undiscriminated-union I still think that the fact that VS Code allows the field means that we should allow it too. But it seems that jsonrpc-core upstream did not want to maintain "non-strict" parsing of JSON RPC messages: ^ this is the commit introduced after version 12 that caused the problem noted in the original report. |
Yeah unfortunately that part of the protocol is open to interpretation. Removing that regex would be nice, but at least it's working as expected now. 👍 |
:checkhealth LanguageClient
?Describe the bug
I had been using an older (stable) version, but wanted to upgrade to get floating window support. Upgrading made LanguageClient-neovim fail to initialize specifically when working with Sorbet.
I bisected the bug (using
make release
andgit bisect
locally) to isolate the issue to this commit to LanguageClient-neovim: 815e44a which bumpsjsonrpc-core
from version 12 to 13.Environment
nvim --version
orvim --version
): NVIM v0.4.2git rev-parse --short HEAD
): 4fd272bTo Reproduce
Steps to reproduce the behavior:
vimrc:
project:
Current behavior
/tmp/languageclient-neovim.log
/tmp/sorbet-nvim.log
Heads up I'm a Sorbet developer. If you need any help reading this log I'm happy to explain.
The important lines are the
Read:
andWrite:
lines. Those are lines that the language server process has read from stdin and written to stdout.Diagnosis
The Sorbet language server sends an additional field in it's
initialize
response message:requestMethod
. This is not mentioned in the LSP spec; it's an implementation detail that makes it easier for us to generate typed C++ bindings to the spec.I bisected the issue back to this commit: 815e44a
If I manually revert that commit on master (i.e., downgrade the version of jsonrpc-core from 13 back to 12), everything works as expected.
The text was updated successfully, but these errors were encountered: