-
-
You must be signed in to change notification settings -
gh-65052: Prevent pdb from crashing when trying to display objects #110578
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
Lib/test/test_pdb.py
Outdated
(Pdb) args | ||
self = *** AttributeError: 'A' object has no attribute 'a' *** | ||
(Pdb) display self | ||
display self: *** AttributeError: 'A' object has no attribute 'a' *** |
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.
I think it would be easier for a user to understand what happened if this printed sometime like what traceback._safe_string
prints when str or repr fail (the fact that repr failed is more informative in this context than the precise exception. This could be read as "self is an AttributeError").
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.
Yeah that makes sense. I added an error message repr({expr}) failed:
before the exception. Still think it might be helpful for users to know what actually failed - and now it's less ambiguous with the extra error message (hopefully).
try: | ||
return repr(obj) | ||
except Exception as e: | ||
return _rstr(f"*** repr({expr}) failed: {self._format_exc(e)} ***") |
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.
Do we use f-strings or %
in this script? Or a mixture of the two?
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.
I'm using f-strings for all new code. There are plenty of usages with %
as pdb
is old. Do you think it would be helpful to have a PR to update all strings to f-strings? It would eliminate the mixture.
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.
Probably not worth the risk of breaking something.
Should this be backported? In working cases it does not change the output of |
Yes, good point. |
Thanks @gaogaotiantian for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Thanks @gaogaotiantian for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Sorry, @gaogaotiantian and @iritkatriel, I could not cleanly backport this to
|
…cts (pythonGH-110578) (cherry picked from commit c523ce0) Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
GH-110734 is a backport of this pull request to the 3.12 branch. |
@iritkatriel , do you need me to cp this to 3.11? |
If you think it should be backported there then yes, it would require a manual PR. |
…ay objects (pythonGH-110578) (cherry picked from commit c523ce0) Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
GH-111002 is a backport of this pull request to the 3.11 branch. |
Currently in
pdb
we have a couple of explicit/implicit usage ofrepr
when the object is arbitrary - which could potentially cause apdb
crash when the object is not displayable. Even though it's not super common, we should try out best makingpdb
not crash. Hence_safe_repr
is introduced to print objects for: