Handle unavailable sources in compliance with DAP spec - #609
Conversation
Sorry, something went wrong.
Closes #67 Zed and java-debug appear to disagree in their interpretations of the DAP-spec. Since we have no control over the release cycle of java-debug and indeed whether they accept the change to begin with, and the Zed team declines to adopt a more lenient stance in their interpretation of the spec, we must run our own fork of java-debug. I've created a fork with the required fixes and a release for us to use here: https://github.com/playdohface/java-debug This PR will make the extension download the forked release once if not installed yet, and then use it indefinitely. When the underlying issues are resolved either from Zeds side (zed-industries/zed#37080) or java-debug merges our changes and makes a new release (microsoft/java-debug#609) we can switch back our code to use the latest official release for java-debug.
|
It's good to go. Thanks for the contribution. Karl-Erik Enkelmann (@playdohface) |
Sorry, something went wrong.
|
This is breaking nvim-dap and from what I can tell not compliant with the DAP spec: }, {
column = 1,
id = 148,
line = 0,
name = "0x00000000a114abb0.run()",
presentationHint = "subtle",
source = {
name = "Unknown Source",
path = "unknown",
sourceReference = 0
}
}, {
According to https://microsoft.github.io/debug-adapter-protocol/specification#Types_Source path is nullable, which for a client is a more reasonable value than unknown which if interpreted as a path will raise unknown file errors, unless the user happens to have a /unknown file on the filesystem - in which case it would still be wrong. But I think even clearer would be to keep source null - there isn't one after all. (Also note that line=0 may not be enough for a client to identify if source is available or not if it uses linesStartAt1 = false in the initialize request) |
Sorry, something went wrong.
Mathias Fußenegger (@mfussenegger) Yes, I read the spec again and I agree that the spirit of the spec probably intends for an absent source in this case. The reason why I added an explicit "unknown" source was because that was required to get it working with Zed, but it turns out that probably both sides were to blame here: java-debug for setting line=-1 which violates the spec, and Zed for not handling the source=null case. For reference. zed-industries/zed#37080 I will work on a downstream fix for Zed, and would ask to merge #614 |
Sorry, something went wrong.
Follow up to: - microsoft#614 - microsoft#609 With the change to set the line number to 0 the jdiLineNumber != lspFrame.line comparison can evaluate to true: dap> lspFrame Types$StackFrame@78 column: 1 id: 6 line: 0 name: "0x000000002f0bc000.invokeVirtual(Object,Object)" presentationHint: "subtle" source: null dap> jdiLineNumber -1 `source` being null caused an NPE
* Fix lspFrame.source NPE on stackTrace request Follow up to: - #614 - #609 With the change to set the line number to 0 the jdiLineNumber != lspFrame.line comparison can evaluate to true: dap> lspFrame Types$StackFrame@78 column: 1 id: 6 line: 0 name: "0x000000002f0bc000.invokeVirtual(Object,Object)" presentationHint: "subtle" source: null dap> jdiLineNumber -1 `source` being null caused an NPE * Fix line number comparison in StackTraceRequestHandler --------- Co-authored-by: Changyong Gong <shawn.gong@hotmail.com>
Fixes #605