Closed wiseman closed 3 years ago
@wiseman I sent an email to @ebkalderon so we know what his plans are with the repo. Because I don't want to unnecessarily fork and publish a new crate if it is not needed.
I'll give it a few weeks if no further response was received we can work from there.
Check out lspower (active fork)
Sorry for the delayed response, everyone! Only recently looked into this thread and the backlog of work on this repo.
This seems like a pretty serious bug. Does it tie into the deserialization overlap fixed by #244 in any way?
I just double-checked locally, and it appears that #244 does indeed fix this bug. I'm checking to make sure that it doesn't break detection of other invalid messages, particularly those missing "method"
keys.
So, it seems that while #244 fixes the specific serde
error described, it actually breaks some intentional behavior defined by the JSON-RPC 2.0 spec, namely:
-32600
(invalid request) to invalid requests missing a "method"
and/or "id"
, e.g. {"jsonrpc":"2.0","params":{}}
. Instead, we return -32700
(parse error), which is incorrect according to the spec."method"
or other fields, e.g. {"jsonrpc":"2.0"}
. Instead, we return -32700
(parse error).As such, I don't think that this PR is actually viable as a fix. :disappointed:
Thankfully, there may be an alternative solution with judicious use of #[serde(deny_unknown_fields)]
(being careful not to combine it with #[serde(flatten)]
, since that doesn't work). I can open up a PR with the changes in a few moments.
There exists an even simpler, shorter solution that seems to do the trick just as well, without breaking JSON-RPC specification invariants: reverse the order of the Incoming
enum variants. :stuck_out_tongue: This fixes the problem and gets the code in the issue description to work again, but it also preserves the correct JSON-RPC response codes as outlined in the previous comment. See PR https://github.com/ebkalderon/tower-lsp/pull/269 for the solution and an explanation of why this works.
CC @wiseman
Just wanted to give a friendly heads-up about the subtle drawbacks of PR #244, which was previously adopted in lspower
as the solution for this issue. CC @silvanshade
Thanks, @ebkalderon. And I'm glad you're back!
Thanks, @wiseman! Glad to be back. It's been a pretty happening year and a half, to say the least.
@wiseman @ralpha This issue should be fixed in tower-lsp
0.14.0, among other improvements. :tada:
Great work! I'll be updating my repos to the new version soon. Thank you for the quick responses lately and welcome back :)
The following code works in 0.13.3 but hangs in and after 7af7102f83d118ca67e995c8fe6522df7c1acad8:
In the trace I can see the configuration data come back, but the future never resolves.
(Since it's getting close to a year since any changes have been made to this repo, I wonder if maybe someone like @ralpha would be interested in an alt fork to collect updates and fixes in the meantime, e.g. #264.)