Add tooltips to exception categories

[andersea]

The options for debugging exceptions are very confusion. There are now three different options, with some combinations seeming to not make sense.

I would very much like to see, not only thorough documentation, with examples, on each individual option, but also their expected interactions. With three options to chose from, there are now 8(!) possible combinations for how these settings could be picked. Probably most of these don't make sense.

UI wise, VSCode probably is lacking here, as it could be presented better, but good documentation is a first step.

I am seeing a lot of weird behaviour, when debugging exceptions. For instance, as far as I can see an old bug from ptvsd #2027 has regressed in debugpy (at least for async trio code), but it is hard for me to test and report anything, when I have no clue what the expected behaviour is, under the various conditions that are available.

[andersea]

See also #364 for similar concerns. Actually I believe that that issue has regressed now for all trio code, but again, hard to know, when I don't know what to expect.

[fabioz]

• Raised Exceptions:
  Triggers whenever any exception is raised or reraised (if "justMyCode":true -- which is the default -- only triggers when an exception is raised or reraised in user code).

• Uncaught Exceptions:
  Triggers only when an exception would get to the entry point method of a thread (so, only if an exception would make a thread exit).

• User Uncaught Exceptions:
  Triggers when a raised/reraised exception would cross the barrier from user code to library code (so, when an exception is raised or reraised it checks whether the caller is library code and the current method is in user code, if it is, then the exception is shown -- in general it's helpful if a user wants to see exceptions which are being raised in a test case and would end up being caught inside the test framework -- it can be helpful to uncover bugs in other situations too, but it may be hard to use if in your usage you do get such exceptions regularly as a part of a framework you're using).

All modes are kind of independent (so, enabling one shouldn't change the behavior of the other, although raised exceptions with "justMyCode":false is kind of a superset of the others).

@karthiknadig @int19h maybe something like that could be added to: https://code.visualstudio.com/docs/python/debugging.

[andersea]

Thanks for taking a look at this. I am sorry if this seems nitpicky, but I really think it would be helpful to be precise on these points. Maybe we can go through these one at a time, to make sure I completely understand and that we are in agreement?

Raised Exceptions

Precondition:
justMyCode: true
Uncaught exceptions unchecked
User uncaught exception unchecked

Expectation:
You say that the exception must be raised in user code.

Lets say I have a third party library (bad_sync_lib) with this test function:

def bad_func():
    print('Gone bad')
    time.sleep(0.5)
    raise RuntimeError('Bad!')

In my code I have this function:

from bad_sync_lib import bad_func

print('Test bad function.')
# Since the exception is not raised in user code, but in library code, the debugger should let this error propagate, as per your explanation
# In other words, with justMyCode set to true. This error should simply halt the program with the RuntimeError exception printed in the console
bad_func()

Actual:

The debugger stops the code on the bad_func() line.
This is actually what my expectation would be, but not your exact phrasing.

---

Precondition:
justMyCode: false
Uncaught exceptions unchecked
User uncaught exception unchecked

Expectation:
Take the same example code. In this case i think the expectation is, and this is consistent with your description, that the debugger should halt inside the third party code on the line where RuntimeError is raised.

Actual:
This works as expected.

[fabioz]

Actually, in that case it only stops at your code when the exception is reraised there.

-- if that's not the case then either it's a bug or the library code is being considered user code.

[fabioz]

i.e.: in that case it seems to work for me (it shows the error in the __main__ file, not on the library file):

[andersea]

The third party library is in a different project, which is build as a wheel file with poetry build. The wheel file is then added to the test project using poetry add <path-to-wheel> on the command line.

Can you please explain what you mean by 'reraise'. To me reraise is a pattern like this:

try:
    bad_func()
except RuntimeError:
    print('Caught RuntimeError')
    # Reraise it
    raise

[fabioz]

Yes, that'd be an explicit reraise, but if you don't have any try..except, then python reraises it by default (i.e.: any method without a try..except block will always reraise the exception).

[andersea]

Ok? So as designed, the debugger will halt on the same exception through the entire stack frame?

I am seeing this behaviour, and let me say I find it most surprising. My expectation would be, that the debugger only halts the first time a particular exception is raised (or reraised and you have the ability to see it). I don't see how it would be useful to see the same exception raised, again and again through the entire stack frame?

This also explains the confusion about the second bit "if "justMyCode":true -- which is the default -- only triggers when an exception is raised or reraised in user code": In my example, if justMyCode is true, the reason I see the exception at all, is that it is reraised at that point.

So in conclusion on the first issue, I see that the way you have explained it, is indeed how it behaves, however it leaves room for improvement, from my perspective, in that I think once a particular exception has been shown once, it shouldn't be shown again. As it currently works, the debugger halts again and again out through the entire python interpreter, which tells me nothing. I already know what I need to know, from the first time I saw the error. If I press play I shouldn't see that error again. Of cause, if the error causes a different error to be raised elsewhere, it would be fine to see that, but I think the debugger needs to keep track of, which errors the user has seen and hasn't seen.

[int19h]

Keep in mind that we're significantly constrained by the design of the Python runtime wrt exceptions here.

In most languages these days, exception handling is "two-pass" - that is, when exception is raised, first the stack is walked to determine which handler, if any, matches it, and then the stack is unwound to that handler. This makes it easy to determine whether the exception is unhandled or not at the point where it's raised, and whether the handler is in user code.

In Python, however, exceptions are more like an open union type with regular values - when an exception is raised, semantically it's more like the function returns a special value to the caller to signal it. If the caller is currently in a try-block, it'll jump to the corresponding handler; if not, it "returns" that exception to its own caller etc. Which means that it's not actually possible to reliably tell whether a given exception is handled or not, and where it is handled, at the point where it's raised - the debugger actually has to look at the bytecode of all the callers on the stack to try to figure it out, and even then that breaks as soon as you have any native frames on the stack. It also means that exception propagating through the stack looks the same as the one that was freshly raised.

This might change in the future if python/peps#2070 gets adopted, since it specifically distinguishes between raising and unwinding.

[andersea]

I dont understand. Each exception instance has a unique hash. Can't you just store the hash when you stop at the exception and if this exception is raised again, ignore it? Seems like it should be straightforward?

Anyway, we are getting sidetracked. It would be a great step forward, if the suggested documentation improvements were added. There is still the issue of how these different options interact. I am guessing that when an exception is raised, it is checked for each enabled option and if one of them matches, the debugger stops? I am guessing that this is what is meant by them being independent?

From a user perspective, I don't think it is that simple. For example. If you select 'Raised Exceptions', as far as I can tell it doesn't matter if any of the other options are selected or not, because the 'Raised Exceptions' option would trigger in any case where the other options would. Looking at it that way, it does seem to me, that there can be a conceptual interaction between the options, even though it is not the case in the actual implementation.

For me, it would help a lot, if this was explained further in the documentation.

[fabioz]

As a note, pydevd actually has the option available to stop only on the first raise (https://github.com/microsoft/debugpy/blob/main/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command_json.py#L915) but in debugpy this is always False right now because of how it interacts with justMyCode (i.e.: it wouldn't be reported on a reraise, only on the first raise if it was raised, so, if it was raised in library code with justMyCode:true, it wouldn't be shown) -- I guess we could do a variation of that which could use some heuristics to determine if it's the first raise in user code when justMyCode is true (that should be doable by inspecting the exception stack trace) -- pydevd actually provides a feature to report either on the first raise or only when the exception is raised and is uncaught from that initial call (so, an exception raised inside a try..except which is handled wouldn't be reported).

I dont understand. Each exception instance has a unique hash. Can't you just store the hash when you stop at the exception and if this exception is raised again, ignore it? Seems like it should be straightforward?

Sort of... unique hashes are kind of tricky in that they are only valid while the object they refer to is alive as the same hash can be reused (the way that Python works, reusing a hash is actually pretty common), so, just knowing about the hash isn't enough -- i.e.: it'd almost always work except when a hash is reused by a new exception. Doing a strong ref to the exception isn't really an option (the debugger shouldn't keep objects alive) and a weak-ref to the exception may not work (anyways, inspecting the exception stack to determine if this would be the first place it should be shown is probably enough to implement that feature).

Anyway, we are getting sidetracked. It would be a great step forward, if the suggested documentation improvements were added. There is still the issue of how these different options interact. I am guessing that when an exception is raised, it is checked for each enabled option and if one of them matches, the debugger stops? I am guessing that this is what is meant by them being independent?

Sort of... User Uncaught Exceptions and Raised Exceptions are handled in the same place, so, whenever any one matches, it'll be shown (in a way, there's not much to interact, it's just an additional if which checks if the exception should be shown). As for Unhandled Exceptions it is completely separate, so, the same exception could be shown twice, one for the part where it's raised and one where it makes the thread terminate.

@int19h To sum up the part related to showing exceptions just on the first raise, it should be doable, but may need some thought on the UI side of things too... pydevd actually provides more features related to exception breakpoints which we could provide in debugpy. I'm posting a screenshot below showing which features are available in pydevd right now (in particular, we may want to provide the same customization related to the Suspend on caught exceptions as well as which exceptions we should stop in the future, but the UI using the DAP makes things a bit trickier here -- i.e.: the VSCode UI is much simpler for users, so, that's a plus, but if the user wants to customize things in a certain way we don't really make it possible right now... maybe having some of those customized by environment variables could be an option initially so we wouldn't have to worry about the UI).

[andersea]

Hi, just wanted to say thanks for the fantastic and constructive input. I hope some improvements will end up in the documentation eventually. I have nothing to add at this moment.

[int19h]

Yes, it all boils down to the UI. Full VS also has many more options for exceptions (although it does conflate "user-unhandled" with Just My Code, basically). VSCode is very basic probably because the baseline was JS, where exception handling is very basic itself.

At the minimum, we need some way to add tooltips to exception categories, so that "user-uncaught" could be properly explained. Putting it in the docs is also something that needs to happen - so let's keep this issue open - but docs alone aren't great for discoverability.

[int19h]

Turns out that DAP already has the requisite properties to add tooltips, so: #798

[gramster]

I've added Fabio's initial explanation to the FAQ in the Wiki.

[int19h]

Resolved by #798