10000 gh-129605: Add more exception handling when computing suggestions in traceback.py by ADThomas-astro · Pull Request #129632 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-129605: Add more exception handling when computing suggestions in traceback.py #129632

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Lib/test/test_traceback.py
Original file line number Diff line number Diff line change
Expand Up @@ -4514,6 +4514,32 @@ def func():
actual = self.get_suggestion(func)
self.assertIn("forget to import '_io'", actual)

def test_name_error_with_bad_getattr(self):
class A:
def __getattr__(self, x):
spam

actual = self.get_suggestion(A(), "eggs")
self.assertIn("name 'spam' is not defined", actual)

def test_name_error_with_bad_getattribute(self):

class A:
def __getattribute__(self, x):
spam

actual = self.get_suggestion(A(), "eggs")
self.assertIn("name 'spam' is not defined", actual)

def test_name_error_with_bad_property(self):
class A:
@property
def eggs(self):
eggs

actual = self.get_suggestion(A(), "eggs")
self.assertIn("name 'eggs' is not defined", actual)



class PurePythonSuggestionFormattingTests(
Expand Down
37 changes: 20 additions & 17 deletions Lib/traceback.py
Original file line number Diff line number Diff line change
Expand Up @@ -1504,24 +1504,27 @@ def _compute_suggestion_error(exc_value, tb, wrong_name):
return None
else:
assert isinstance(exc_value, NameError)
# find most recent frame
if tb is None:
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a very broad try-except block. If you scoped it only around the hasattr call would the new test cases still pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes they would still pass. It's usually good practice to minimise code in a try block, but I think this is an exceptional case. This function attempts to provide "Did you mean foo?" suggestions after there has already been an error in user code. Hence a different exception propagating from this function is very unhelpful (and the cause of the issue). So I think it's worth having wider try blocks here to minimise the risk of unhandled exceptions in the future. The existing code has wide try blocks ignoring all Exception subclasses when doing processing for AttributeError (L1485-1494) and ImportError (L1499-1502). I think we we should treat the processing for NameError the same. (Although - I haven't found a way to cause an exception in L1508-1517, so this is mostly theoretical.) Happy to narrow the scope of the try if folks feel strongly.

# find most recent frame
if tb is None:
return None
while tb.tb_next is not None:
tb = tb.tb_next
frame = tb.tb_frame
d = (
list(frame.f_locals)
+ list(frame.f_globals)
+ list(frame.f_builtins)
)

# Check first if we are in a method and the instance
# has the wrong name as attribute
if 'self' in frame.f_locals:
self = frame.f_locals['self']
if hasattr(self, wrong_name):
return f"self.{wrong_name}"
except Exception:
return None
while tb.tb_next is not None:
tb = tb.tb_next
frame = tb.tb_frame
d = (
list(frame.f_locals)
+ list(frame.f_globals)
+ list(frame.f_builtins)
)

# Check first if we are in a method and the instance
# has the wrong name as attribute
if 'self' in frame.f_locals:
self = frame.f_locals['self']
if hasattr(self, wrong_name):
return f"self.{wrong_name}"

try:
import _suggestions
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve exception handling when computing suggestions in traceback.py. This fixes a bug that could cause PyREPL to exit unexpectedly.
Loading
0