Debug Adapter Protocol: Response messages do not contain required success key

[fwcd]

This is a sample JSON response from my debug adapter:

The specification - however - requires a success keyword:

interface Response extends ProtocolMessage {
  type: 'response';

  /**
   * Sequence number of the corresponding request.
   */
  request_seq: number;

  /**
   * Outcome of the request.
   */
  success: boolean;

  /**
   * The command requested.
   */
  command: string;

  /**
   * Contains error message if success == false.
   */
  message?: string;

  /**
   * Contains request result if success is true and optional error details if success is false.
   */
  body?: any;
}

See https://microsoft.github.io/debug-adapter-protocol/specification#Base_Protocol_Response

[fwcd]

The issue can probably be fixed by adding the following code to the else branch:

out.name("success"); 
out.value(true); 

https://github.com/eclipse/lsp4j/blob/0fa92a2881bcb952cef9976ad2b31d57c63b1728/org.eclipse.lsp4j.jsonrpc.debug/src/main/java/org/eclipse/lsp4j/jsonrpc/debug/adapters/DebugMessageTypeAdapter.java#L420-L442

This would also correspond to this claim:

https://github.com/eclipse/lsp4j/blob/0fa92a2881bcb952cef9976ad2b31d57c63b1728/org.eclipse.lsp4j.jsonrpc.debug/src/main/java/org/eclipse/lsp4j/jsonrpc/debug/adapters/DebugMessageTypeAdapter.java#L130-L133

[jonahgraham]

Thanks, your suggestion looks right. I think there is also a related issue that a missing success field in the response parsing means the message is assume to be successful, when it should be treated as unsuccessful.

[jonahgraham]

All fixed now. @fwcd are you able to rebuild locally until a release is made in early Sept?

[fwcd]

@jonahgraham The frontend in Visual Studio Code converts JSON messages to JavaScript objects without any further validations. When the success value is polled, JavaScript automatically converts a missing value to false:

This was also the reason why I got some pretty obscure error messages in the frontend.

I will integrate a fresh build of LSP4J in my adapter soon.

Thanks for your quick response!

[jonahgraham]

Thanks, I am pretty sure that LSP4J does the same thing now in this regard. There are other places where there will be differences, e.g. LSP4J (for debug and language servers) will fail to parse a message if it can't be parsed into the desired type, this means the errors that are raised will be at a different place. I believe VSCode will fail when the receiving message actually tries to use the parsed object,

AFAIK you are the first person implementing a debug adapter (as opposed to client) with LSP4J's DAP implementation. Please do let me know if you find any additional issues.

[KamasamaK]

AFAIK you are the first person implementing a debug adapter (as opposed to client) with LSP4J's DAP implementation.

@jonahgraham

What is the reason for a DAP implementation in LSP4J when it's a distinct protocol from LSP? Sorry if this should be its own issue.

[fwcd]

@KamasamaK I would suggest a new issue for this. My guess is that it was easier to bundle the JSON-RPC functionality with the protocols in a single repo.

[jonahgraham]

What is the reason for a DAP implementation in LSP4J when it's a distinct protocol from LSP? Sorry if this should be its own issue.

@KamasamaK

My personal argument is that the LSP4J project is badly named ;-) The vast majority (code/logic wise) of the LSP4J project is a Java implementation of JSONRPC 2.0 with extensions to that spec for LSP. The DAP is a JSONRPC 2.0-like protocol (it is similar, but different). So to benefit from code reuse LSP4J's JSONRPC part was refactored to make it generic enough to be used for both. What is DAP specific is pretty much logic-free definitions of the protocol.

However that does not really answer the question -- The reason is that it was a lot of overhead to have a separate project with release engineering, reviews, etc. That theoretical new project could depend on LSP4J's JSONRPC bundle, but all that would be in that new project with very high dependencies on LSP4J and have little in it.

Please raise a new issue if you would like to continue this conversation.