Elements in args-list does not update when restarting debug session

[xerosugar]

Issue Type: Bug

• Create a launch.js for a node.js script.
• Set an args-field, like this in your launch.json:
  "args": ["--update", "filename.json", "filename2.json"]
• Set a breakpoint somewhere in your js-script.
• Run the debug-session, let VS Code stop at the breakpoint.
• Now, make a change to the update the args-list:
  "args": ["--update", "someOtherFile.json", "filename2.json"]
• Restart the debug-session (Ctrl+Shift+F5)
• The script still uses the old parameter-list.

VS Code version: Code 1.53.2 (622cb03, 2021-02-11T11:48:04.245Z)
OS version: Windows_NT x64 10.0.18363

System Info

Item	Value
CPUs	Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz (16 x 3600)
GPU Status	2d_canvas: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on oop_rasterization: enabled opengl: enabled_on protected_video_decode: unavailable_off rasterization: enabled skia_renderer: enabled_on video_decode: enabled vulkan: disabled_off webgl: enabled webgl2: enabled
Load (avg)	undefined
Memory (System)	31.89GB (23.69GB free)
Process Argv	--crash-reporter-id 4f8c59a8-5f72-44b3-a421-86f3d9ddce35
Screen Reader	no
VM	0%

Extensions (16)

Extension	Author (truncated)	Version
Bookmarks	ale	13.0.3
xml	Dot	2.5.1
haxe-hl	Hax	1.1.1
template-string-converter	meg	0.4.7
prettify-json	moh	0.0.3
vscode-docker	ms-	1.10.0
remote-wsl	ms-	0.53.4
cmake-tools	ms-	1.6.0
vscode-js-profile-flame	ms-	0.0.14
vshaxe	nad	2.22.1
lime-vscode-extension	ope	1.4.2
cmake	twx	0.0.17
hxcpp-debugger	vsh	1.2.4
gitblame	wad	6.0.2
codedox	wig	1.3.1
propertylist	zho	0.0.2

A/B Experiments

vsliv368cf:30146710
vsreu685:30147344
python383:30185418
vspor879:30202332
vspor708:30202333
vspor363:30204092
vstry244cf:30256637
pythonvsdeb440:30248342
pythonvsded773:30248341
pythonvspyt875:30259475
dockersubset:30265998
pythontb:30265425
vspre833cf:30267465

[isidorn]

I can reproduce this.
The issue is that the js-debug supportsRestartRequest. So we let the js-debug handle the restart. Even if there are changes in the configuration.

@connor4312 @weinand I am not sure what is the right behaviour here.
If a change happened to the launch configuration should VS Code disconnect and start the debug session again ignoring the supportsRestartRequest? Or should js-debug handle configuration changes as part of its restart?
Let me know what you think.

[weinand]

Today the spec of the DAP restart request doesn't say anything about changed launch configurations.
But we could add an optional config argument to the restart request so that an implementation has easy access to it and can compare it with the current values.

I suggest that we create a DAP feature request in the DAP's repo.

@connor4312 @isidorn what do you think?

[isidorn]

@weinand this sounds good to me
Let me know if you would like me to create a DAP feature request in the DAP repo.
Also let's see what @connor4312 thinks about this

[connor4312]

That sounds good to me

[weinand]

@isidorn could you please create a DAP feature request in https://github.com/Microsoft/debug-adapter-protocol/issues

[isidorn]

Created microsoft/debug-adapter-protocol#180
Once we add that I can adopt it in VS Code, and then @connor4312 can adopt it in js-debug.

[isidorn]

@vscode-triage-bot I hate you bot!

[weinand]

@connor4312 @isidorn Now the restart request takes an argument for the "launch" or "attach" configuration.
Please update VS Code and js-debug accordingly.

[isidorn]

@weinand great, I can look into this.

[isidorn]

I have pushed a change such that VS Code will send the configuration as part of the restart request. I send a resolved configuration, not the exact one that is in the launch configuration.
I can easily change that I send the exact one in launch.json.
Here's an example of what I would send

Assigning this item to you @connor4312 so you can look into adopting it on the js-debug side.
Also let me know if you would like me to change something on the client side. Thanks

[weinand]

@isidorn yes, sending the resolved configuration is correct (since that is the same what a DA sees in the "launch" or "attach" requests when it does not implement DAP's "restart" request).

(and it goes without saying that the resolved configuration must be based on the most recent launch config on disk...)

[isidorn]

Great, that is what I am doing now - respecting the most recent launch config on disk.

[connor4312]

It looks like for js-debug, restart is always being dispatched to the child session, so there's no configuration to re-read. Should restarting via the toolbar go to the root session like stop/detach does?

[isidorn]

@connor4312 yes this makes sense, I have pushed a change to tackle this.
So now restart, stop and disconnect are aligned, they always send to the parent session.

[weinand]

@connor4312 @isidorn since parent/child relations between debug sessions are not part of the DAP, I think the supportsLifecycle flag is more an option on the startDebugging extension API, right?

Name suggestion: manageByParent

[isidorn]

Yeah I think it would belong under the DebugSessionOptions in the startDebugging API since we already have parent child session flags there like parentSession and compact.

[connor4312]

Sounds good to me

[weinand]

I'll add the manageByParent property to the DebugSessionOptions in July.
Please see #128058.

[weinand]

@isidorn a new flag lifecycleManagedByParent is now available on IDebugSessionOptions passed into DebugService.startDebugging.

@connor4312 you can now start passing the correct value for your case via DebugSessionOptions.lifecycleManagedByParent.

[isidorn]

@weinand thanks for adding the flag.

Since the flag's name if lifecycleManagedByParent I think it means that all the disconnect and restart requests are actually sent to the parent session? I do not think this flag would affect any other requests, correct?

[weinand]

@isidorn yes, only if lifecycleManagedByParent is true, disconnect and restart requests are sent to the parent session.

@connor4312 that's OK with you?

[isidorn]

I pushed a change that the debugSession now respects lifecycleManagedByParent for disconnect, terminate and restart requests.

Leaving this issue open so @connor4312 can pass the lifecycleManagedByParent flag when calling the startDebugging API. Let me know how it goes. Thanks!

[weinand]

@connor4312 are you planning to add support for lifecycleManagedByParent to js-debug in this milestone?
If yes, I would create a test item for #128058 and you could implement the support in js-debug as part of testing.

[connor4312]

Fixed in microsoft/vscode-js-debug@f6635d9

It seems to work well. I'm ok marking the feature request as verified, unless we want to also have a TPI for it for a second verification?

[weinand]

@connor4312 yes, please mark #128058 as verified.

[weinand]

Verified that this works fine now.