microsoft / vscode-chrome-debug-core

A library for implementing VS Code debug adapters for targets that implement the Chrome Debugging Protocol.
Other
157 stars 119 forks source link

Prerequisites for `setVariable` to work? #514

Open mickaelistria opened 5 years ago

mickaelistria commented 5 years ago

I'm embedding this debug adapter (the vscode-node-debug2 more specifically, but I tracked down the issue to this component) in Eclipse IDE. Most request work fine, however I tried to enable support for setVariable, which capability declare is enabled, but I face

{"seq":93,"type":"event","event":"output","body":{"category":"telemetry","output":"ClientRequest/variables","data":{"Versions.DebugAdapterCore":"6.7.55","Versions.DebugAdapter":"1.33.0","Versions.Target.Version":"v10.16.3","successful":"true","timeTakenInMilliseconds":"1.438987","requestType":"request"}}}Content-Length: 385

{"seq":94,"type":"event","event":"output","body":{"category":"telemetry","output":"ClientRequest/setVariable","data":{"Versions.DebugAdapterCore":"6.7.55","Versions.DebugAdapter":"1.33.0","Versions.Target.Version":"v10.16.3","successful":"false","exceptionType":"firstChance","exceptionMessage":"The debug adapter doesn't recognize this command","timeTakenInMilliseconds":"0.318617"}}}Content-Length: 254

{"seq":95,"type":"response","request_seq":12,"command":"setVariable","success":false,"message":"[node-debug2] Unrecognized request: setVariable","body":{"error":{"id":1014,"format":"[node-debug2] Unrecognized request: setVariable","sendTelemetry":true}}}

Is there some other prerequisite to make setVariable work?

The js file I'm debugging is a base

var n = 4;
console.log("hi" + n);

and I try to change value when breakpoint is on the 2nd line.

If I tweak the chromeDebugAdapter.js file to add

    setVariable(args) {
          this._variablesManager.setVariable(args);
    }
``` nearby `setBreakpoints` I then get the following error
```json
{"seq":96,"type":"event","event":"output","body":{"category":"telemetry","output":"ClientRequest/setVariable","data":{"Versions.DebugAdapterCore":"6.7.55","Versions.DebugAdapter":"1.33.0","Versions.Target.Version":"v10.16.3","successful":"true","timeTakenInMilliseconds":"4.713406","requestType":"request"}}}Content-Length: 1991

{"seq":97,"type":"event","event":"output","body":{"category":"telemetry","output":"error","data":{"Versions.DebugAdapterCore":"6.7.55","Versions.DebugAdapter":"1.33.0","Versions.Target.Version":"v10.16.3","successful":"false","exceptionType":"unhandledRejection","exceptionMessage":"Setting value not supported","exceptionName":"Error","exceptionStack":"Error: Setting value not supported\n    at Object.setValueNotSupported (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/errors.js:54:12)\n    at VariablesManager.setVariable (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/variablesManager.js:169:42)\n    at NodeDebugAdapter.setVariable (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugAdapter.js:909:31)\n    at Object.<anonymous> (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:102:78)\n    at Generator.next (<anonymous>)\n    at /home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:10:71\n    at new Promise (<anonymous>)\n    at __awaiter (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:6:12)\n    at reportTelemetry (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:92:113)\n    at Object.<anonymous> (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:147:19)","exceptionId":"2004"}}}Content-Length: 1600

{"seq":98,"type":"event","event":"output","body":{"category":"stderr","output":"******** Unhandled error in debug adapter - Unhandled promise rejection: Error: Setting value not supported\n    at Object.setValueNotSupported (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/errors.js:54:12)\n    at VariablesManager.setVariable (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/variablesManager.js:169:42)\n    at NodeDebugAdapter.setVariable (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugAdapter.js:909:31)\n    at Object.<anonymous> (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:102:78)\n    at Generator.next (<anonymous>)\n    at /home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:10:71\n    at new Promise (<anonymous>)\n    at __awaiter (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:6:12)\n    at reportTelemetry (/home/mistria/git/wildwebdeveloper/org.eclipse.wildwebdeveloper/language-servers/node_modules/vscode-chrome-debug-core/out/src/chrome/chromeDebugSession.js:92:113)\n    at Object.<[...]\n"}}Content-Length: 81
roblourens commented 5 years ago

@EricCornelson I think that we lost a handler for this request with your refactoring.

EricCornelson commented 5 years ago

Got it, I'll take a look

roblourens commented 5 years ago

And @mickaelistria from the error after the handler is fixed, it sounds like variablesReference is not included which I think should be the variablesReference for the scope.

mickaelistria commented 5 years ago

Sorry, I'm getting a bit lost here ;). Do you confirm the Unrecognized request: setVariable", is a bug in the debug adapter?

About the unhandledRejection after my tweak, variablesReference here is what I get as result of variables

{"seq":95,"type":"response","request_seq":12,"command":"variables","success":true,"body":{"variables":[{"name":"this","value":"Object","type":"Object","variablesReference":1015,"evaluateName":"this"},{"name":"__dirname","value":"\"/home/mistria/workspace-demo/l\"","variablesReference":0,"evaluateName":"__dirname","type":"string"},{"name":"__filename","value":"\"/home/mistria/workspace-demo/l/blah.js\"","variablesReference":0,"evaluateName":"__filename","type":"string"},{"name":"[[StableObjectId]]","value":"1","variablesReference":0,"evaluateName":"[[StableObjectId]]","type":"number"},{"name":"exports","value":"Object {}","type":"Object","variablesReference":1012,"evaluateName":"exports"},{"name":"module","value":"Module {id: \".\", exports: Object, parent: null, …}","type":"Object","variablesReference":1014,"evaluateName":"module"},{"name":"n","value":"4","variablesReference":0,"evaluateName":"n","type":"number"},{"name":"require","value":"function require(path) { … }","type":"Function","variablesReference":1013,"evaluateName":"require"}]}}

The interesting part is {"name":"n","value":"4","variablesReference":0,"evaluateName":"n","type":"number"}, and in this interesting part, I see variablesReference=0. I guess one reason is that this variable doesn't have children, but it seems to prevent the setVariable to work later. Here is the request sent by my IDE for setVariable

{"type":"request","seq":13,"command":"setVariable","arguments":{"variablesReference":0,"name":"n","value":"5","format":{}}}

Which seems correct per se: the variablesReference re-uses the value received earlier, I just imagine this 0 retrieved from the Debug Adapter may be wrong.

mickaelistria commented 5 years ago

An important thing I forgot to mention: I'm using version 6.7.55 which is extracted transitively from VSCode with node-debug2 1.33.0.

mickaelistria commented 5 years ago

Another interesting part about this variablesReference=0 is that it's assigned to multiple variables. Here we see for instance __filename, __dirname and [[StableObjectId]] and my local n that have the same variablesReference=0. This is defintely a bug, however it doesn't seem related to the initial one about Unrecognized request: setVariable, so if you prefer, I can open a new bug for the variablesReference=0 after Unrecognized request: setVariable is resolved.

roblourens commented 5 years ago

Yeah the "unrecognized request" is our bug.

The variablesReference of 0 is misleading - it only has a "real" value for objects. Primitive values have it set to 0. For setVariable, it needs to be set to the variablesReference for the parent scope of the variable being set. Because we are just looking up a variable based on its scope and name.

mickaelistria commented 5 years ago

The 'variablesReferenceof 0 is misleading - it only has a "real" value for objects. Primitive values have it set to 0. ForsetVariable`, it needs to be set to the variablesReference for the parent scope of the variable being set. Because we are just looking up a variable based on its scope and name.

Thanks, that's very helpful. I may have some more questions later, but that already unblocks many things on my end.

mickaelistria commented 5 years ago

Thanks to your hints, I can confirm that one a handler is added for setVariable, I can successfully get the set variable workflow run in Eclipse IDE.