Debug Protocol: Support more flexible handling of exceptions

[andrewcrawley]

We'd like to be able to support the VS exception experience, where a list of exceptions is presented to the user, who can set various properties to control which exceptions will cause the debugger to break.

Since the number of exceptions supported in this way is large (the default list in VS contains ~500 exceptions for C# and ~230 for Java) and the list does not change from session to session, we propose that the list of exceptions would be maintained in a separate file rather than provided by the debug adapter in its Capabilities. The schema of this file would look like:

/** Represents a category of exceptions supported by a debug adapter */
export interface ExceptionCategory {
    /** Name of the category */
    name: string;
    /** List of exceptions in this category */
    exceptions: ExceptionInfo[];
}

export interface ExceptionInfo {
    /** Name of the exception. */
    name: string;
    /** Default state of the exception. */
    defaultMode?: ExceptionBreakMode;
    /** Internal identifier for the exception type.  If not specified, the name will be used. */
    id?: string;
}

export enum ExceptionBreakMode {
    /** Debug adapter should not break when this exception is thrown */
    disabled = 0,
    /** Debug adapter should break if the exception is unhandled */
    unhandled = 1,
    /** Debug adapter should break if the exception is not handled by user code */
    userUnhandled = 2,
    /** Debug adapter should break when the exception is thrown */
    thrown = 4
}

Proposed protocol changes:

export interface Capabilities {
    // ...
    /** The debug adapter supports the modifyExceptionStateRequest. */
    supportsModifyExceptionStateRequest?: boolean;
    // ...
}

/** ModifyExceptionState request; value of command field is 'modifyExceptionState'.  
        The request modifies the debug adapter's response to exceptions that are thrown.
        When an exception is thrown, the debug adapter will stop with a StoppedEvent (event type 'exception'). */
export interface ModifyExceptionStateRequest extends Request {
    arguments: ModifyExceptionStateArguments;
}

/** Arguments for the 'modifyExceptionState' request. */
export interface ModifyExceptionStateArguments {
    /** Categories of exceptions for which state will be modified */
    categories: ExceptionCategoryState[];
}

/** Response to 'modifyExceptionState' request.  This is just an acknowledgement, so no body field is required. */
export interface ModifyExceptionStateResponse extends Response {
}

/** Represents the state of a category of exceptions. */
export interface ExceptionCategoryState {
    /** Name of the category. */
    name: string;
    /** Debug adapter behavior when an exception in this category is thrown. */
    mode?: ExceptionBreakMode;
    /** Specific exceptions in this category to modify.  If empty, the mode specified will apply to all exceptions in this category. */
    exceptions?: ExceptionState[];
}

/** Represents the state of a list of exceptions in a single category. */
export interface ExceptionState {
    /** Set of exceptions to modify. */
    id: string[];
    /** Debug adapter when one of the listed exceptions is thrown.  If not specified, the behavior of the category to which the exception belongs will be used. */
    mode?: ExceptionBreakMode;
}

@jacdavis @gregg-miskelly @richardstanton @tzwlai

[gregg-miskelly]

We need a thrownOrUserUnhandled value. Otherwise LGTM.

[jacdavis]

What does the "Debug adapter when one of the listed exceptions is thrown" comment mean? Should it be "Debug adapter behavior"?

[jacdavis]

So, the proposal is to place the code based exceptions (like win32 exceptions) inside the name? Are we planning on the debug adapter just knowing the numerical format? (In vs, it is hex for javascript, native prefixed by 0x).

[jacdavis]

Except for those two comments, LGTM

[gregg-miskelly]

@jacdavis it would be in the id field. I would propose having VS use EXCEPTION_CODE_DISPLAY_IN_HEX bit for the exception category in the VS exception metrics to decide if it should format the code in hex (with a '0x' prefix) or decimal.

[andrewcrawley]

@gregg-miskelly - My intent was that the ExceptionBreakMode enum would be used as flags, so you could do "thrown | userUnhandled". Is this not what you meant?

@jacdavis - Typo. : \ You're correct as to what it should say.

[gregg-miskelly]

@andrewcrawley: if you can do that with the json schema - sounds great to me. I would have thought that wouldn't work there, but happy to be wrong if that works.

[andrewcrawley]

We'll verify this proposal against our scenarios and send a PR.

[weinand]

This protocol addition is most likely something that VS Code will provide UI for. So I need some more time before I can provide feedback.

[weinand]

@andrewcrawley here is my feedback and a modified proposal that tries to be more general and would work with a (not yet existing) VS Code exception configuration UI.

The debug protocol cannot prescribe a file based persistency mechanism or a file format. So if a client and/or a debug adapter want to use a file based approach, they could introduce a private protocol for this. E.g. the debug adapter could return the location of the file in the exceptionDetails request (see below) if it detects a UI client that supports this (and the DA would return an empty exceptions array in this case).

The original proposal is very specific about how exceptions are grouped in categories, resulting in a two level hierarchy. I could easily imagine that other debuggers only need a flat list without categories or a multi level hierarchy if the class hierarchy of exception classes is reflected.
For this reason I propose a recursive type ExceptionDetails:

    /** An ExceptionDetails is shown in the UI as an option for configuring an exception or a group of exceptions. */
    export interface ExceptionDetails {
        /** The internal ID of the exception. This value is for configuring the exception in the setExceptionBreakpoints request. */
        id: string;
        /** The name is shown in the UI either as the name of an exception or the name of a group of exceptions. */
        name: string;
        /** Id for the default behavior of the exception. See type 'ExceptionBehavior'.*/
        defaultBehaviorId: string;

        /** Optional set of sub exceptions. */
        exceptions?: ExceptionDetails[];
    }

Since the value set of the original 'ExceptionBreakMode' is language/runtime specific, we need a way to communicate the individual values to the UI. I suggest that we introduce a capability exceptionBehaviors which returns an array of 'ExceptionBehavior':

    export interface Capabilities {
        // ...
        /** Available exception break behaviors for the setExceptionBreakpoints request. */
        exceptionBehaviors?: ExceptionBehavior[];
        // ...
    }

    export interface ExceptionBehavior {
        /** The internal ID of the behavior. */
        id: string;
        /** The name of the behavior. This will be shown in the UI. */
        name: string;
    }

The 'official' way to get the list of all configurable exceptions is via the exceptionDetails request. A capability supportsExceptionDetailsRequest announces that the DA implements the request.

    export interface Capabilities {
        // ...
        /** The debug adapter supports the exceptionDetailsRequest. */
        supportsExceptionDetailsRequest?: boolean;
        // ...
    }

    /** Details of all configurable exceptions can be retrieved with the ExceptionDetailsRequest. */
    export interface ExceptionDetailsRequest extends Request {
        // command: 'exceptionDetails';
        arguments: ExceptionDetailsArguments;
    }
    /** Arguments for 'exceptionDetails' request. */
    export interface ExceptionDetailsArguments {
    }
    /** Response to 'exceptionDetails' request. */
    export interface ExceptionDetailsResponse extends Response {
        body: {
            /** All exception details. */
            exceptions: ExceptionDetails[];
        };
    }

For configuration of the exception behavior I suggest that we use the existing setExceptionBreakpoints request and add an optional attribute for the new approach. 'Categories' or the exception hierarchy is not surfaced in this API because the assumption is that this information can always be inferred from the exceptionId.

    /** Arguments for the 'setExceptionBreakpoints' request. */
    export interface SetExceptionBreakpointsArguments {
        // ... old 'filters' attribute

        /** Exception Ids and their mode. */
        exceptions?: ExceptionConfiguration[];
    }

    /** Represents the configuration of a single exception. */
    export interface ExceptionConfiguration {
        /** Id of the exception. */
        exceptionId: string;
        /** Id of the exception behavior when exception is thrown. */
        behaviorId: string;
    }

[andrewcrawley]

I think this looks reasonable, for the most part. name and defaultBehaviorId on ExceptionDetails should probably be nullable, because in many cases the ID will be usable as a name (e.g. "System.ArgumentException"), and most exceptions are ignored by default. VS only supports a single level of categorization, but we can flatten the list we get if necessary.

I'm concerned with the chattiness of the SetExceptionBreakponts mechanism, which is why my original proposal had a separate mechanism for allowing exceptions / categories to be enabled and disabled without having to re-send the entire list. SetExceptionBreakpoints would allow enabling all exceptions in a category with a short request, but if I then disable a single exception in that category, I'd have to send the full list. For C#, that single message would be almost 40KB!

Retrieving the list of exceptions via the protocol is problematic for a couple of reasons. Off the top of my head:

• VS requires the list of exceptions to be in the registry. This is done as part of the installation of a debug engine - there's no way to do it dynamically.
• Sending the exceptions via the protocol means that the UI won't be able to show the list of exceptions until the debug adapter has been run at least once. That seems awkward from a user perspective - I can set breakpoints before I'm actively debugging, why could I not set my exception settings?

As you say, we can come up with a private contact for a file-based mechanism, but in practice, any extension that wants to support exceptions in VS is going to have to support that private contract, so it might as well be standardized. I think the best option would be for an extension's package.json to point to a JSON file defining the exceptions, just like it points to the debug adapter itself. This avoids the "first run" issue and gives us something we can consume on install to populate our reg keys.

[gregg-miskelly]

@weinand I am not sure you are correctly understanding what is meant by 'categories'. Categories are things like 'JavaScript exceptions', 'C++ exceptions', '.NET Framework exceptions', etc. Therefore, all debug adapters that support exceptions at all, can support categories.

We used to have a UI that supported a full tree. We found that this made the UI harder to use, so when we rewrote it for VS 2015 we eliminated the flexible hierarchy in favor of the simple one level tree with categories at the top.

[weinand]

@gregg-miskelly I think I understand the concept "Categories":

I'm just trying to find an API that supports debuggers/runtimes that do not have a concept of categories or even need a deeper hierarchy.

A quick 'survey' showed this:

Xamarin:

Eclipse:

xCode:

So a two level hierarchy does not seem to be a widespread concept (but a many-level hierarchy isn't either). My proposal is a pay-as-you-go approach that supports 1, 2, and n levels (including the category/exception separation of VS).

But before we can finalise the shape of the ExceptionDetails, I'll try to respond to @andrewcrawley's concerns about how the list of exceptions is retrieved.

[gregg-miskelly]

Right, I understand that not all debugger UIs might have a category concept, but it isn't hard for a debug adapter to support this concept (if (category != myExceptions) return). I think it is rather fundamental to having any sort of exception configuration UI because otherwise all debug adapters will need to understand exception settings that every other debug adapter added). I definitely don't see a problem with supporting nested exception settings, but it does make the UI more complicated. So I will leave that one up to you.

[weinand]

After discussing this in the weekly planning call, we came to the conclusion that this protocol is too specific to be added to the VS Code debug protocol at the moment.

[weinand]

Reopening after another round of discussions (e.g. #11552).

[weinand]

My summary of the discussions we had so far:

• It is difficult to retrieve the full set of exceptions dynamically from the debug adapter. Instead this set might come from a language server, a static file contributed in the debug extension etc. For this reason we will not provide 'getExceptions' API in the debug protocol.
• A consequence of this is that we cannot use opaque IDs between a 'getExceptions' and a 'setExceptions' API of the adapter. Instead the API will use the user visible names, e.g. exception name or class names. So a minimalistic generic UI to configure exceptions could be a text input box where the user can enter the class name of an exception and the debug adapter will receive these names in the 'setExceptions' API.
• Another requirement is that exceptions are grouped in categories and that either individual exceptions or the whole category can be configured. In addition to that in some languages or debuggers exceptions are forming a tree. Here again it should be possible to configure either exceptions individually or a whole subtree. Combining both requirements suggests that categories and exception names can be expressed as a tree hierarchy and every node or leaf in that tree can be addressed by a path.
• Since the UI for configuring exceptions might be available even if the debug adapter is not running, the UI cannot rely on an incremental API of the debug adapter. Instead the exception UI would work independently from the debug adapter and would transfer the full exception state when the adapters gets initialised. For this the 'setExceptions' API should support a bulk mode where a full configuration can be passed in one request. If possible this API should be the only API, so that there is no need for an incremental API.
• To keep the amount of transmitted data small, the API needs an efficient way to express configurations that would typically result in big data sets when using a naive approach. E.g. a single excluded exception of a category should not be expressed by enumerating all but one of the included exceptions under that category.

[weinand]

@andrewcrawley here is a new proposal:

API using glob pattern syntax

A simple setExceptionBreakpoints API that obeys the 5 bullet items from above could be based on category/exception paths and a subset of glob patterns (see https://www.npmjs.com/package/minimatch).

/** Arguments for the 'setExceptionBreakpoints' request. */
export interface SetExceptionBreakpointsArguments {
    /** Categories or exceptions for which state will be modified */
    filters?: FilterState[];
}

export interface FilterState {
    pattern?: string;
    mode: ExceptionBreakMode;
}

Here are five glob patterns packed into a single SetExceptionBreakpointsArguments:

"filters": [
    { // every exception
        pattern: "**",
        mode: ExceptionBreakMode.thrown
    },
    { // every exception in 'cat1'
        pattern: "cat1/**",
        mode: ExceptionBreakMode.thrown
    },
    { // every exception in 'cat1' or 'cat2'
        pattern: "{cat1,cat2}/**",
        mode: ExceptionBreakMode.thrown
    },
    { // exception 'ex1' in 'cat1'
        pattern: "cat1/ex1/**",
        mode: ExceptionBreakMode.thrown
    },
    { // every exception but 'ex1' or 'ex3' in 'cat1'
        pattern: "cat1/!(ex1|ex3)/**",
        mode: ExceptionBreakMode.thrown
    }
]

(combining these particular patterns doesn't make sense because the first already includes all exceptions; I'm just using them here to show 5 useful patterns).

Nice characteristic of this approach:

• small size even for problematic configurations like the disabled single exception in a category (see the 40k scenario from above),
• compact syntax: a user could even enter these patterns without fancy UI,
• using patterns allows to select groups of exceptions based on pattern matching, e.g. "cat1/**/*Error*" matches all exceptions in category 'cat1' that contain the word 'Error'.

API without glob pattern syntax

If we want to avoid patterns and their parsing in the debug adapter we could model the filters by using these types:

export interface FilterState {
    path?: FilterPathSegment[];
    mode: ExceptionBreakMode;
}

export interface FilterPathSegment {
    negate?: boolean;   // use complement set of or'ed names
    names: string[];    // or'ed names
}

With this the 5 filters from above would look like this:

"filters": [
    { // every exception
        mode: ExceptionBreakMode.thrown
    },
    { // every exception in 'cat1'
        path: [
            { names: [ "cat1" ] }
        ],
        mode: ExceptionBreakMode.thrown
    },
    { // every exception in 'cat1' or 'cat2'
        path: [
            { names: [ "cat1", "cat2" ] }
        ],
        mode: ExceptionBreakMode.thrown
    },
    { // exception 'ex1' in 'cat1'
        path: [
            { names: [ "cat1" ] },
            { names: [ "ex1" ] }
        ],
        mode: ExceptionBreakMode.thrown
    },
    { // every exception but 'ex1' or 'ex3' in 'cat1'
        path: [
            { names: [ "cat1" ] },
            { negate: true, names: [ "ex1", "ex2" ] }
        ],
        mode: ExceptionBreakMode.thrown
    }
]

I'm leaning more towards the second proposal because it simplifies the implementation in debug adapters.

[tzwlai]

FYI @WardenGnaw @rajkumar42 @wesrupert

[wesrupert]

From a protocol standpoint, I think proposal 2 is more consistent, as it conveys the same information through the json structure. Similarly to how we don't pass breakpoints as { location: "path/to/file.type:23::verified" } and require the debug adapter to perform additional parsing, but as { source: { path: "path/to/file.type" }, line: 23, verified: true }.

[weinand]

@wesrupert yes, I started with proposal 1 because it was easier to grasp for someone who is familiar with glob patterns. But proposal 2 is less readable but more in line with the rest of the protocol.

[weinand]

@andrewcrawley @wesrupert @WardenGnaw @rajkumar42 here is the complete proposal (see PR #88):

/** Information about the capabilities of a debug adapter. */
export interface Capabilities {
	//...
	/** The debug adapter supports 'exceptionOptions' on the setExceptionBreakpoints request. */
	supportsExceptionOptions?: boolean;
	//...
}

/** SetExceptionBreakpoints request; value of command field is 'setExceptionBreakpoints'.
    The request configures the debuggers response to thrown exceptions.
    If an execption is configured to break, a StoppedEvent is fired (event type 'exception').
*/
export interface SetExceptionBreakpointsRequest extends Request {
	// command: 'setExceptionBreakpoints';
	arguments: SetExceptionBreakpointsArguments;
}

/** Arguments for 'setExceptionBreakpoints' request. */
export interface SetExceptionBreakpointsArguments {
	/** IDs of checked exception options. The set of IDs is returned via the 'exceptionBreakpointFilters' capability. */
	filters: string[];
	/** Configuration options for selected exceptions. */
	exceptionOptions?: ExceptionOptions[];
}

/** An ExceptionOptions assigns configuration options to a set of exceptions. */
export interface ExceptionOptions {
	/** A path that selects a single or multiple exceptions in a tree.
	    If 'path' is missing, the whole tree is selected.
	    By convention the first segment of the path is a category that is used to group exceptions in the UI. */
	path?: ExceptionPathSegment[];
	/** Condition when a thrown exception should result in a break. */
	breakMode: ExceptionBreakMode;
}

/** This enumeration defines all possible conditions when a thrown exception should result in a break.
    never: never breaks,
    always: always breaks,
    unhandled: breaks when excpetion unhandled,
    userUnhandled: breaks if the exception is not handled by user code.
*/
export type ExceptionBreakMode = 'never' | 'always' | 'unhandled' | 'userUnhandled';

/** An ExceptionPathSegment represents a segment in a path that is used to match leafs or nodes in a tree of exceptions.
    If a segment consists of more than one name, it matches the names provided if 'negate' is false or missing
    or it matches anything except the names provided if 'negate' is true. */
export interface ExceptionPathSegment {
	/** If false or missing this segment matches the names provided, otherwise it matches anything except the names provided. */
	negate?: boolean;
	/** Depending on the value of 'negate' the names that should match or not match. */
	names: string[];
}

[weinand]

@andrewcrawley @gregg-miskelly @jacdavis @wesrupert @WardenGnaw @rajkumar42
Some examples:

Break on every exception:

"exceptionOptions": [
  {
    breakMode: "always"
  }
]

Break on every exception in the '.NET Exceptions' category:

"exceptionOptions": [
  {
    path: [
      { names: [ ".NET Exceptions" ] }
    ],
    breakMode: "always"
  }
]

Break on every exception in the '.NET Exceptions' or 'Win32 Exceptions' categories:

"exceptionOptions": [
  {
    path: [
      { names: [ ".NET Exceptions", "Win32 Exceptions" ] }
    ],
    breakMode: "always"
  }
]

Break on 'System.InvalidOperationException' in category '.NET Exceptions':

"exceptionOptions": [
  {
    path: [
      { names: [ ".NET Exceptions" ] }
      { names: [ "System.InvalidOperationException" ] }
    ],
    breakMode: "always"
  }
]

Break on every exception in category '.NET Exceptions' but not "System.InvalidOperationException":

"exceptionOptions": [
  {
    path: [
      { names: [ ".NET Exceptions" ] }
      { negate: true, names: [ "System.InvalidOperationException" ] }
    ],
    breakMode: "always"
  }
]

Break on all non user handled '.NET Exceptions' with the exception of "System.InvalidOperationException" for which to always break on:

"exceptionOptions": [
  {
    path: [
      { names: [ ".NET Exceptions" ] }
    ],
    breakMode: "userUnhandled"
  },
  {
    path: [
      { names: [ ".NET Exceptions" ] }
      { names: [ "System.InvalidOperationException" ] }
    ],
    breakMode: "always"
  }
]

[gregg-miskelly]

LGTM. BTW: andrewcrawley is on vacation this week.

[weinand]

I'm planning to merge #88 tomorrow.