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

JSON-RPC request without params violates specification with null #655

Open
sim642 opened this issue Aug 5, 2022 · 3 comments
Open

JSON-RPC request without params violates specification with null #655

sim642 opened this issue Aug 5, 2022 · 3 comments

Comments

@sim642
Copy link

sim642 commented Aug 5, 2022

When a JSON-RPC request without params (i.e. them being null in Java) is emitted, that gets written as "params": null instead of just omitting the params field from the output. The latter should be done instead.

Specifications

JSON-RPC specification says:

params
A Structured value that holds the parameter values to be used during the invocation of the method. This member MAY be omitted.

where "two structured types (Objects and Arrays)".

LSP specification says (in TypeScript syntax):

/**
 * The method's params.
 */
params?: array | object;

which exactly matches the JSON-RPC specification with the ? meaning omission, but not null (which would have to be explicitly given as a | null variant).

Implementation

The writing of params here:
https://github.com/eclipse/lsp4j/blob/22f8edce55379bd9d4ae1b96bc614e74549bc0f9/org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/json/adapters/MessageTypeAdapter.java#L404-L409
uses the writeNullValue function here:
https://github.com/eclipse/lsp4j/blob/22f8edce55379bd9d4ae1b96bc614e74549bc0f9/org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/json/adapters/MessageTypeAdapter.java#L449-L457
which goes out of its way to output a null instead of omitting missing params.

This clearly violates the specifications and trips up other JSON-RPC/LSP implementations that require incoming requests to follow the specification to the letter.

Background

According to git blame, this behavior was introduced in #137, the goal of which was to fix the behavior for response result that indeed may not be omitted but may be null. Looks like the same logic was erroneously applied to request/notification params as well.

@jonahgraham
Copy link
Contributor

I suspect to change this back (i.e. revert #137) we need to get xtext involved so to not regress all the xtext generated language servers. See eclipse/xtext-core#558 for the discussion that led to #137's behaviour.

@cdietrich
Copy link
Contributor

cdietrich commented Sep 10, 2022

Is

Received message which is neither a response nor a notification message

fixed in vscode meanwhile?

@jonahgraham
Copy link
Contributor

If this is still relevant a failing unit test or similar would be helpful so we can decide how to proceed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants