Expose DAP IDs of breakpoints in vscode API

[connor4312]

For performance profiling, we let users pick the breakpoints to run to. For this I need to know the DAP ID of the breakpoint, since that's what my debug adapter, in turn, knows about. However, the breakpoint IDs exposed on the API only have a GUID which does not seem correlated with anything else.

In the past I got around this by just having a custom "get breakpoints" request through DAP that returned the breakpoints from the debug adapter and we can show the right UI. But with Jackson's good suggestion here #96473 (comment) I now also want to get the file URI of the breakpoint to show it as a preview.

This ultimately means I need to either redo the URI-generation behavior, or have logic to correlate my DAP breakpoints with debug breakpoints. I ended up doing the latter, but it would be much nicer if breakpoints had some API like:

getDapIdInSession(session: DebugSession): string | undefined;

My current hack-around: https://github.com/microsoft/vscode-js-debug/blob/88e380e64f5bac3a722f879a95793e2688bd4062/src/ui/profiling/breakpointTerminationCondition.ts#L58-L71

const vscodeBp = vscode.debug.breakpoints.find(
  (bp): bp is vscode.SourceBreakpoint =>
    bp instanceof vscode.SourceBreakpoint &&
    bp.enabled &&
    bp.location.range.start.line === line - 1 &&
    comparePathsWithoutCasingOrSlashes(bp.location.uri.fsPath, path),
);

[isidorn]

@connor4312 if we were to add this I think the most elegant way is simply add an additonal method as you propose to the Breakpoint class.

Please note that currently the vscode api has no notion of the debug adapter protocol. But has a notion of debug adapters, and thus I would not include the word protocol in the name of this method. Also this feels very internal and thus maybe not a best candidate for new API.

@weinand for thoughts and suggestions. Where there other requests for something like this from other extension authors?

[isidorn]

Unassigning myself for now. @weinand feel free to assign back to me once you have decided on the best path forward. Thanks

[weinand]

I've planned to investigate in July what to do here.

[weinand]

VS Code breakpoints are independent from debug adapters and they live outside of debug sessions. VS Code breakpoints are identified by a UUID (of type string).

When a debug sessions starts, VS Code breakpoints are registered with the debug adapter of the debug session and VS Code receives a DAP breakpoint in return. That means one VS Code breakpoint maps to one or more DAP breakpoints (depending on the number of concurrently active debug sessions). DAP breakpoints are identified within a debug session by a numeric ID (of type integer). VS Code maintains a mapping between its breakpoints and corresponding DAP breakpoints.

Surfacing this mapping in the extension API is possible in basically three different ways.

As already suggested above we could add a prominent method on the VS Code breakpoint:

export class Breakpoint {
    getDAPBreakpointForSession(session: DebugSession) : DAPBreakpoint | undefined;
}

Another approach is to provide the mapping prominently on the session:

export class DebugSession {
    getDAPBreakpoint(breakpoint: Breakpoint) : DAPBreakpoint | undefined;
}

or we could "hide" this mapping in the debug namespace:

export namespace debug {
    export function getDAPBreakpoint(session: DebugSession, breakpoint: Breakpoint) : DAPBreakpoint | undefined;
}

I'm leaning towards the third option because it better hides this really advanced API.

Another issue is the use of the DAPBreakpoint type. Currently we are trying to avoid including (or copying) the Debug Adapter Protocol in the VS Code extension API.

Whenever there is a need to connect VS Code's extension API with DAP we introduce an opaque stand-in type. Examples for this are DebugProtocolMessage and DebugProtocolSource. Following this strategy we could introduce a new stand-in type DebugProtocolBreakpoint.

This leads to the following proposal:

/**
 * A DebugProtocolBreakpoint is an opaque stand-in type for the [Breakpoint](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Breakpoint) type defined in the Debug Adapter Protocol.
 */
export interface DebugProtocolBreakpoint {
  // Properties: see details [here](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Breakpoint).
}

export namespace debug {
    export function getDebugProtocolBreakpoint(session: DebugSession, breakpoint: Breakpoint) : DebugProtocolBreakpoint | undefined;
}

@connor4312 what do you think?

[connor4312]

Regardless of where it's exposed, the stand-in type looks good.

Putting the method on the breakpoint or debug session is prominent, mainly because these two types have few existing properties or methods. Having this one prominent advantaged API on either of those feels odd to me as well. On the other hand, from a OOP point of view one of those would be the most natural place, but it's ambiguous which one. So option 3 makes sense to me.

[weinand]

@isidorn it would not be a big effort to implement the debug.getDebugProtocolBreakpoint(session, breakpoint) API that returns the DAP breakpoint for a given VS Code breakpoint, correct?

[isidorn]

@weinand correct. That should be doable. Breakpoint in the debug model also contains all the DAP breakpoint data I believe, if it does not we could easily add it.

[weinand]

After discussing the proposal in the API sync call, we've agreed to add the new API on the DebugSession (instead of the debug namespace). Here is the final proposal:

/**
 * A DebugProtocolBreakpoint is an opaque stand-in type for the [Breakpoint](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Breakpoint) type defined in the Debug Adapter Protocol.
 */
export interface DebugProtocolBreakpoint {
  // Properties: see details [here](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Breakpoint).
}

export class DebugSession {
    getDebugProtocolBreakpoint(breakpoint: Breakpoint) : DebugProtocolBreakpoint | undefined;
}

[isidorn]

This looks good to me. Let me know if you need help with the implementation.
Each Breakpoint has a map of session id to session data here. Using that data you should be able to create the DebugProtocolBreakpoint object.

The stored object contains a bit more, as seen with the mixin here
You could change the mixin of objects to make it more easy to extract the original data: DebugProtocol.Breakpoint

[weinand]

I've implemented the proposed API with c233bf8
@isidorn could you please verify the changes and add code to copy the DA's payload data here:

vscode/src/vs/workbench/contrib/debug/common/debugModel.ts

Line 614 in c233bf8

// TODO: copy additional debug adapter data

[isidorn]

Looks good, pushed a minor fix on top.

[weinand]

I've implemented the proposed API which is now ready for finalisation (but still needs a jsdoc).

[weinand]

@connor4312 the new API is now available in Insiders. Please give it a try...

[connor4312]

I've adopted this in microsoft/vscode-js-debug@2540489#diff-f44b7dbc3c9b27f609323871139f7d34, seems to work very nicely 🙂

[weinand]

The final proposed API which is now ready for finalisation:

vscode/src/vs/vscode.proposed.d.ts

Lines 740 to 756 in b0f0a1b

	/**
	* A DebugProtocolBreakpoint is an opaque stand-in type for the [Breakpoint](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Breakpoint) type defined in the Debug Adapter Protocol.
	*/
	export interface DebugProtocolBreakpoint {
	// Properties: see details [here](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Breakpoint).
	}
	export interface DebugSession {
	/**
	* Maps a VS Code breakpoint to the corresponding Debug Adapter Protocol (DAP) breakpoint that is managed by the debug adapter of the debug session.
	* If no DAP breakpoint exists (either because the VS Code breakpoint was not yet registered or because the debug adapter is not interested in the breakpoint), the value `undefined` is returned.
	*
	* @param breakpoint A VS Code [breakpoint](#Breakpoint).
	* @return A promise that resolves to the Debug Adapter Protocol breakpoint or `undefined`.
	*/
	getDebugProtocolBreakpoint(breakpoint: Breakpoint): Thenable<DebugProtocolBreakpoint | undefined>;
	}