microsoft / ptvsd

Python debugger package for use with Visual Studio and Visual Studio Code.
Other
550 stars 68 forks source link

Debug Adapter: Compatibility with node 13 #2068

Closed puremourning closed 4 years ago

puremourning commented 4 years ago

Environment data

Expected behaviour

Debug adapter starts up under node v13 environment, when running node out/client/debugger/debugAdapter/main.js

Under node 12, it works:

bash-4.2$ node out/client/debugger/debugAdapter/main.js
(node:5366) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

Actual behaviour

bash-4.2$ node out/client/debugger/debugAdapter/main.js
Content-Length: 195

{"seq":0,"type":"event","event":"error","body":"Debugger Error: The number of constructor arguments in the derived class s must be >= than the number of constructor arguments of its base class."}Content-Length: 2067

{"seq":0,"type":"event","event":"output","body":{"category":"stderr","output":"Debugger Error: The number of constructor arguments in the derived class s must be >= than the number of constructor arguments of its base class.\nDebugger Error: The number of constructor arguments in the derived class s must be >= than the number of constructor arguments of its base class.\nError\nError: The number of constructor arguments in the derived class s must be >= than the number of constructor arguments of its base class.\n    at /mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:16:27311\n    at Array.forEach (<anonymous>)\n    at e (/mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:16:26875)\n    at Object.t.plan (/mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:16:27386)\n    at /mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:16:24044\n    at e._get (/mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:16:23953)\n    at e.get (/mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:16:22664)\n    at s.get (/mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:68:141080)\n    at new _ (/mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:1:269045)\n    at /mma/users/benj/.vim/bundle/vimspector/gadgets/linux/download/vscode-python/2020.1.58038/root/extension/out/client/debugger/debugAdapter/main.js:1:272287"}}bash-4.2$

Steps to reproduce:

  1. Download the extension from the GitHub releases, and unpack it
  2. Using nvm swtich to node 13 environment (nvm install 13; nvm use 13)
  3. node node out/client/debugger/debugAdapter/main.js

(error above)

Contrast with node 12:

  1. Using nvm swtich to node 12 environment (nvm install 12; nvm use 12)
  2. node node out/client/debugger/debugAdapter/main.js

(starts)

Logs

None - debug adapter doesn't start up.

Additional context

For simplicity's sake vimpector uses the debug adaptor out of this extension (rather than directly launching ptvsd). When users run in a node 13 environment, the adapter fails to start with the above error.

I assume that VScode is bundling and controlling the node environment normally, which is why VScode is not itself affected. Is there any interest in fixing the adapter to work in a node 13 environment ?

puremourning commented 4 years ago

Here's a more useful version of the trace, from master:

bash-4.2$ node ./out/client/debugger/debugAdapter/main.js
Content-Length: 208

{"seq":0,"type":"event","event":"error","body":"Debugger Error: The number of constructor arguments in the derived class ProtocolParser must be >= than the number of constructor arguments of its base class."}Content-Length: 1568

{"seq":0,"type":"event","event":"output","body":{"category":"stderr","output":"Debugger Error: The number of constructor arguments in the derived class ProtocolParser must be >= than the number of constructor arguments of its base class.\nDebugger Error: The number of constructor arguments in the derived class ProtocolParser must be >= than the number of constructor arguments of its base class.\nError\nError: The number of constructor arguments in the derived class ProtocolParser must be >= than the number of constructor arguments of its base class.\n    at /mma/users/benj/proj/vscode-python/node_modules/inversify/lib/planning/planner.js:111:27\n    at Array.forEach (<anonymous>)\n    at _createSubRequests (/mma/users/benj/proj/vscode-python/node_modules/inversify/lib/planning/planner.js:94:20)\n    at Object.plan (/mma/users/benj/proj/vscode-python/node_modules/inversify/lib/planning/planner.js:136:9)\n    at /mma/users/benj/proj/vscode-python/node_modules/inversify/lib/container/container.js:318:37\n    at Container._get (/mma/users/benj/proj/vscode-python/node_modules/inversify/lib/container/container.js:311:44)\n    at Container.get (/mma/users/benj/proj/vscode-python/node_modules/inversify/lib/container/container.js:230:21)\n    at ServiceContainer.get (/mma/users/benj/proj/vscode-python/out/client/ioc/container.js:28:89)\n    at new DebugManager (/mma/users/benj/proj/vscode-python/out/client/debugger/debugAdapter/main.js:341:58)\n    at startDebugger (/mma/users/benj/proj/vscode-python/out/client/debugger/debugAdapter/main.js:439:30)"}}bash-4.2$

Related: https://github.com/inversify/InversifyJS/blob/068732327fcbfe061975d1400e582120fea4ea36/wiki/inheritance.md

karthiknadig commented 4 years ago

@puremourning We are slowly phasing it out the TypeScript debug adapter. We have a new python based debug adapter.

If you want to try the new one, you will need to follow these steps. Switch to the insiders version of the extension:

"python.insidersChannel": "daily"

Then add these settings:

"python.experiments.optInto": [
        "DebugAdapterFactory - experiment",
        "PtvsdWheels37 - experiment"
    ]

Reload, VS Code after adding. Check the Output > Python panel , it should have these lines:

User belongs to experiment group 'DebugAdapterFactory - experiment'
User belongs to experiment group 'PtvsdWheels37 - experiment'

Let me know if this helps.

int19h commented 4 years ago

@puremourning Since you're not actually using this with VSCode, you should benefit significantly from the new (ptvsd>=5) debugger being self-contained - you should be able to just run the package directly with any supported Python version, and avoid having any dependencies from vscode-python repo.

To use it as a debug adapter, run the ptvsd.adapter module. By default, this will use stdin/out as the DAP transport, per spec. If you have a local copy of ptvsd, you can also run the directory instead of using -m to run as a module - i.e. instead of python -m ptvsd.adapter, you can also do python .../ptvsd/adapter. When running in this mode, the adapter can service "launch" requests, and "attach" requests with "processId" specified (i.e. attach-by-PID).

For "attach" requests with "host" and "port specified (i.e. attach-by-socket), the debug server that is started by using ptvsd.enable_attach(), or the corresponding ptvsd CLI, is a DAP server that is listening on the specified socket. So the client needs to connect to that socket instead of using the adapter with stdin/out - but from there it's just a regular DAP session.

puremourning commented 4 years ago

Thanks. I’ll try it this weekend. Does this self contained ptvsd adapter use the same launch config as vscode-python? I considered switching in the past, but as the launch configs are different it would be a breaking change for users.

int19h commented 4 years ago

For now it can handle the old configs as well, although there are some changes to defaults - e.g. "console" defaults to "integratedTerminal" now, and "subProcess" to true. We'll be deprecating the old stuff going forward though - but that will apply to the VSCode extension also.

The biggest incentive to switch, though, is that "pythonPath" is now just "python" - so you won't have to explain to the users how and why it's totally different from PYTHONPATH anymore. ~

puremourning commented 4 years ago

:) thanks 🙏

int19h commented 4 years ago

One more thing. We're in the process of doing a rename of the project, in part to remove the legacy association with VS - instead of ptvsd 5.0, it's going to be debugpy 1.0. In fact, the package is already on PyPI, and the repo is over at https://github.com/microsoft/debugpy - so if you're moving to the new version, you might as well start with that one. This also means that users can have both installed side by side, and we'll keep this repo as a documentation and list of known issues for the existing stable ptvsd 4.x releases.

puremourning commented 4 years ago

@int19h - thanks I was able to make debugpy work, though I did have to specify "python": "python" (or a specific path), whereas with "vscode-python" this wasn't necessary. (just FYI)

int19h commented 4 years ago

It should allow it to be omitted, and default to sys.executable used to start the adapter in that case. Can you file a separate bug on that not working correctly?

karthiknadig commented 4 years ago

@puremourning with the python extension, it adds "pythonPath" : "${config:pythonPath}" by default. So you don't need to put that in the launch configuration.

An advantage of using "python", is that you can pass interpreter arguments to it as well. So these are all valid:

puremourning commented 4 years ago

@karthiknadig I'm all for that change, I was just reporting that as a strict compatibility with the typescript adapter, this is a change.

@int19h raised: https://github.com/microsoft/ptvsd/issues/2070

As it happens there's a simple way in vimspector to specify a default for this value, so I'm not super worried.

int19h commented 4 years ago

Even with the extension not in the picture, there's some logic to pick a default for "python" in the adapter itself (or rather in the launcher). I added that there specifically for non-VSCode use cases, hence why I'm treating it as a bug. I think that code just got regressed at some point, probably when I added the ability to use an array there instead of a single value - it's still trying to do this, it's just not doing it correctly.

int19h commented 4 years ago

I'm going to go ahead and close this issue - if there are any more issues with debugpy Vimspector integration, just file them directly in the new repo.

puremourning commented 4 years ago

Thanks for everything. Much appreciated.