Introduce noDebug flag to DebugSessionOptions

[isidorn]

Code pointer:

vscode/src/vs/workbench/api/browser/mainThreadDebugService.ts

Line 232 in e942645

noDebug: false,

startDebugging api makes an assumption that noDebug is false
The problem is shown when you want to Run Without Debugging using js-debug. The breakpoints are still hit.

Not sure who authored this code originally. Assigning all 3 of us, I can also look into it.
Assigning to June so we look into this if possible.

[weinand]

@isidorn IIRC you've introduced the VS Code API DebugSessionOptions which only surfaces a parentSession and a consoleMode. I understand that it would make sense to have a noDebug property on DebugSessionOptions because this makes it possible to use startDebugging both for the "debug" and the "run without debugging" cases.

So in June we should introduce the noDebug property on DebugSessionOptions.

But the way how DebugSessionOptions is converted into the internal IDebugSessionOptions is questionable but I do not really understand why the current hardcoded "false" results in the problem we are seeing with js-debug.

Are there three possible semantics for "noDebug" in DebugSessionOptions:

• false: start debugging
• true: run without debuging
• undefined: the decision is made somewhere else

[isidorn]

Good point, we can introduce that to the options.

There are just two ways how noDebug is interpretted:
If it is passed and true we set that noDebug: true otherwise we do not set the noDebug attribute in the launch configuration.

The hardcoded false results in the problem becuase:

1. User starts debugging with noDebug: true
2. VS Code sends to this js-debug
3. JS-debug wants to create a child session and calls the startDebugging api
4. The startDebugging api set the noDebug to false and thus the new session does not have a noDebug field set.

[weinand]

So the smallest fix for js-debug would be to pass a noDebug received for the parent session via startDebugging to a child session, correct?

Additionally my 3rd case "undefined" from above could become: startDebugging automatically uses the noDebug value from the parent session. With that js-debug would just work.

Possible descriptions for the three "noDebug" values in DebugSessionOptions:

• false: start debugging
• true: run without debugging
• undefined: if a parent session is given, value is inherited from the parent.

[isidorn]

@weinand yes, the js-debug could pass a noDebug flag to the child session configuration.
Makes sense for the description of the settings.
Before we introduce this option though let's see what @connor4312 thinks

[connor4312]

the js-debug could pass a noDebug flag to the child session configuration

I think this would be the best way to do it. We already pass information down from parents to children, we can do the same thing for noDebug.

However, js-debug right now doesn't respect the noDebug flag -- so that may have been one issue that you were seeing.

[weinand]

Summary and conclusion:

• VS Code will surface the noDebug property on DebugSessionOptions
• when the noDebug property is not specified in a startDebugging call, we will use the value from the parent session (if there is one). This behavior can already be implemented even if noDebug is not yet a property on DebugSessionOptions. This should fix one problem with js-debug.
• js-Debug needs to implement support for the noDebug property.

[isidorn]

As agreed I have surfaced the noDebug property on the DebugSessionOptions, thus adding API proposal so I bring this up in next weeks api sync fyi @jrieken
I have also changed the behavior of noDebug such that when undefined the behvaior of the parent session is inhereted.

@connor4312 since I will take this issue through the API process maybe we create a seperate one for js-debug and we unassign you from this one?

[connor4312]

Works for me

[isidorn]

For reference here's the js-debug issue microsoft/vscode-js-debug#523

[weinand]

@isidorn the comment for noDebug does not yet explain (the new behavior) when noDebug is missing.

[isidorn]

@weinand thanks, tackled this via 9bebaef

[isidorn]

Feedback from API sync call:
it is fine to add the noDebug flag to the DebugSessionOptions. Maybe consider another name runOnly, however since we already use noDebug in the "protocol" that might be the correct name.

[isidorn]

We have done what we planned for this milestone.
Thus moving this out to on-deck and changing to api-finalisation.
Will create a test plan item to cover this.

[isidorn]

We plan to finalize this API this August milestone. Just in case there is more feedback before we finalize it.

fyi @connor4312

[connor4312]

I've adopted this in js-debug for setting noDebug in child sessions. Work well, no complaints :)

[isidorn]

I have finalized this and created this test plan item to cover the finalisation
#104986

@connor4312 @weinand please assign yourself if you feel like testing this. Only one tester is need, since this is just finalisation