Skip to content

Debug Protocol: Support more flexible handling of exceptions #64

Closed

Description

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;
}

Jackson Davis (@jacdavis) Gregg Miskelly (@gregg-miskelly) Richard Stanton (@richardstanton) Thomas Lai (@tzwlai)

Activity

  1. gregg-miskelly commented on Sep 22, 2016

    @gregg-miskelly
    Member

    We need a thrownOrUserUnhandled value. Otherwise LGTM.

  2. jacdavis commented on Sep 22, 2016

    @jacdavis

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

  3. jacdavis commented on Sep 22, 2016

    @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).

  4. jacdavis commented on Sep 22, 2016

    @jacdavis

    Except for those two comments, LGTM

  5. gregg-miskelly commented on Sep 22, 2016

    @gregg-miskelly
    Member

    Jackson Davis (@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.

  6. andrewcrawley commented on Sep 22, 2016

    @andrewcrawley
    ContributorAuthor

    Gregg Miskelly (@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?

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

  7. gregg-miskelly commented on Sep 22, 2016

    @gregg-miskelly
    Member

    Andrew Crawley (@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.

  8. andrewcrawley commented on Oct 11, 2016

    @andrewcrawley
    ContributorAuthor

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

  9. weinand commented on Oct 13, 2016

    @weinand
    Contributor

    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.

  10. weinand commented on Nov 4, 2016

    @weinand
    Contributor

    Andrew Crawley (@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;
        }
  11. andrewcrawley commented on Nov 5, 2016

    @andrewcrawley
    ContributorAuthor

    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.

  12. gregg-miskelly commented on Nov 5, 2016

    @gregg-miskelly
    Member

    Andre Weinand (@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.

  13. weinand commented on Nov 7, 2016

    @weinand
    Contributor

    Gregg Miskelly (@gregg-miskelly) I think I understand the concept "Categories":

    6355 toolwindow1_thumb_1429483b

    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:
    2016-11-07_14-48-38

    Eclipse:
    2016-11-07_16-21-33

    xCode:
    2016-11-07_16-56-00

    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 Andrew Crawley (@andrewcrawley)'s concerns about how the list of exceptions is retrieved.

  14. 3 remaining items

  15. weinand commented on Nov 14, 2016

    @weinand
    Contributor

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

  16. weinand commented on Nov 15, 2016

    @weinand
    Contributor

    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.
  17. weinand commented on Nov 16, 2016

    @weinand
    Contributor

    Andrew Crawley (@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.

  18. wesrupert commented on Nov 17, 2016

    @wesrupert
    Contributor

    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 }.

  19. weinand commented on Nov 17, 2016

    @weinand
    Contributor

    Wes Rupert (@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.

  20. weinand commented on Nov 22, 2016

    @weinand
    Contributor

    Andrew Crawley (@andrewcrawley) Wes Rupert (@wesrupert) Andrew Wang (@WardenGnaw) Rajkumar Janakiraman (@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[];
    }
  21. added a commit that references this issue on Nov 22, 2016
    853b125
  22. weinand commented on Nov 23, 2016

    @weinand
    Contributor

    Andrew Crawley (@andrewcrawley) Gregg Miskelly (@gregg-miskelly) Jackson Davis (@jacdavis) Wes Rupert (@wesrupert) Andrew Wang (@WardenGnaw) Rajkumar Janakiraman (@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"
      }
    ]
  23. gregg-miskelly commented on Nov 23, 2016

    @gregg-miskelly
    Member

    LGTM. BTW: andrewcrawley is on vacation this week.

  24. weinand commented on Nov 30, 2016

    @weinand
    Contributor

    I'm planning to merge #88 tomorrow.

  25. added a commit that references this issue on Dec 1, 2016
    0299af6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions