-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
I can't add reviewers, can someone add I'm not sure this needs a news as it's a fix for something that was added a week ago. |
How do I reproduce the bug on pdb? |
Oh, yes, sure:
|
Lib/pdb.py
Outdated
if not _chained_exceptions and isinstance(tb_or_exc, BaseException): | ||
raise ValueError( | ||
"A valid traceback must be passed if no exception is being handled" | ||
) |
There was a problem hiding this comment.
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 in pdb.post_mortem()
, I assume there are case where this can happen in production.
2e6c7de
to
3607006
Compare
Is there any meaningful case where the latest exception (the one passed into Here's what I thought:
However:
So, I think the better way to do it is:
It would be much clearer to the future maintainers that Thoughts @iritkatriel ? |
@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 to Note that |
Yes, I think it would be good if all exceptions were shown, and those without traceback would display that somehow (rather than erroring). |
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 do
|
Ok, what about the following ? in do_exceptions I print
And on attempt to jump to it, it says: There is one caveat WRT testing, I can't properly test |
I added a private version of post-mortem that takes an instance so that I can use it in the test. |
Co-authored-by: Irit Katriel <[email protected]>
… the next previous one with tb
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:
***@***.***>
|
The introduction of chained exception in gh-106676 would sometime lead to
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.