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.