More control on breakpointing with userUnhandled: unexpected breakpoint in framework codes

wookayin commented 2 years ago

Before creating a new issue, please check the FAQ to see if your question is answered there.

I checked the FAQ which states the behavior of "User Uncaught Exceptions" (userUnhandled).

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

This is useful for catching exceptions for python unit tests (#722, #550, #392 etc.) but I feel we need more control and custom conditions for the exception breakpoints to be engaged. Especially in the case of Inversion of Control, breakpoints can be excessive. Here are motivating examples:

Example

Steps to reproduce:

# Use code
from typing import Sequence

class MyList(Sequence[int]):    

  def __len__(self):
     return 5
  def __getitem__(self, i: int):
    if i < 5:
      return i
    else:
     raise IndexError(i)  # <--------- raises IndexError if i >= len(self._list)  and debugpy breaks here when userUnhandled

def test_hello():
  s = [i for i in MyList()]
  assert s == [0, 1, 2, 3, 4]

debugpy version: 1.6.3

Actual behavior

This test case would pass and list(MyList()) has no problems. However, with the userUnhandled exeptionBreakpoints mode turned on, a breakpoint will be set on the raise IndexError line. This is because the caller of MyList.__getitem__ is a non-user, system library code at https://github.com/python/cpython/blob/main/Lib/_collections_abc.py#L1015-L1023:

# module: _collections_abc

    def __iter__(self):
        i = 0
        try:
            while True:
                v = self[i]   # <------------- calls __getitem__
                yield v
                i += 1
        except IndexError:  # <-------- suppress the exception
            return

which works "as expected" and intended -- userUnhandled is supposed to pause at an exception when the exception was thrown from an user code and it was catched in library code. However, users would usually do NOT want such exceptions would hit a breakpoint --- it is a very common pattern to catch an user exception regularly, when inversion of control is used where user code would run on top of the framework code (also mentioned in the FAQ).

Expected behavior

debugpy does NOT break on such IndexErrors.

Another Example

def test_foo(self):
  with pytest.raises(SomeUserExpectedError):
      _an_user_code_raises_exception()  # <--  will always hit the breakpoint
  OK_the_test_is_successful()

Suggested Features

To my knowledge, such exception breakpoint options were come to birth to run a debugger on assertion failures in unit tests (e.g., https://github.com/emacs-lsp/dap-mode/issues/506, https://github.com/nvim-neotest/neotest-python/issues/23, etc.) What is desired is to have the breakpoint set when it is captured by a specified framework or package, e.g., pytest.

Therefore, such exception breakpoints could be "conditional" -- for example, on the package/module (regex) of the caller (either includelist or excludeist), or via some arbitrary conditions that user can specify through a DAP configuration.

Other References

63c0faee415e32bbeb526f04740e784aa79acbeb for the (initial) implementation of userUnhandled.

fabioz commented 2 years ago

I'm not sure about this... rather than making it configurable and having clients then have to configure in a different way for each framework, maybe we could just provide an API and then clients could use that API to stop at the proper place?

i.e.: in this case it's pytest, but each different framework -- say nose, unittest, django, etc -- would need a different customization which may not be straightforward to get right.

As a note, there's already an open feature request which is tracking this:

https://github.com/microsoft/debugpy/issues/723

The comment: https://github.com/microsoft/debugpy/issues/723#issuecomment-919089064 has some code which explains how to ask the debugger to stop at a given traceback frame using internal APIs (it can be used until a better public API is provided).

@wookayin do you think that would suffice?

wookayin commented 2 years ago

clients then have to configure in a different way for each framework, maybe we could just provide an API and then clients could use that API to stop at the proper place?

I agree. I proposed this feature request as an alternative to #722 as #722 was closed with a suggestion to use userUnhandled, but I think #723 is probably the right way to implement the post-mortem mode. Although userUnhandled with custom filter expression might be useful in rare cases, most of the practical use cases could be covered with #723 once a public API is introduced.

Let me close this as unplanned for now, but if anyone would think this could be useful in some other use cases I'd be happy to discuss again.

microsoft / debugpy