eclipse-lsp4j / lsp4j

A Java implementation of the language server protocol intended to be consumed by tools and language servers implemented in Java.
https://eclipse.org/lsp4j
Other
599 stars 143 forks source link

configurationDone signature incorrectly says "Void" #721

Closed mickaelistria closed 1 year ago

mickaelistria commented 1 year ago

vscode-js-debug sends the following response to configurationDone:

{
  "jsonrpc": "2.0",
  "id": 4,
  "method": "configurationDone",
  "params": {}
}

This is legal according to the specification. However, this cause LSP4J to fail at parsing the message, because LSP4J mandates a Void. The exception is

Caused by: java.lang.ClassCastException: class com.google.gson.JsonObject cannot be cast to class java.lang.Void (com.google.gson.JsonObject is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @694b1ddb; java.lang.Void is in module java.base of loader 'bootstrap')
    at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:934)
    at java.base/java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:911)
    at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
    at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2179)
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleResponse(RemoteEndpoint.java:212)
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:193)
    at org.eclipse.lsp4j.jsonrpc.validation.ReflectiveMessageValidator.consume(ReflectiveMessageValidator.java:68)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
    at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
    at java.base/java.lang.Thread.run(Thread.java:1589)
jonahgraham commented 1 year ago

This seems to imply all the CompletableFuture<Void> returns are incorrect in IDebugProtocolServer - or that we need to change LSP4J to be more accepting and allow a response.

Do you have the original JSON for this case? The quoted text in the description does not look right (it looks like the toString output in the debugger rather than what came over the wire).

jonahgraham commented 1 year ago

This is essentially a follow-up to #674 where better, but apparently still incomplete, Void handling was implemented.

mickaelistria commented 1 year ago

The json message received from vscode-js-debug/dapDebugServer is

{"seq":5,"type":"response","request_seq":5,"command":"configurationDone","success":true,"body":{}}
mickaelistria commented 1 year ago

675 allows message to be parsed, and allows to keep access to the actual value from the message. The error here happens later, when trying to feed the future with a non-void response; this response is cast to the Future parameter type (Void here), leading to this class cast exception.

I imagine 2 possible solutions: either when Void is expected, we always return null when the message is parsed (ie we rewrite test from #675 so the result is parsing is always null); or we just change the API so configurationDone returns a ConfigurationDoneResponse.

jonahgraham commented 1 year ago

either when Void is expected, we always return null when the message is parsed (ie we rewrite test from https://github.com/eclipse-lsp4j/lsp4j/pull/675 so the result is parsing is always null);

I have tried this in #728