Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
iritkatriel merged 12 commits intopython:mainfromCarreau:fix-pdb-no-tb
Sep 6, 2023

Conversation

Carreau
Copy link
Contributor

@CarreauCarreau commentedSep 4, 2023
edited by bedevere-bot
Loading

The introduction of chained exception ingh-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.

The introduction of chained exception inpythongh-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 rangeThis fixes that by filtering exceptions that that do not have astack/traceback. Update tests to not use stack-less exceptions whentesting another feature, and add an explicit test on how we handlestackless exceptions.
@Carreau
Copy link
ContributorAuthor

I can't add reviewers, can someone add@gaogaotiantian and@iritkatriel ?

I'm not sure this needs a news as it's a fix for something that was added a week ago.

@iritkatriel
Copy link
Member

How do I reproduce the bug on pdb?

@Carreau
Copy link
ContributorAuthor

How do I reproduce the bug on pdb?

Oh, yes, sure:

Python 3.13.0a0 (heads/main-dirty:08727d0af61, Sep  4 2023, 11:11:14) [Clang 14.0.6 ] on darwinType "help", "copyright", "credits" or "license" for more information.>>> def f():...     raise Exception() from Exception()...>>> f()ExceptionThe above exception was the direct cause of the following exception:Traceback (most recent call last):  File "<stdin>", line 1, in <module>  File "<stdin>", line 2, in fException>>> import pdb>>> pdb.pm()> <stdin>(2)f()(Pdb) exceptions    0 Exception()>   1 Exception()(Pdb) exceptions 0Traceback (most recent call last):  File "<stdin>", line 1, in <module>  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 2026, in pm    post_mortem(sys.last_exc)  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 2022, in post_mortem    p.interaction(None, t)  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 501, in interaction    self._cmdloop()  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 405, in _cmdloop    self.cmdloop()  File "/Users/bussonniermatthias/dev/cpython/Lib/cmd.py", line 138, in cmdloop    stop = self.onecmd(line)           ^^^^^^^^^^^^^^^^^  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 592, in onecmd    return cmd.Cmd.onecmd(self, line)           ^^^^^^^^^^^^^^^^^^^^^^^^^^  File "/Users/bussonniermatthias/dev/cpython/Lib/cmd.py", line 217, in onecmd    return func(arg)           ^^^^^^^^^  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 1178, in do_exceptions    self.setup(None, self._chained_exceptions[number].__traceback__)  File "/Users/bussonniermatthias/dev/cpython/Lib/pdb.py", line 298, in setup    self.curframe = self.stack[self.curindex][0]                    ~~~~~~~~~~^^^^^^^^^^^^^^^IndexError: list index out of range

Lib/pdb.py Outdated
Comment on lines 494 to 497
if not _chained_exceptions and isinstance(tb_or_exc, BaseException):
raise ValueError(
"A valid traceback must be passed if no exception is being handled"
)
Copy link
ContributorAuthor

@CarreauCarreauSep 4, 2023
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is supposed to catch this reproducer

>>> import sys>>> import pdb>>> sys.last_exc = Exception()>>> pdb.pm()

It's contrived, but as it is already handled inpdb.post_mortem(), I assume there are case where this can happen in production.

@gaogaotiantian
Copy link
Member

Is there any meaningful case where the latest exception (the one passed intopm() orsys.last_exc) does not have traceback, but the chained exceptions could be useful?

Here's what I thought:

  • It's probably common where a chained exception does not have traceback, but it's normally not the exception raised and caught.
  • When the exceptionpdb catches (throughpm(),interaction is not documented API) does not have the traceback, there's normally nothing interesting.

However:

  • We SHOULD keep the exceptions without tracebacks when possible, because they are part of the chain and it would confuse the users if there's an exception missing just because there's no trackback attached.
  • We should try NOT to raise exceptions from a common function deep inpdb - it would be frustrating to users whenpdb errors out andinteraction is a function used else where. It may only raise the exception in this very specific situation now, but in the future it could be triggered under other circumstances when there are some code changes. If we want to raise an exception, we should raise early.

So, I think the better way to do it is:

  • Check whether we can handle it (whether the raised exception has traceback) inpost_mortem(), the same place as the existingValueError. If not, raise the exception there.
  • Onceinteraction() is called, we do not ever raise any exceptions. The code should make sure the latest exception has a trackback to display, anddo_exceptions should check if the "target exception" has a traceback and display a message to user when it does not. No exception raised.

It would be much clearer to the future maintainers thatpdb does not raise exception under a very complicated condition, and it would make the code ininteraction() easier to understand - keep the exception related logic indo_exceptions.

Thoughts@iritkatriel ?

@Carreau
Copy link
ContributorAuthor

@gaogaotiantian I'll see what I can do, but it might probably be quite invasive to support exceptions w/o tb. I can maybe keep them but refuse toexceptions #tb-without-traceback and/or display them differently inexceptions.

Note thatinteraction() is fairly early in Pdb's life, were you maybe thinkingof_cmdloop maybe ?

@iritkatriel
Copy link
Member

Yes, I think it would be good if all exceptions were shown, and those without traceback would display that somehow (rather than erroring).

@gaogaotiantian
Copy link
Member

@gaogaotiantian I'll see what I can do, but it might probably be quite invasive to support exceptions w/o tb. I can maybe keep them but refuse toexceptions #tb-without-traceback and/or display them differently inexceptions.

Note thatinteraction() is fairly early in Pdb's life, were you maybe thinkingof_cmdloop maybe ?

Yes, as we can guarantee the default exception(latest one) is with traceback, we can simply decline all the access to the exceptions without tracebacks when the users doexceptions #exception_without_traceback. Send a message saying there's no traceback, and keep them in the current exception - so we don't need to take care of all the commands in a setup without the traceback.

interaction() is fairly early, but it's still called for every trace trigger - including the exception trigger which might take advantage of the exception feature in the future. We want to put the exception as closer to the surface as possible.

@Carreau
Copy link
ContributorAuthor

Ok, what about the following ? in do_exceptions I print- as number for exceptions that do not have a traceback (keeping the numbering though, so you would get something like

 (Pdb) exceptions     - Exception()     1 Exception()     2 Exception()     - Exception() >   4 Exception()     5 Exception()     - Exception()     7 Exception()

And on attempt to jump to it, it says:This exception has not traceback, cannot jump to it

There is one caveat WRT testing, I can't properly testpost_mortem() as the testing needs topdb.Pdb(nosigint=True, readrc=False) andpost_mortem() create its own instance I have no control over.

iritkatriel reacted with thumbs up emoji

@Carreau
Copy link
ContributorAuthor

There is one caveat WRT testing, I can't properly testpost_mortem() as the testing needs topdb.Pdb(nosigint=True, readrc=False) andpost_mortem() create its own instance I have no control over.

I added a private version of post-mortem that takes an instance so that I can use it in the test.

@iritkatrieliritkatrielenabled auto-merge (squash)September 6, 2023 09:41
@iritkatrieliritkatriel added skip news stdlibPython modules in the Lib dir labelsSep 6, 2023
@iritkatrieliritkatriel merged commit5f3433f intopython:mainSep 6, 2023
@Carreau
Copy link
ContributorAuthor

Carreau commentedSep 6, 2023 via email

thanks !
On Wed, Sep 6, 2023 at 11:42 Irit Katriel ***@***.***> wrote: Merged#108865 <#108865> into main. — Reply to this email directly, view it on GitHub <#108865 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACR5TZLKHNFVV2YC73XJL3XZBAPBANCNFSM6AAAAAA4KEPKUQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>
iritkatriel reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@iritkatrieliritkatrieliritkatriel approved these changes

@gaogaotiantiangaogaotiantianAwaiting requested review from gaogaotiantiangaogaotiantian is a code owner

Assignees
No one assigned
Labels
skip newsstdlibPython modules in the Lib dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@Carreau@iritkatriel@gaogaotiantian@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp