Skip to content

gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__#113359

Merged
vsajip merged 5 commits into
python:mainfrom
perrinjerome:fix/exception_broken_getattr
Jan 16, 2024
Merged

gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__#113359
vsajip merged 5 commits into
python:mainfrom
perrinjerome:fix/exception_broken_getattr

Conversation

@perrinjerome

@perrinjerome perrinjerome commented Dec 21, 2023

Copy link
Copy Markdown
Contributor

@ghost

ghost commented Dec 21, 2023

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app

bedevere-app Bot commented Dec 21, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@perrinjerome
perrinjerome marked this pull request as ready for review December 21, 2023 15:31
Comment thread Lib/traceback.py Outdated
try:
notes = getattr(exc_value, '__notes__', None)
except Exception:
pass

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.

We don't swallow exceptions like this. What is your use case for defining an exception with a broken __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.

I had much the same question on the issue. Python reporting bugs in user code does not seem like a python bug. OP needs to justify the python bug claim by quoting doc that says that user exceptions will by caught, or make this a feature request, with justification.

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.

Thank you for your feedback, I am sorry I did not explain enough. I have tried to add more context to the issue.

About not swallowing exceptions, I don't like it either (and I understand the negative feedback), maybe this patch should be adjusted to at least report that an exception occurred while getting the notes, by doing something like this instead ?

        try:
            self.__notes__ = getattr(exc_value, '__notes__', None)
        except Exception as e:
            self.__notes__ = [
                f'Ignored exception getting __notes__: {_safe_string(e, '__notes__', repr)}']

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.

Unfortunately we do swallow exceptions like this. _safe_string() set a bad precedence since 2823f03.

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.

I have updated the patch to include the "Ignored exception getting notes ..." in the notes as suggested above, from the discussion on the issue it seemed better to include information about the error in that case.

@terryjreedy
terryjreedy requested a review from vsajip December 22, 2023 14:28
Comment thread Lib/traceback.py Outdated
Comment thread Lib/test/test_traceback.py Outdated
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@vsajip
vsajip merged commit 04fabe2 into python:main Jan 16, 2024
@vsajip vsajip added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Jan 16, 2024
@miss-islington-app

Copy link
Copy Markdown

Thanks @perrinjerome for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Thanks @perrinjerome for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 16, 2024
…en __getattr__ (pythonGH-113359)

(cherry picked from commit 04fabe2)

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@miss-islington-app

Copy link
Copy Markdown

Sorry, @perrinjerome and @vsajip, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 04fabe22dd98b4d87f672254b743fbadd5206352 3.11

@bedevere-app

bedevere-app Bot commented Jan 16, 2024

Copy link
Copy Markdown

GH-114105 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 16, 2024
vsajip pushed a commit to vsajip/cpython that referenced this pull request Jan 16, 2024
… a broken __getattr__ (pythonGH-113359)

(cherry picked from commit 04fabe2)

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented Jan 16, 2024

Copy link
Copy Markdown

GH-114109 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Jan 16, 2024
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 16, 2024
… a broken __getattr__ (pythonGH-113359)

(cherry picked from commit 04fabe2)

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 16, 2024
… a broken __getattr__ (pythonGH-113359)

(cherry picked from commit 04fabe2)

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented Jan 16, 2024

Copy link
Copy Markdown

GH-114118 is a backport of this pull request to the 3.11 branch.

2 similar comments
@bedevere-app

bedevere-app Bot commented Jan 16, 2024

Copy link
Copy Markdown

GH-114118 is a backport of this pull request to the 3.11 branch.

@bedevere-app

bedevere-app Bot commented Jan 16, 2024

Copy link
Copy Markdown

GH-114118 is a backport of this pull request to the 3.11 branch.

perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 16, 2024
…en __getattr__ (pythonGH-113359)

cherry picked from commit 04fabe2

Adjusted for 3.11, because exception printing also happens in C
code. For simplicity, instead of including the repr of the exception
that happened during __getattr__, this just prints that an error
occured.

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 16, 2024
…en __getattr__ (pythonGH-113359)

cherry picked from commit 04fabe2

Adjusted for 3.11, because exception printing also happens in C
code. For simplicity, instead of including the repr of the exception
that happened during __getattr__, this just prints that an error
occured.

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 17, 2024
…en __getattr__ (pythonGH-113359)

cherry picked from commit 04fabe2

Adjusted for 3.11, because exception printing also happens in C
code.

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 17, 2024
…en __getattr__ (pythonGH-113359)

cherry picked from commit 04fabe2

Adjusted for 3.11, because exception printing also happens in C
code.

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 17, 2024
…en __getattr__ (pythonGH-113359)

cherry picked from commit 04fabe2

Adjusted for 3.11, because exception printing also happens in C
code.

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented Jan 17, 2024

Copy link
Copy Markdown

GH-114173 is a backport of this pull request to the 3.12 branch.

perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 17, 2024
…en __getattr__ (pythonGH-113359)

cherry picked from commit 04fabe2

Adjusted for 3.12, because exception printing also happens in C
code.

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
iritkatriel pushed a commit that referenced this pull request Jan 19, 2024
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 20, 2024
… a broken __getattr__ (pythonGH-113359)

(cherry picked from commit 04fabe2)

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
perrinjerome added a commit to perrinjerome/cpython that referenced this pull request Jan 20, 2024
… a broken __getattr__ (pythonGH-113359)

(cherry picked from commit 04fabe2)

Co-authored-by: Jérome Perrin <perrinjerome@gmail.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
iritkatriel pushed a commit that referenced this pull request Jan 21, 2024
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
…en __getattr__ (pythonGH-113359)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…en __getattr__ (pythonGH-113359)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…en __getattr__ (pythonGH-113359)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants