Visitar URL original
Instance attr error suggestions can execute `__getattr__` · Issue #132385 · python/cpython · GitHub
Skip to content

Instance attr error suggestions can execute __getattr__ #132385

Closed
Closed
Instance attr error suggestions can execute __getattr__#132385
Assignees
sobolevn
Labels
stdlibStandard Library Python modules in the Lib/ directorytriagedThe issue has been accepted as valid by a triager.type-bugAn unexpected behavior, bug, or error

Description

@sobolevn
sobolevn
opened on Apr 11, 2025

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

Activity

  1. sobolevn
    added
    stdlibStandard Library Python modules in the Lib/ directory
    type-bugAn unexpected behavior, bug, or error
    on Apr 11, 2025
  2. sobolevn
    self-assigned this
    on Apr 11, 2025
  3. sobolevn
    added a commit that references this issue on Apr 11, 2025

    pythongh-132385: Fix instance error suggestions side effects in `trac…

    f1fc9f3

  4. pablogsal commented on Apr 11, 2025

    @pablogsal
    pablogsal
    on Apr 11, 2025
    Member

    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

  5. skirpichev commented on Apr 11, 2025

    @skirpichev
    skirpichev
    on Apr 11, 2025
    Member

    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;-)

  6. sobolevn commented on Apr 11, 2025

    @sobolevn
    sobolevn
    on Apr 11, 2025
    MemberAuthor

    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.

  7. pablogsal commented on Apr 11, 2025

    @pablogsal
    pablogsal
    on Apr 11, 2025
    Member

    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

  8. iritkatriel commented on Apr 11, 2025

    @iritkatriel
    iritkatriel
    on Apr 11, 2025
    Member

    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.

  9. millerdev commented on Apr 11, 2025

    @millerdev
    millerdev
    on Apr 11, 2025

    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__?

  10. sobolevn commented on Apr 11, 2025

    @sobolevn
    sobolevn
    on Apr 11, 2025
    MemberAuthor

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

  11. pablogsal commented on Apr 11, 2025

    @pablogsal
    pablogsal
    on Apr 11, 2025
    Member

    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.

  12. picnixz
    added
    extension-modulesC modules in the Modules dir
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    stdlibStandard Library Python modules in the Lib/ directory
    triagedThe issue has been accepted as valid by a triager.
    and removed
    stdlibStandard Library Python modules in the Lib/ directory
    extension-modulesC modules in the Modules dir
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Apr 11, 2025

  13. picnixz commented on Apr 11, 2025

    @picnixz
    picnixz
    on Apr 11, 2025
    Member

    (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)

  14. sobolevn
    added a commit that references this issue on May 2, 2025

    gh-132385: Fix instance error suggestions trigger potential exception…

    641253c
  15. miss-islington
    added a commit that references this issue on May 2, 2025

    pythongh-132385: Fix instance error suggestions trigger potential exc…

    3c96eaf
  16. sobolevn
    closed this as completedon May 2, 2025
  17. sobolevn
    added a commit that references this issue on May 2, 2025

    [3.13] gh-132385: Fix instance error suggestions trigger potential ex…

    16e9e74
  18. Pranjal095
    added a commit that references this issue on Jul 12, 2025

    pythongh-132385: Fix instance error suggestions trigger potential exc…

    88f00e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

stdlibStandard Library Python modules in the Lib/ directorytriagedThe issue has been accepted as valid by a triager.type-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions