Skip to content

gh-129605: Fix crashing pyrepl when NameError occurres in __getattr__ method - #129615

Closed
srinivasreddy wants to merge 15 commits into
python:mainfrom
srinivasreddy:gh-129605
Closed

srinivasreddy wants to merge 15 commits into
python:mainfrom
srinivasreddy:gh-129605

Conversation

@srinivasreddy

@srinivasreddy srinivasreddy commented Feb 3, 2025 •

Copy link
Copy Markdown
Contributor

@srinivasreddy
srinivasreddy marked this pull request as ready for review February 3, 2025 13:53
@srinivasreddy srinivasreddy changed the title Fix crashing pyrepl when NameError occurres in __getattr__ method gh-129605: Fix crashing pyrepl when NameError occurres in __getattr__ method Feb 3, 2025
Comment thread Misc/NEWS.d/next/Library/2025-02-03-16-39-35.gh-issue-129605.sN_0uq.rst Outdated
Comment thread Lib/traceback.py Outdated
self = frame.f_locals['self']
if hasattr(self, wrong_name):
return f"self.{wrong_name}"
if frame.f_code.co_name != '__getattr__':

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This also happens with __getattribute__.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed 05a9858

Comment thread Lib/traceback.py Outdated
Comment thread Lib/test/test_traceback.py Outdated

@skirpichev skirpichev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

But see the issue thread, maybe there should be a better way.

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 21, 2026
Copilot AI balanced review requested due to automatic review settings October 20, 2026 21:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The tests emit unnecessary stdout noise contrary to the surrounding test convention and the news entry has an awkward grammatical phrasing, both of which should be cleaned up before approval.

2 open issues
What changed in this PR

This PR adds regression coverage and a news entry for gh-129605, where the interactive interpreter (pyrepl) would crash while computing a "Did you mean" suggestion for a NameError. The underlying fix guards the hasattr(self, wrong_name) lookup in traceback._compute_suggestion_error so that a re-entrant NameError raised from a user-defined __getattr__/__getattribute__/property (triggered by the hasattr probe itself) no longer propagates and breaks traceback formatting. The diff under review contains only the tests and the news fragment.

Changes:

  • Add test_name_error_with_getattr covering NameError raised inside __getattr__ and __getattribute__.
  • Add test_name_error_with_property_name covering NameError raised inside a property getter whose name matches the looked-up attribute.
  • Add a Misc/NEWS.d entry describing the fix.
File Description
Lib/​test/​test_traceback.py Adds two regression tests verifying suggestion formatting does not crash and reports exactly one NameError when attribute access re-raises during suggestion computation.
Misc/​NEWS.d/​next/​Library/​2025-02-03-16-39-35.gh-issue-129605.sN_0uq.rst News entry documenting the pyrepl crash fix.

Review effort: Balanced

Comment on lines +5091 to +5097
def __getattr__(self, x):
print(x)
print(qq)
class B:
def __getattribute__(self, x):
print(x)
print(qq)
@@ -0,0 +1 @@
Fix pyrepl crash when a :exc:`NameError` occurs in the :meth:`~object.__getattr__`, :meth:`~object.__getattribute__` and any method of a class.
@ambv

ambv commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

This was addressed by #129632. After rebase the only things left were NEWS and the test.

@ambv ambv closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants