LemMinX causes KeyError about lsp HOT 7 CLOSED

Handling server bugs would probably be the last thing we would want to do here. Why not just report it in the server repo?

The unregisterations param is required per spec so it's a spec violation.

from lsp.

I am not aware of spec details as I don't follow them anymore. Just noticed that traceback and asking for feedback. I can't judge it being malformed or not, just a hunch.

That said, changing lin 1982 to

        unregistrations = params.get("unregisterations") or []  # typo in the official specification

would cause the empty params to be ignored silently.

EDIT: I actually not sure which side initiates unregistration. Could also have been a malformed request from LSP -> LemMinX.

from lsp.

Server initiates this

<-- LemMinX client/unregisterCapability (16): {}

My feedback is that it's a spec violation on the server side. It's specified in https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#client_unregisterCapability where the unregisterations property is not optional. It can be set to empty array though although it doesn't really make sense to send it then.

And yes, my point is that we don't want to hide spec violations with changes like the suggested one.

from lsp.

Shouldn't it just return a relevant error code to the server then, instead of raising a local exception?

from lsp.

Potentially yes. But then we'd need to run all the code within a try block which could also hide real issues on our side. If I'd to pick one of those two then I'd rather prefer the current behavior. But maybe there is a third option...

from lsp.

Third option could be to verify the schema of incoming requests/notifications and raise errors in that case.

from lsp.

Yea, basing LSP on pydantic. 😆

from lsp.