Support scriptParsed like event in the protocol.

[dlebu]

The Chrome debugging protocol has a scriptParsed event that is being fired every time the virtual machine parses a script.

I talked to @roblourens about this and he told me about the Open Loaded Script command which causes the debug adapter to return a list of all loaded scripts, but an event would be a lot better since it eliminates polling.

[andrewcrawley]

The current experience in VS is that loaded script documents are treated as modules, so I propose re-using the existing module event rather than creating a new one.

export interface Module {
    // ...
    /** Document that represents this module. */
    document?: Source;
    // ...
}

In addition to the Modules window, VS shows loaded script documents in a "Script Documents" node in Solution Explorer, and also allows connections between project documents and generated source, so the loaded script document (app.js) has the project document (app.ts) as a child:

I'm not sure how best to represent this relationship in the protocol. Possibly we could add a "parent" field to Module and send two Module events in this case, with the .ts file referencing the .js as its parent.

[richardstanton]

Based on some discussion with @andrewcrawley, we think it would make more sense to have a new event that provides notification of new scripts instead of overloading the module event. The event would have a reference to the source file and some metadata about the script.

[richardstanton]

Here is the an event I prototyped for doing this. Note that scripts can have children if they have related files. For example a .js script might have its .ts script as a child.

/** Event message for 'script' event type.
    The event indicates that some information about a script has changed.
*/
export interface ScriptEvent extends Event {
    // event: 'script';
    body: {
        /** The reason for the event. */
        reason: 'new' | 'removed';
        /** The new or removed script. In case of 'removed' only the script id is used. */
        script: Script;
    };
}

/** A Script object represents a source that is dynamically loaded by the debugger.
    An id identifies a script and is used in a ScriptEvent for identifying a script for loading or unloading.
*/
export interface Script {
    /** Unique identifier for the script. */
    id: number;
    /** The source for the script. */
    source: Source;
    /** An optional list of scripts that are related to the main source.
        These may be the source that generated the main source.
        These will be displayed in a tree in the UI.
    */
    children?: Script[];
}

[andrewcrawley]

We'll probably need a request for retrieving all loaded scripts as well, to handle attach scenarios where we didn't receive the initial events. The other option would be for debug adapters to send a bunch of script events immediately after an attach, but that's kind of ad-hoc. Suggested:

export interface Capabilities {
    // ...
    /** The debug adapter supports the 'scripts' request. */
    supportsScriptsRequest?: boolean;
    // ...
}

/** Retrieves the set of script documents currently loaded by the debugged process. */
export interface ScriptsRequest extends Request {
    // command: 'scripts';
    arguments: ScriptsArguments;
}

/** Arguments for 'scripts' request. 
    The 'scripts' request has no standardized arguments.
*/
export interface ScriptsArguments {
}

/** Response to 'scripts' request. */
export interface ScriptsResponse extends Response {
    body?: {
        /** Set of loaded script documents. */
        scripts: Script[];
    };
}

[andrewcrawley]

@weinand - Any thoughts on this? We've implemented this event in the VS host and the node2 adapter for verification, and it looks like Richard's proposal will meet our needs. I can send a PR against the schema if you'd like.

[weinand]

We have prototyped something similar as part of microsoft/vscode#28521.

The motivation behind that effort was to design and create debug extension API that would support to create a "Script Explorer" solely in an extension (instead of VS Code), because it is difficult to provide a generic "Script Explorer" in the VS Code core debugger that supports the various requirements of different debuggers/runtimes. The "Script Explorer" will appear in tomorrows Insider build.

Currently this "Script Explorer" relies on two custom protocol additions: a "scriptLoaded" event and a "getLoadedScripts" request. The next step is to consolidate the custom protocol additions with the proposal at hand and/or the existing modules support.

[andrewcrawley]

I don't see commits associated with any of those issues - can you point me towards the schema changes?

[weinand]

Those protocol additions are custom (aka "private") events and requests. They are not part of the schema or protocol. They are used in the node-debug extension between the DA and the extension. They do not appear outside of the extension.

They are introduced in an ad-hoc way here https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/nodeDebug.ts#L437 and here https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/nodeDebug.ts#L3718. The event has just a "path" attribute and the request an array of paths (strings).

The Script Explorer shows the scripts in a tree grouped by various categories (internal modules, workspace scripts, external scripts). The explorer receives the events here: https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/extension/loadedScripts.ts#L75 and requests them here: https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/extension/loadedScripts.ts#L305

In this screenshot you can see the loaded scripts of two parallel debug sessions:

[andrewcrawley]

Thanks for the info. Maybe I'm misunderstanding, but I don't think a simple path string is enough for us to implement the features we want. For instance:

• How do you open an item from the tree view that isn't backed by a physical file (e.g. a file internal to the Node runtime)? In a stack trace, this would be represented by a Source object with the sourceReference field set, but I'm not sure how you'd represent this with just a string.
• How do you represent one file being a child of another (e.g. informing the user that "app.js" is a generated file based on "app.ts" in the actual project, as seen in my original screenshot)? I'm guessing from your screenshot that you're determining the tree structure from the file paths, but that won't work for nesting one file under another.
• How do you indicate when a script is unloaded?

What's the difficulty in providing a generic "Loaded Scripts" tree? Is there an existing discussion on this I can read? VS has a generic "Script Documents" tree that multiple debuggers integrate with. If every debug adapter out there has to invent their own custom UI and a set of messages for populating it, there's no way we're going to be able to map them onto VS's system.

[weinand]

I probably didn't make it clear enough, but my info from above is neither a proposal nor a counter-proposal. It is just a summary of an experiment that we did.

The motivation behind that effort was to design and create VS Code debug extension API that would support to create a "Script Explorer". It is only related to the debug adapter protocol in so far as it uses an internal and private request to retrieve the loaded scripts. We could have used any other example and not touch the "scriptParsed" issue in any way, but since we had already a half-baked QuickPick based version of the Loaded Scripts functionality we decided to go the whole nine yards.

The VS Code debug extension API is the api that extension authors can use to interact with any VS Code debug session from an extension. This makes it even possible to create custom UI for specific debug functionality (which is not possible from a debug adapter alone). The VS Code debug extension API is completely unrelated to the debug protocol.

Since we now have a prototype of a extension-based "Script Explorer", we can use it as the vehicle to design/validate the debug protocol and the UI.

So the next step will be to use the "official debug adapter protocol for loaded scripts" instead of the private "getLoadedScripts" request.

But before we "bless" the proposal from this protocol change request into the "official debug adapter protocol for loaded scripts", I would like to understand a few things:

"Scripts" are not a concept that all runtimes/debuggers support. For C# and C++ we've introduced a full "modules" complex some time ago, and we envisioned that "modules" would cover node's "scripts" as well.

• Is the "modules" functionality used by any debug adapter?
• Did someone try to use "modules" for "loaded scripts"? Did it fail?
• Can we improve "modules" to support scripts as well?
• Are "modules" and "scripts" independent concepts that can be used at the same time and therefore need independent protocol requests?

I just want to avoid that we introduce another very specific protocol complex "scripts" although we got already the "modules" complex for that.

/cc @dlebu @andrewcrawley @richardstanton @gregg-miskelly @DavidKarlas

[andrewcrawley]

Thanks for the clarification!

As you note, "Scripts" and "Modules" are similar concepts. My original thought (see comment 2) was to make a script be a kind of module, but we ended up leaning towards making "scripts" a separate concept because of several semantic differences:

• In VS, scripts are represented in a hierarchy (e.g. "app.js" has "app.ts" as a child, as mentioned above), but modules are treated as a simple list.
• A script represents a document that can be opened, and a module doesn't necessarily.
• There was no overlap between script-relevant fields and module-relevant fields on the Module object.

To answer your specific questions:

• Is the "modules" functionality used by any debug adapter?
  • Yes, the VsDbg engine used in both VS and VSCode implements the existing protocol "modules" functionality for debugging C++ and C#, and the VS debug adapter host uses module events to populate VS's "Modules" window.
• Did someone try to use "modules" for "loaded scripts"? Did it fail?
  • We considered this approach, but were concerned about the differences I mentioned above. We never got as far as actually implementing it.
• Can we improve "modules" to support scripts as well?
  • This could definitely be done - see comment 2 for my initial proposal, although I didn't have a good idea about how to best handle hierarchical representation.
• Are "modules" and "scripts" independent concepts that can be used at the same time and therefore need independent protocol requests?
  • They can technically be used at the same time, although I'm not aware of any debuggers that do this. Overloading "Module" to represent both would still work as long as we could tell the difference between a module and a script, since VS displays them in separate places.

[weinand]

Could we try to avoid introducing the Script type as a wrapper for a Source?
All breakpoint, location and content retrieval functionality is based on Source objects.

If we introduce a new Script type, we run into the asymmetry that you can set breakpoints in a Script but that when hitting those breakpoints the corresponding stack frame does not refer to the Script but just to the Source. So in order to find the Script of a stack frame, we have to match the Source within all Scripts.

One way to improve this would be by making a Script a subclass of Source.

But if we look closer at the two additional attributes id and children, I wonder whether they are really needed or where they are needed:

• id: why do we need another id on top of the already existing id-like attributes sourceReference and path of Source?
• children: I think this information could live directly on the Source because it makes always sense to have the "originating source" available for a given script or file.

So my proposal would be to fold Scriptand Source into the existing type Source, eliminate the more or less redundant id and introduce a new attribute for linked resources. I would not call this "children" because that's somewhat misleading as this attribute is not used to structure the sources into a hierarchy. I propose to call this "sources" because that expresses the semantics that we want to convey.

Alternatively (to "sources") we could introduce a dictionary "related" where the key denotes the type of relation (e.g. "sources") and the value is the set of linked sources. But since I'm not aware of a convincing example for another type of "related" sources, I think we do not need this now.

Here is the modified proposal:

export interface Source {

    //.... existing attributes

    /** An optional list of sources that are related to the main source.
        These may be the source that generated the main source.
    */
    sources?: Source[];
}

/** Event message for 'loadedSource' event type.
    The event indicates that some source has been added or removed from the set of all loaded sources.
*/
export interface LoadedSourceEvent extends Event {
    // event: 'loadedSource';
    body: {
        /** The reason for the event. */
        reason: 'new' | 'removed';
        /** The new or removed source. */
        source: Source;
    };
}

export interface Capabilities {
    // ...
    /** The debug adapter supports the 'loadedSources' request. */
    supportsLoadedSourcesRequest?: boolean;
    // ...
}

/** Retrieves the set of all sources currently loaded by the debugged process. */
export interface LoadedSourcesRequest extends Request {
    // command: 'loadedSources';
    arguments: LoadedSourcesArguments;
}

/** Arguments for 'loadedSources' request.
    The 'loadedSources' request has no standardized arguments.
*/
export interface LoadedSourcesArguments {
}

/** Response to 'loadedSources' request. */
export interface LoadedSourcesResponse extends Response {
    body?: {
        /** Set of loaded sources. */
        sources: Source[];
    };
}

@dlebu @andrewcrawley @richardstanton @roblourens @gregg-miskelly
Is this OK with you?

[andrewcrawley]

Yeah, our intent with the Script type was just to provide a Source along with additional context that was relevant in the context of the event. If you want to put that stuff directly on Source, then we don't need it.

The purpose of the "id" property was basically just to make it easier to figure out what script is being unloaded in a loadedSource event with reason=removed, to avoid having to compare Source objects directly.

I'm OK with your proposal. @richardstanton, any thoughts?