traceback.FrameSummary does not handle exceptions from `repr()`

[blueyed]

BPO	39228
Nosy	@blueyed, @serhiy-storchaka, @iritkatriel, @moi90, @akulakov
PRs	[WIP/RFC] bpo-39228: traceback.FrameSummary: handle error via repr #17855

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-05-12.14:54:11.369>
created_at = <Date 2020-01-06.04:14:25.321>
labels = ['invalid', 'type-bug', 'library', '3.11']
title = 'traceback.FrameSummary does not handle exceptions from `repr()`'
updated_at = <Date 2021-11-08.07:23:59.440>
user = 'https://github.com/blueyed'

bugs.python.org fields:

activity = <Date 2021-11-08.07:23:59.440>
actor = 'moi90'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-12.14:54:11.369>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2020-01-06.04:14:25.321>
creator = 'blueyed'
dependencies = []
files = []
hgrepos = []
issue_num = 39228
keywords = ['patch']
message_count = 11.0
messages = ['359400', '359770', '389844', '389852', '389859', '389861', '389889', '389894', '393200', '393297', '405933']
nosy_count = 5.0
nosy_names = ['blueyed', 'serhiy.storchaka', 'iritkatriel', 'moi90', 'andrei.avk']
pr_nums = ['17855']
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39228'
versions = ['Python 3.11']

[blueyed]

Exceptions within __repr__ methods of captured locals
(e.g. via the capture_locals argument of TracebackException) are not handled:

import traceback


class CrashingRepr:
    def __repr__(self):
        raise RuntimeError("crash")


traceback.FrameSummary("fname", 1, "name", locals={"crash": CrashingRepr()})

Result:

Traceback (most recent call last):
  File "test_framesummary_repr.py", line 9, in <module>
    traceback.FrameSummary("fname", 1, "name", locals={"crash": CrashingRepr()})
  File "…/pyenv/3.8.0/lib/python3.8/traceback.py", line 260, in __init__
    self.locals = {k: repr(v) for k, v in locals.items()} if locals else None
  File "…/pyenv/3.8.0/lib/python3.8/traceback.py", line 260, in <dictcomp>
    self.locals = {k: repr(v) for k, v in locals.items()} if locals else None
  File "test_framesummary_repr.py", line 6, in __repr__
    raise RuntimeError("crash")
RuntimeError: crash

The following patch would fix this:

diff --git i/Lib/traceback.py w/Lib/traceback.py
index 7a4c8e19f9..eed7082db4 100644
--- i/Lib/traceback.py
+++ w/Lib/traceback.py
 class FrameSummary:
     """A single frame from a traceback.

@@ -257,7 +265,17 @@ def __init__(self, filename, lineno, name, *, lookup_line=True,
         self._line = line
         if lookup_line:
             self.line
-        self.locals = {k: repr(v) for k, v in locals.items()} if locals else None
+        if locals:
+            self.locals = {}
+            for k, v in locals.items():
+                try:
+                    self.locals[k] = repr(v)
+                except (KeyboardInterrupt, SystemExit):
+                    raise
+                except BaseException as exc:
+                    self.locals[k] = f"<unrepresentable repr ({exc})>"
+        else:
+            self.locals = None

     def __eq__(self, other):
         if isinstance(other, FrameSummary):

[terryjreedy]

As is true for most special methods, it is a bug for __repr__ methods to raise. They should return a string, as documented.

Special method wrappers generally assume that the wrapped methods work. In particular, repr assumes this, and so do the __repr__ methods of all collections classes. They do not try to hide bugs. Example:

>>> class BadRep:
	def __repr__(self): 1/0

	
>>> br = BadRep()
>>> [br]
Traceback (most recent call last):
  File "<pyshell#8>", line 1, in <module>
    [br]
  File "C:\Programs\Python39\lib\idlelib\rpc.py", line 620, in displayhook
    text = repr(value)
  File "<pyshell#6>", line 2, in __repr__
    def __repr__(self): 1/0
ZeroDivisionError: division by zero

Bugs should be reported, not masked. I don't think that FrameSummary should be an exception to this. Therefore I think that this issue should be closed (along with the PR) as 'not a bug'.

[moi90]

As is true for most special methods, it is a bug for __repr__ methods to raise.

Is this codified anywhere? I only learned about that in this bug report.

[serhiy-storchaka]

__repr__() and repr() can raise exceptions. Silencing arbitrary exception is usually a bad idea. Even correctly raised code can raise exceptions such as MemoryError, RecursionError and KeybordInterrupt.

For this reason I think that the proposed change should not be made.

[iritkatriel]

If we decide to reject this then we should add unit tests that show this state of affairs is deliberate.