Instance attr error suggestions can execute __getattr__

[sobolevn]

Bug report

>>> class A:
...     def __getattr__(self, key):
...         if key == 'foo': raise SystemExit('bye')
...     def bar(self):
...         foo
...             
>>> A().bar()
bye
# interperter exits :(

Originally found by @millerdev in #99140 (comment)

I think that this is not ideal. We probably want to silence all errors. I have a PR ready.

Linked PRs

• gh-132385: Fix instance error suggestions potential exceptions in traceback #132387

[pablogsal]

I am ok being more defensive but this is a known limitation in general and was discussed plenty of times. If users create an object with known unwanted effects on things like __getattr__ or __dir__ there are basically on their own since the VM can call those at any point. The same if some user adds side effects to __repr__ or similar

[skirpichev]

We probably want to silence all errors.

You meant except AttributeError? Hmm, why? Accordingly to the docs, the dunder method in example is broken. But I doubt we should silently hide this from the end user.

The SystemExit is a special snowflake: no traceback will be printed. But I think that the example is slightly artificial in using that specific exception. With a different you will got something more meaningful. Well, when issues like #129605 will be fixed in the new REPL;-)

[sobolevn]

If users create an object with known unwanted effects on things like getattr or dir there are basically on their own since the VM can call those at any point

@pablogsal yes, I understand that and agree.

But, I think that error suggestions are a bit unique here. Because most of the time NameError happens when code is not working as intended. And at the same time perfectly valid __getattr__ can produce things like AttributeError, TypeError, etc.

Here's the demo of the difference between a regular NameError:

2025-04-11.13.36.02.mov

And a side-effect which terminates the whole REPL:

2025-04-11.13.35.29.mov

I propose to fix the second behavior.
@iritkatriel suggested that we should not catch BaseException, I agree. I will change by PR to only handle Exception.

[pablogsal]

Yeah I am not against fixing the second behaviour I am just being cautious of not trying to go crazy and promise things that would complicate or restrict everything

[iritkatriel]

If users create an object with known unwanted effects on things like getattr or dir there are basically on their own since the VM can call those at any point

KeyboardInterrupt comes from the system, not from the object.

[millerdev]

It's true, it was a very contrived example. SystemExit was used for dramatic effect, but is admittedly quite unrealistic.

A far more likely scenario would be a NameError resulting in attribute lookup that causes a unintended side effect like hitting a database or loading a cache, that then further complicates debugging attempts by distracting from the real problem.

Would it make sense to use an attribute lookup strategy more like attr in self.__dict__ or something along those lines that does not trigger property accessors or special methods like __getattr__?

[sobolevn]

self.__dict__ assumes that we have __dict__ and working __getattribute__. So, let's keep hasattr in place, but just silence exceptions :)

[pablogsal]

If users create an object with known unwanted effects on things like getattr or dir there are basically on their own since the VM can call those at any point

KeyboardInterrupt comes from the system, not from the object.

Yeah, but this is not because someone would override stuff but because someone sent the signal no?

A far more likely scenario would be a NameError resulting in attribute lookup that causes a unintended side effect like hitting a database or loading a cache, that then further complicates debugging attempts by distracting from the real problem.

This is precisely what I meant: if the user is implementing that behavior then is up to them, we should not try to protect anything here or we will be drowning in complexity.

[picnixz]

(sorry, I thought that the error was at the interpreter's level but it's more in traceback; thus I added the triaged label to be sure I'm not passing over that issue again)