bpo-4643: Handles failure with getattr exception in cgitb#24038
Conversation
This convert a patch to a GitHub PR written by Arthur Petitpierre and proposed by Allan Crooks Co-authored-by: Arthur Petitpierre Co-authored-by: Allan Crooks
| value = getattr(parent, token, __UNDEF__) | ||
| try: | ||
| value = getattr(parent, token, __UNDEF__) | ||
| except Exception: |
There was a problem hiding this comment.
Don't catch all subclasses of Exception. Make it more specific.
There was a problem hiding this comment.
Actually this getattr should not raise exceptions at all because it is given a default value.
The docs for __getattr__ say: "This method should either return the (computed) attribute value or raise an AttributeError exception." It is an error for it to raise a different exception, so I think this patch should not be merged.
There was a problem hiding this comment.
Well, the issue is saying this is error handling and the code should be more robust in the presence of such malformed objects, so maybe this patch can be merged, but in any case you should not swallow SystemError/MemoryError/OverflowErrors - they should propagate to the user.
There was a problem hiding this comment.
So I found an existing better PR for this: #15094 related to https://bugs.python.org/issue966992
I will close this PR here. Thanks @iritkatriel for the pushback, it helped me dig into the issue a bit more.
| value = getattr(parent, token, __UNDEF__) | ||
| try: | ||
| value = getattr(parent, token, __UNDEF__) | ||
| except Exception: |
There was a problem hiding this comment.
Actually this getattr should not raise exceptions at all because it is given a default value.
The docs for __getattr__ say: "This method should either return the (computed) attribute value or raise an AttributeError exception." It is an error for it to raise a different exception, so I think this patch should not be merged.
4 participants
This convert a patch to a GitHub PR written by Arthur Petitpierre
and proposed by Allan Crooks
Co-authored-by: Arthur Petitpierre
Co-authored-by: Allan Crooks
https://bugs.python.org/issue4643