Repository navigation
Debug Protocol: Support more flexible handling of exceptions #64
Copy link
Copy link
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)
Reactions are currently unavailable
Activity
gregg-miskelly commentedon Sep 22, 2016
We need a
thrownOrUserUnhandledvalue. Otherwise LGTM.jacdavis commentedon Sep 22, 2016
What does the "Debug adapter when one of the listed exceptions is thrown" comment mean? Should it be "Debug adapter behavior"?
jacdavis commentedon Sep 22, 2016
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 commentedon Sep 22, 2016
Except for those two comments, LGTM
gregg-miskelly commentedon Sep 22, 2016
Jackson Davis (@jacdavis) it would be in the
idfield. I would propose having VS useEXCEPTION_CODE_DISPLAY_IN_HEXbit 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 commentedon Sep 22, 2016
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.
gregg-miskelly commentedon Sep 22, 2016
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.
andrewcrawley commentedon Oct 11, 2016
We'll verify this proposal against our scenarios and send a PR.
weinand commentedon Oct 13, 2016
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 commentedon Nov 4, 2016
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
exceptionDetailsrequest (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: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
exceptionBehaviorswhich returns an array of 'ExceptionBehavior':The 'official' way to get the list of all configurable exceptions is via the
exceptionDetailsrequest. A capabilitysupportsExceptionDetailsRequestannounces that the DA implements the request.For configuration of the exception behavior I suggest that we use the existing
setExceptionBreakpointsrequest 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 theexceptionId.andrewcrawley commentedon Nov 5, 2016
I think this looks reasonable, for the most part.
nameanddefaultBehaviorIdonExceptionDetailsshould 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
SetExceptionBreakpontsmechanism, 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.SetExceptionBreakpointswould 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:
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.jsonto 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 commentedon Nov 5, 2016
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.
weinand commentedon Nov 7, 2016
Gregg Miskelly (@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 Andrew Crawley (@andrewcrawley)'s concerns about how the list of exceptions is retrieved.3 remaining items
weinand commentedon Nov 14, 2016
Reopening after another round of discussions (e.g. #11552).
weinand commentedon Nov 15, 2016
My summary of the discussions we had so far:
weinand commentedon Nov 16, 2016
Andrew Crawley (@andrewcrawley) here is a new proposal:
API using glob pattern syntax
A simple
setExceptionBreakpointsAPI 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).Here are five glob patterns packed into a single
SetExceptionBreakpointsArguments:(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:
"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:
With this the 5 filters from above would look like this:
I'm leaning more towards the second proposal because it simplifies the implementation in debug adapters.
tzwlai commentedon Nov 17, 2016
FYI Andrew Wang (@WardenGnaw) Rajkumar Janakiraman (@rajkumar42) Wes Rupert (@wesrupert)
wesrupert commentedon Nov 17, 2016
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 commentedon Nov 17, 2016
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.
weinand commentedon Nov 22, 2016
Andrew Crawley (@andrewcrawley) Wes Rupert (@wesrupert) Andrew Wang (@WardenGnaw) Rajkumar Janakiraman (@rajkumar42) here is the complete proposal (see PR #88):
add ExceptionOptions; fixes #64
weinand commentedon Nov 23, 2016
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:
Break on every exception in the '.NET Exceptions' category:
Break on every exception in the '.NET Exceptions' or 'Win32 Exceptions' categories:
Break on 'System.InvalidOperationException' in category '.NET Exceptions':
Break on every exception in category '.NET Exceptions' but not "System.InvalidOperationException":
Break on all non user handled '.NET Exceptions' with the exception of "System.InvalidOperationException" for which to always break on:
gregg-miskelly commentedon Nov 23, 2016
LGTM. BTW: andrewcrawley is on vacation this week.
weinand commentedon Nov 30, 2016
I'm planning to merge #88 tomorrow.
Merge pull request #88 from Microsoft/andre/exception_config