gh-106670: Allow Pdb to move between chained exceptions#106676

Carreau

Allow Pdb to move between chained exception.

This lets Pdb receive an exception, instead of a traceback, and when
this is the case and the exception are chained, the new exceptions command
allows to both list (no arguments) and move between the chained exceptions.

That is to say if you have something like

def out():
    try:
        middle()                                # B
    except Exception as e:
        raise ValueError("foo(): bar failed")   # A

def middle():
    try:
        return inner(0)                         # D
    except Exception as e:
        raise ValueError("Middle fail") from e  # C

def inner(x):
    1 / x                                       # E

Only A was reachable after calling out() and doing post mortem debug.

With this all A-E points are reachable with a combination of up/down,
and exceptions <number>.

This also change the default behavior of pdb.pm(), as well as
python -m pdb <script.py> to receive sys.last_exc so that chained
exception navigation is enabled.

We do follow the logic of the traceback module and handle the
_context__ and __cause__ in the same way. That is to say, we try
__cause__ first, and if not present chain with __context__. In
the same vein, if we encounter an exception that has
__suppress_context__ (like when raise ... from None), we do stop
walking the chain.

Some implementation notes:

We do handle cycle in exceptions
cleanup of references to tracebacks are not cleared in forget(), as
setup() and forget() are both for setting a single
exception.
We do not handle sub-exceptions of exception groups.
We ensure we do not hold references to exceptions too long with a new
context manager.
Have the MAX_CHAINED_EXCEPTION_DEPTH class variable to control the
maximum number we allow

Closes gh-106670

Carreau

Addressed some of the comments in the linked issue, rebased pushed, and updated description of this issue to reflect the codebase status.

gaogaotiantian

Left some comments. For this feature to be merged, you'll need to work on the docs as well (it's a new command). We need review from a core dev, especially on the interface. The main concern I have is about the conflict between backward-compatibility and readability:

def post_mortem(traceback=None):
    # We allow(actually prefer) the input traceback to be an Exception.

How should we deal with that? Changing the argument name traceback would break some backward compatibility, enforcing the argument to be an Exception would break a lot.

This is a more serious concern for public interfaces, but also on private methods, for example, Pdb.interaction takes a tb argument which can be an Exception now. This is not good for readability.

Could @iritkatriel or @brandtbucher share some thoughts on this? Thanks!

Carreau

How should we deal with that? Changing the argument name traceback would break some backward compatibility, enforcing the argument to be an Exception would break a lot.

I'm' not a particularly huge fan of the following but we can do:

def post_mortem(exc=None, /, *, traceback=None):
    # fail if both exc and traceback are given, 
    # deprecate if traceback is given, say it takes a single positional argument.
    # check is exc is a traceback and emit a FutureWarning to pass an exception ?

That way we only emit on use that use the keyword way, and this avoid using *args, **kwargs and custom hard to understand logic.

Then in N versions you can go to def post_mortem(exc,/). positional only for a function that takes a single parameter seem safe(r).

We can try to apply something similar to post_mortem(t=None) and for interaction we can use *args, **kwargs and roll some custom logic if we wish.

gaogaotiantian

I'm' not a particularly huge fan of the following but we can do:
def post_mortem(exc=None, /, *, traceback=None):
    # fail if both exc and traceback are given, 
    # deprecate if traceback is given, say it takes a single positional argument.
    # check is exc is a traceback and emit a FutureWarning to pass an exception ? 
That way we only emit on use that use the keyword way, and this avoid using *args, **kwargs and custom hard to understand logic.

Then in N versions you can go to def post_mortem(exc,/). positional only for a function that takes a single parameter seem safe(r).

We can try to apply something similar to post_mortem(t=None) and for interaction we can use *args, **kwargs and roll some custom logic if we wish.

This might be too complicated. I'm personally leaning to the more aggressive way - rename the argument to exc but allow passing trackbacks, which would break the code for the few who use keyword argument to pass the argument.

I did a preliminary search on github with post_mortem(traceback= language:Python and no real code shows up - maybe we won't impact any user at all. However I'm not the person who can make the decision here, we still need the approval from the core dev.

gaogaotiantian

I think the overall direction is great and we should have this feature in pdb. However, we do need opinions from core devs, especially for the backward compatibility part. I'll wait for a couple of days and reach out to core devs if no one volunteers :)

Carreau

Thanks for the feedback and patience, I'm back from taking some time off and updated to reflect your comment.

I'd appreciate if you can get a core dev review.

Carreau

Rebased on main (except last commit to allow to just see the changes since @iritkatriel helpful comment on __cause__.

I now to a breadth first listing of the context/cause tree of exceptions.

I've rephrased the first commit description as well as the PR description to mention __context__ and __cause__.

I think we could have a better UI to see the chain as a tree when doing exceptions, but I'm not too sure.

This lets Pdb receive an exception, instead of a traceback, and when this is the case and the exception are chained, the new `exceptions` command allows to both list (no arguments) and move between the chained exceptions. That is to say if you have something like def out(): try: middle() # B except Exception as e: raise ValueError("foo(): bar failed") # A def middle(): try: return inner(0) # D except Exception as e: raise ValueError("Middle fail") from e # C def inner(x): 1 / x # E Only A was reachable after calling `out()` and doing post mortem debug. With this all A-E points are reachable with a combination of up/down, and ``exception <number>``. This also change the default behavior of ``pdb.pm()``, as well as `python -m pdb <script.py>` to receive `sys.last_exc` so that chained exception navigation is enabled. We do follow the logic of the ``traceback`` module and handle the ``_context__`` and ``__cause__`` in the same way. That is to say, we try ``__cause__`` first, and if not present chain with ``__context__``. In the same vein, if we encounter an exception that has ``__suppress_context__`` (like when ``raise ... from None``), we do stop walking the chain. Some implementation notes: - We do handle cycle in exceptions - cleanup of references to tracebacks are not cleared in ``forget()``, as ``setup()`` and ``forget()`` are both for setting a single exception. - We do not handle sub-exceptions of exception groups. Closes pythongh-106670

Also move the release of the list of exception to a context manager for security

iritkatriel

LGTM, just need to resolve the warning in the news file.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood

@iritkatriel FYI the latest force-push automatically disabled auto-merge

Carreau

Thanks all for the reviews and the merge let me know if there is anything I can help with.

Closes ipython#13982 This is a "backport" of python/cpython#106676 See documentation there

Closes #13982 This is a "backport" of python/cpython#106676 See documentation there

The introduction of chained exception in pythongh-106676 would lead to File .../Lib/pdb.py", line 298, in setup self.curframe = self.stack[self.curindex][0] ~~~~~~~~~~^^^^^^^^^^^^^^^ IndexError: list index out of range This fixes that by filtering exceptions that that do not have a stack. Update tests to not use stack-less exceptions when testing another feature, and add an explicit test on how we handle stackless exceptions.

The introduction of chained exception in pythongh-106676 would sometime lead to File .../Lib/pdb.py", line 298, in setup self.curframe = self.stack[self.curindex][0] ~~~~~~~~~~^^^^^^^^^^^^^^^ IndexError: list index out of range This fixes that by filtering exceptions that that do not have a stack/traceback. Update tests to not use stack-less exceptions when testing another feature, and add an explicit test on how we handle stackless exceptions.

Carreau

I seemed to have missed and edge case that make the debugger crash, see #108865 as a followup.
Sorry about that.

user202729

Actually there's a bug in the documentation, it says you can use exception <number> but you actually need exceptions <number>. So for example exception 0 gives ** SyntaxError: invalid syntax, exceptions 0 works.

gaogaotiantian

@user202729 thank you for pointing this out. Fixed in #132277

bedevere-bot added the awaiting review label Jul 12, 2023

bedevere-bot mentioned this pull request Jul 12, 2023

Support to jump between chained exception in Pdb #106670

Closed

AlexWaygood requested a review from gaogaotiantian July 12, 2023 09:39

Carreau force-pushed the gh-106670 branch from def928d to 46d4526 Compare July 18, 2023 08:44

Carreau force-pushed the gh-106670 branch from 46d4526 to 79a62de Compare July 18, 2023 11:50

gaogaotiantian requested changes Jul 22, 2023

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jul 22, 2023

Carreau commented Jul 24, 2023

View reviewed changes

Carreau requested a review from gaogaotiantian July 24, 2023 09:28

gaogaotiantian requested changes Jul 27, 2023

View reviewed changes

iritkatriel reviewed Aug 10, 2023

View reviewed changes

Carreau force-pushed the gh-106670 branch from b2f6e85 to 04143d3 Compare August 10, 2023 14:14

iritkatriel reviewed Aug 10, 2023

View reviewed changes

Carreau force-pushed the gh-106670 branch from 04143d3 to 5c45f12 Compare August 10, 2023 16:00

iritkatriel reviewed Aug 10, 2023

View reviewed changes

iritkatriel reviewed Aug 14, 2023

View reviewed changes

Carreau commented Aug 14, 2023

View reviewed changes

Carreau added 2 commits August 16, 2023 09:24

Move the exception logic to its own function. …

920af94

Also move the release of the list of exception to a context manager for security

iritkatriel approved these changes Aug 27, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Aug 27, 2023

Update Doc/whatsnew/3.13.rst …

4e57c32

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

iritkatriel reviewed Aug 28, 2023

View reviewed changes

Carreau force-pushed the gh-106670 branch from 4e57c32 to 14a3b06 Compare August 28, 2023 17:37

iritkatriel enabled auto-merge (squash) August 28, 2023 17:43

AlexWaygood reviewed Aug 28, 2023

View reviewed changes

move paragraph

9e7354b

auto-merge was automatically disabled August 28, 2023 18:00
Head branch was pushed to by a user without write access

Carreau force-pushed the gh-106670 branch from 14a3b06 to 9e7354b Compare August 28, 2023 18:00

iritkatriel enabled auto-merge (squash) August 28, 2023 18:07

iritkatriel merged commit f75cefd into python:main Aug 28, 2023

bedevere-bot removed the awaiting merge label Aug 28, 2023

Carreau added a commit to Carreau/ipython that referenced this pull request Aug 29, 2023

Add support for chained exceptions. …

8467472

Closes ipython#13982 This is a "backport" of python/cpython#106676 See documentation there

Carreau mentioned this pull request Aug 29, 2023

Add support for chained exceptions. ipython/ipython#14146

Merged

Carreau added a commit to Carreau/ipython that referenced this pull request Aug 29, 2023

Add support for chained exceptions. …

95e0c37

Closes ipython#13982 This is a "backport" of python/cpython#106676 See documentation there

Carreau added a commit to ipython/ipython that referenced this pull request Aug 29, 2023

Add support for chained exceptions. (#14146) …

e5103f9

Closes #13982 This is a "backport" of python/cpython#106676 See documentation there

Carreau mentioned this pull request Sep 4, 2023

gh-106670: Fix Pdb handling of chained Exceptions with no stacks. #108865

Merged

Carreau mentioned this pull request Apr 8, 2025

Pdb chained exception decumentation issue ipython/ipython#14870

Closed

Conversation

Carreau commented Jul 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Carreau commented Jul 18, 2023

Uh oh!

gaogaotiantian left a comment

Choose a reason for hiding this comment

Uh oh!

Carreau commented Jul 24, 2023

Uh oh!

gaogaotiantian commented Jul 27, 2023

Uh oh!

gaogaotiantian left a comment

Choose a reason for hiding this comment

Uh oh!

Carreau commented Aug 9, 2023

Uh oh!

Carreau commented Aug 10, 2023

Uh oh!

iritkatriel left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Aug 28, 2023

Uh oh!

Carreau commented Aug 28, 2023

Uh oh!

Carreau commented Sep 4, 2023

Uh oh!

user202729 commented Apr 8, 2025

Uh oh!

gaogaotiantian commented Apr 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Carreau commented Jul 12, 2023 •

edited

Loading