Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Attach to extension host #4

Closed
roblourens opened this issue Aug 26, 2016 · 16 comments
Closed

Attach to extension host #4

roblourens opened this issue Aug 26, 2016 · 16 comments

Comments

@roblourens
Copy link
Member

Not sure how this is different from attaching to a typical node process

@weinand
Copy link
Contributor

weinand commented Aug 29, 2016

@roblourens This is basically a launch followed by an attach. But the launch uses VS Code to launch an extension host via some special command line arguments. And the disconnectRequest needs special code to support reloading the extension host.

Please note, that for testing this VS Code needs to be modified so that it uses a version of node that supports CDP (instead of the node version that Electron uses). For that reason I do not think that this feature needs to be implemented before node v7 has arrived in Electron.

@roblourens
Copy link
Member Author

Oh, good point. I'll move it to lowest priority

@roblourens roblourens mentioned this issue Oct 5, 2016
15 tasks
@roblourens
Copy link
Member Author

Blocked? electron/electron#6634

@roblourens
Copy link
Member Author

Blocked on microsoft/vscode#28582

@weinand
Copy link
Contributor

weinand commented Jul 12, 2017

@roblourens I'll provide detailed info for what needs to be done here.

@weinand
Copy link
Contributor

weinand commented Jul 26, 2017

@roblourens here are some steps for what needs to be done to support "inspector" protocol based EH debugging:

  • I've added a startSession command for debug type "extensionHost" here which allows to use "protocol": "inspector" to switch the debug type to "extensionHost2". In the final version we don't need the switch because we will hardcode this (because we know the Electron version).
  • node-debug2 needs to introduce a debug type "extensionHost2" similar to "node2". There is no need to specify an attribute schema because "extensionHost2" will not be used in launch.configs (and there is no schema validation based on it).
  • the "extensionHost2" type will be used in the node-debug2 implementation to do things a bit differently in three places.
  • Optionally you can work around the issue that some "rejects" in the startup phase of the EH result in the debugger breaking on reject exceptions. This is painful if it happens with every reload of the EH. node-debug has introduced a third exception configuration option for this. We enable the existence of the option in the initializeRequest .
  • And most importantly, when launching the EH process in debug mode, an "--inspector*" flags must be used.

@roblourens
Copy link
Member Author

Thanks! Will start on this today.

@roblourens
Copy link
Member Author

I implemented this but there are two issues: when the attach request comes in, the debug config has type 'extensionHost' instead of 'extensionHost2'. But that's not a big deal.

Also I can't get 'restart' to work. It looks like it's vscode's job to restart the extensionHost window? I have the fix to not kill the process for a restart, and I send a TerminatedEvent the same as vscode-node-debug, but it doesn't reload the window. Debugged the vscode side but I'm not sure what's missing. cc @isidorn

@weinand
Copy link
Contributor

weinand commented Jul 27, 2017

@isidorn could you send the "attach" request with the massaged config that you receive in the 'vscode.startDebug' command?
I'm converting 'extensionHost' to 'extensionHost2' for Electron 1.7.4. So attach should use the same type.

@roblourens Isi has fixed "restart" in today's Insiders by basically doing a reload window on the extension host window. Did you try with that version? Without the fix I was not seeing any effect with node-debug when pressing the restart button. That was bug microsoft/vscode#31445.

@roblourens
Copy link
Member Author

Thanks for the tip, I'm building out of the electron/1.7.4 branch so I'll try with newer stuff merged in.

@weinand
Copy link
Contributor

weinand commented Jul 27, 2017

Yes, if the electron branch hasn't been updated, it misses several fixes for EH debugging problems (that got introduced because of the multi-folder work).

I noticed that when trying to use the Electron 1.7.4 build two days ago.

@isidorn
Copy link
Contributor

isidorn commented Jul 28, 2017

@weinand could you please elaborate your question. Currently the only place where I call attach is here and before that the configurations is massaged.

@weinand
Copy link
Contributor

weinand commented Jul 28, 2017

@isidorn in EH debugging, the launchRequest starts the extension host (via VS Code) but the DA does not connect (aka "attach") to it (in normal "node" debugging the DA connects to the launched debuggee automatically).

Instead VS Code knows when the EH is ready and then broadcasts an event that you will pick up and issue the missing "attach" request for it. This is basically the same sequence as the "reload EH window" sequence.

For the "attach" request that you issue in response to the broadcast, you use the initial "launch" configuration and just change the "request" attribute from "launch" to "attach".
Since the initial "launch" config uses the type "extensionHost", the "attach" uses that too.

The fix is to remember the massaged launch config (that got the type "extensionHost2"), and create the "attach" config based on this.

@isidorn
Copy link
Contributor

isidorn commented Jul 28, 2017

@weinand thanks for the explanation, do we need this for july? If not, I will create a new issue for me

@weinand
Copy link
Contributor

weinand commented Jul 28, 2017

@isidorn No, not for July (and as Rob said: this is not a big deal).

We should fix this more for consistency: if a launch config is modified in the extension, we should always use the modified config and not fall back to the original config.

@isidorn
Copy link
Contributor

isidorn commented Jul 28, 2017

@weinand makes sense, created microsoft/vscode#31659

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants