Skip to content

gh-103895: Improve how invalid Exception.__notes__ are displayed #103897

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
May 1, 2023

Conversation

pR0Ps
Copy link
Contributor

@pR0Ps pR0Ps commented Apr 26, 2023

  • Ensure that when displayed a note fails, the failure message includes a trailing newline.
  • Improve isinstance(__notes__, Sequence) check to exclude strings and bytes to avoid exploding them into individual characters.

Potential code reviewer based on the experts index and git blame (d4c4a76): @iritkatriel

 - Ensure that when displayed a note fails, the failure message includes
   a trailing newline.
 - Improve `isinstance(__notes__, Sequence)` check to exclude strings
   and bytes to avoid exploding them into individual characters.
@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Apr 27, 2023
@iritkatriel
Copy link
Member

@ambv any thoughts on backporting this to 3.11? It's improving the traceback display for some "invalid" values of __notes__. I don't think this is a backwards compatibility issue (people shouldn't be parsing tracebacks, right?)

@iritkatriel iritkatriel merged commit 487f55d into python:main May 1, 2023
carljm added a commit to carljm/cpython that referenced this pull request May 1, 2023
* main: (26 commits)
  pythongh-104028: Reduce object creation while calling callback function from gc (pythongh-104030)
  pythongh-104036: Fix direct invocation of test_typing (python#104037)
  pythongh-102213: Optimize the performance of `__getattr__` (pythonGH-103761)
  pythongh-103895: Improve how invalid `Exception.__notes__` are displayed (python#103897)
  Adjust expression from `==` to `!=` in alignment with the meaning of the paragraph. (pythonGH-104021)
  pythongh-88496: Fix IDLE test hang on macOS (python#104025)
  Improve int test coverage (python#104024)
  pythongh-88773: Added teleport method to Turtle library (python#103974)
  pythongh-104015: Fix direct invocation of `test_dataclasses` (python#104017)
  pythongh-104012: Ensure test_calendar.CalendarTestCase.test_deprecation_warning consistently passes (python#104014)
  pythongh-103977: compile re expressions in platform.py only if required (python#103981)
  pythongh-98003: Inline call frames for CALL_FUNCTION_EX (pythonGH-98004)
  Replace Netlify with Read the Docs build previews (python#103843)
  Update name in acknowledgements and add mailmap (python#103696)
  pythongh-82054: allow test runner to split test_asyncio to execute in parallel by sharding. (python#103927)
  Remove non-existing tools from Sundry skiplist (python#103991)
  pythongh-103793: Defer formatting task name (python#103767)
  pythongh-87092: change assembler to use instruction sequence instead of CFG (python#103933)
  pythongh-103636: issue warning for deprecated calendar constants (python#103833)
  Various small fixes to dis docs (python#103923)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 1, 2023
* main: (463 commits)
  pythongh-104057: Fix direct invocation of test_super (python#104064)
  pythongh-87092: Expose assembler to unit tests (python#103988)
  pythongh-97696: asyncio eager tasks factory (python#102853)
  pythongh-84436: Immortalize in _PyStructSequence_InitBuiltinWithFlags() (pythongh-104054)
  pythongh-104057: Fix direct invocation of test_module (pythonGH-104059)
  pythongh-100458: Clarify Enum.__format__() change of mixed-in types in the whatsnew/3.11.rst (pythonGH-100387)
  pythongh-104018: disallow "z" format specifier in %-format of byte strings (pythonGH-104033)
  pythongh-104016: Fixed off by 1 error in f string tokenizer (python#104047)
  pythonGH-103629: Update Unpack's repr in compliance with PEP 692 (python#104048)
  pythongh-102799: replace sys.exc_info by sys.exception in inspect and traceback modules (python#104032)
  Fix typo in "expected" word in few source files (python#104034)
  pythongh-103824: fix use-after-free error in Parser/tokenizer.c (python#103993)
  pythongh-104035: Do not ignore user-defined `__{get,set}state__` in slotted frozen dataclasses (python#104041)
  pythongh-104028: Reduce object creation while calling callback function from gc (pythongh-104030)
  pythongh-104036: Fix direct invocation of test_typing (python#104037)
  pythongh-102213: Optimize the performance of `__getattr__` (pythonGH-103761)
  pythongh-103895: Improve how invalid `Exception.__notes__` are displayed (python#103897)
  Adjust expression from `==` to `!=` in alignment with the meaning of the paragraph. (pythonGH-104021)
  pythongh-88496: Fix IDLE test hang on macOS (python#104025)
  Improve int test coverage (python#104024)
  ...
Carreau added a commit to ipython/ipython that referenced this pull request Jun 2, 2023
[PEP 678](https://peps.python.org/pep-0678/) introduced the ability to
add notes to exception objects. This has been [released in Python
3.11](https://docs.python.org/3/library/exceptions.html#BaseException.add_note)
and is currently not implemented in IPython. These changes are fully
compatible with older Python versions that don't include PEP 678.

Here's a sample test that shows the consistency in Python's stdlib
traceback module (test 1) and the difference between Python and
IPython's runtimes (test 2):
```python
import traceback

print('--- test 1 ---')
try:
    raise Exception('Testing notes')
except Exception as e:
    e.add_note('Does this work?')
    e.add_note('Yes!')
    traceback.print_exc()

print('\n--- test 2 ---')
try:
    raise Exception('Testing notes')
except Exception as e:
    e.add_note('Does this work?')
    e.add_note('No!')
    raise
```

When executed with Python 3.11, both notes are displayed in both
tracebacks:
```
$ python test.py 
--- test 1 ---
Traceback (most recent call last):
  File "/app/test.py", line 5, in <module>
    raise Exception('Testing notes')
Exception: Testing notes
Does this work?
Yes!

--- test 2 ---
Traceback (most recent call last):
  File "/app/test.py", line 13, in <module>
    raise Exception('Testing notes')
Exception: Testing notes
Does this work?
No!
```

In IPython's VerboseTB does not yet handle exception notes:
```
$ ipython test.py 
--- test 1 ---
Traceback (most recent call last):
  File "/app/test.py", line 5, in <module>
    raise Exception('Testing notes')
Exception: Testing notes
Does this work?
Yes!

--- test 2 ---
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
File /app/test.py:13
     11 print('\n--- test 2 ---')
     12 try:
---> 13     raise Exception('Testing notes')
     14 except Exception as e:
     15     e.add_note('Does this work?')

Exception: Testing notes
```

The changes I am suggesting are inspired from implementation of
[Lib/traceback.py](https://github.com/python/cpython/blob/main/Lib/traceback.py)
(search for `__notes__`) and improvements for dealing with edge cases
more nicely in
[cpython#103897](python/cpython#103897).

Although notes are meant to be strings only, I kept some inspiration
from the existing exception handling to ensure that the notes are
uncolored and bytes decoded, if there are any. I am definitely open to
using a different color if deemed better. For context, `bpython` keeps
the notes uncolored, and [Python's
tutorial](https://docs.python.org/3/tutorial/errors.html#enriching-exceptions-with-notes)
puts them in light gray, like the line numbers.

Here's how the test 2 looks like after these changes:

![image](https://user-images.githubusercontent.com/16963011/234723689-6bbfe0ff-94d4-4a90-9da6-acfe1c8e5edf.png)

## 🐍 🤹‍♂️
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants