From e86bff3bed254061266872e579359144a05e724a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 7 Jan 2021 20:09:31 +0000 Subject: [PATCH 1/7] refactor into tail-recursion --- Lib/test/test_traceback.py | 18 ++++++++++++++ Lib/traceback.py | 48 +++++++++++++++++--------------------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index abb5762cd43efb2..fae31c2000e6931 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1148,6 +1148,24 @@ def test_context(self): self.assertEqual(exc_info[0], exc.exc_type) self.assertEqual(str(exc_info[1]), str(exc)) + @unittest.skip("failing") + def test_long_context_chain(self): + def f(): + try: + 1/0 + except: + f() + + try: + f() + except RecursionError: + exc_info = sys.exc_info() + else: + self.fail("Exception not raised") + + te = traceback.TracebackException(*exc_info) + res = list(te.format()) + def test_no_refs_to_exception_and_traceback_objects(self): try: 1/0 diff --git a/Lib/traceback.py b/Lib/traceback.py index 4e008bc0e081a36..406f415c05a9408 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -484,6 +484,26 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, if _seen is None: _seen = set() _seen.add(id(exc_value)) + # TODO: locals. + self.stack = StackSummary.extract( + walk_tb(exc_traceback), limit=limit, lookup_lines=lookup_lines, + capture_locals=capture_locals) + self.exc_type = exc_type + # Capture now to permit freeing resources: only complication is in the + # unofficial API _format_final_exc_line + self._str = _some_str(exc_value) + if exc_type and issubclass(exc_type, SyntaxError): + # Handle SyntaxError's specially + self.filename = exc_value.filename + lno = exc_value.lineno + self.lineno = str(lno) if lno is not None else None + self.text = exc_value.text + self.offset = exc_value.offset + self.msg = exc_value.msg + if lookup_lines: + self._load_lines() + self.__suppress_context__ = \ + exc_value.__suppress_context__ if exc_value else False # Gracefully handle (the way Python 2.4 and earlier did) the case of # being called with no type or value (None, None, None). if (exc_value and exc_value.__cause__ is not None @@ -493,7 +513,7 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, exc_value.__cause__, exc_value.__cause__.__traceback__, limit=limit, - lookup_lines=False, + lookup_lines=lookup_lines, capture_locals=capture_locals, _seen=_seen) else: @@ -505,33 +525,13 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, exc_value.__context__, exc_value.__context__.__traceback__, limit=limit, - lookup_lines=False, + lookup_lines=lookup_lines, capture_locals=capture_locals, _seen=_seen) else: context = None self.__cause__ = cause self.__context__ = context - self.__suppress_context__ = \ - exc_value.__suppress_context__ if exc_value else False - # TODO: locals. - self.stack = StackSummary.extract( - walk_tb(exc_traceback), limit=limit, lookup_lines=lookup_lines, - capture_locals=capture_locals) - self.exc_type = exc_type - # Capture now to permit freeing resources: only complication is in the - # unofficial API _format_final_exc_line - self._str = _some_str(exc_value) - if exc_type and issubclass(exc_type, SyntaxError): - # Handle SyntaxError's specially - self.filename = exc_value.filename - lno = exc_value.lineno - self.lineno = str(lno) if lno is not None else None - self.text = exc_value.text - self.offset = exc_value.offset - self.msg = exc_value.msg - if lookup_lines: - self._load_lines() @classmethod def from_exception(cls, exc, *args, **kwargs): @@ -542,10 +542,6 @@ def _load_lines(self): """Private API. force all lines in the stack to be loaded.""" for frame in self.stack: frame.line - if self.__context__: - self.__context__._load_lines() - if self.__cause__: - self.__cause__._load_lines() def __eq__(self, other): if isinstance(other, TracebackException): From 03cb5393b2316b1bbd4c016fd6a83b4360bc3af7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 7 Jan 2021 23:31:25 +0000 Subject: [PATCH 2/7] remove recursion from TracebackException constructor --- Lib/test/test_traceback.py | 3 +- Lib/traceback.py | 68 ++++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index fae31c2000e6931..cf735cff08cd858 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1148,7 +1148,6 @@ def test_context(self): self.assertEqual(exc_info[0], exc.exc_type) self.assertEqual(str(exc_info[1]), str(exc)) - @unittest.skip("failing") def test_long_context_chain(self): def f(): try: @@ -1164,6 +1163,8 @@ def f(): self.fail("Exception not raised") te = traceback.TracebackException(*exc_info) + + return # format() is still recursive res = list(te.format()) def test_no_refs_to_exception_and_traceback_objects(self): diff --git a/Lib/traceback.py b/Lib/traceback.py index 406f415c05a9408..6c1d5cc302723c9 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -481,6 +481,7 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, # permit backwards compat with the existing API, otherwise we # need stub thunk objects just to glue it together. # Handle loops in __cause__ or __context__. + _is_chained = _seen is not None if _seen is None: _seen = set() _seen.add(id(exc_value)) @@ -504,34 +505,45 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, self._load_lines() self.__suppress_context__ = \ exc_value.__suppress_context__ if exc_value else False - # Gracefully handle (the way Python 2.4 and earlier did) the case of - # being called with no type or value (None, None, None). - if (exc_value and exc_value.__cause__ is not None - and id(exc_value.__cause__) not in _seen): - cause = TracebackException( - type(exc_value.__cause__), - exc_value.__cause__, - exc_value.__cause__.__traceback__, - limit=limit, - lookup_lines=lookup_lines, - capture_locals=capture_locals, - _seen=_seen) - else: - cause = None - if (exc_value and exc_value.__context__ is not None - and id(exc_value.__context__) not in _seen): - context = TracebackException( - type(exc_value.__context__), - exc_value.__context__, - exc_value.__context__.__traceback__, - limit=limit, - lookup_lines=lookup_lines, - capture_locals=capture_locals, - _seen=_seen) - else: - context = None - self.__cause__ = cause - self.__context__ = context + + # Convert __cause__ and __context__ to `TracebackExceptions`s, use a + # queue to avoid recursion + if not _is_chained: + queue = [(self, exc_value)] + while queue: + te, e = queue.pop() + # Gracefully handle (the way Python 2.4 and earlier did) the + # case of being called with no type or value (None, None, None) + if (e and e.__cause__ is not None + and id(e.__cause__) not in _seen): + cause = TracebackException( + type(e.__cause__), + e.__cause__, + e.__cause__.__traceback__, + limit=limit, + lookup_lines=lookup_lines, + capture_locals=capture_locals, + _seen=_seen) + else: + cause = None + if (e and e.__context__ is not None + and id(e.__context__) not in _seen): + context = TracebackException( + type(e.__context__), + e.__context__, + e.__context__.__traceback__, + limit=limit, + lookup_lines=lookup_lines, + capture_locals=capture_locals, + _seen=_seen) + else: + context = None + te.__cause__ = cause + te.__context__ = context + if cause: + queue.append((te.__cause__, e.__cause__)) + if context: + queue.append((te.__context__, e.__context__)) @classmethod def from_exception(cls, exc, *args, **kwargs): From 5ba241ab0c4a5040bda63cd7e1b4304e24171607 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 8 Jan 2021 00:28:47 +0000 Subject: [PATCH 3/7] remove recursion from format() --- Lib/test/test_traceback.py | 10 ++++++++-- Lib/traceback.py | 38 ++++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index cf735cff08cd858..07555a0411a0821 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1163,10 +1163,16 @@ def f(): self.fail("Exception not raised") te = traceback.TracebackException(*exc_info) - - return # format() is still recursive res = list(te.format()) + # many ZeroDiv errors followed by the RecursionError + self.assertGreater(len(res), sys.getrecursionlimit()) + self.assertGreater( + len([l for l in res if 'ZeroDivisionError:' in l]), + sys.getrecursionlimit() * 0.5) + self.assertIn( + "RecursionError: maximum recursion depth exceeded", res[-1]) + def test_no_refs_to_exception_and_traceback_objects(self): try: 1/0 diff --git a/Lib/traceback.py b/Lib/traceback.py index 6c1d5cc302723c9..65eabed6ad4defc 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -630,15 +630,29 @@ def format(self, *, chain=True): The message indicating which exception occurred is always the last string in the output. """ - if chain: - if self.__cause__ is not None: - yield from self.__cause__.format(chain=chain) - yield _cause_message - elif (self.__context__ is not None and - not self.__suppress_context__): - yield from self.__context__.format(chain=chain) - yield _context_message - if self.stack: - yield 'Traceback (most recent call last):\n' - yield from self.stack.format() - yield from self.format_exception_only() + def queue_entry_for_exc(exc): + context = exc.__context__ if not exc.__suppress_context__ else None + return [exc.__cause__, context, exc.stack, exc] + + queue = [queue_entry_for_exc(self)] + while queue: + next = queue.pop() + if isinstance(next, str): + yield next + else: + cause, context, stack, exc = next + if chain: + if cause is not None: + queue.append([None, context, stack, exc]) + queue.append(_cause_message) + queue.append(queue_entry_for_exc(cause)) + continue + elif context is not None: + queue.append([None, None, stack, exc]) + queue.append(_context_message) + queue.append(queue_entry_for_exc(context)) + continue + if stack: + yield 'Traceback (most recent call last):\n' + yield from stack.format() + yield from exc.format_exception_only() From 0902d78d65113ae08d250bac6c2dae7e3aa74fab Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 11 Jan 2021 14:59:38 +0000 Subject: [PATCH 4/7] simplify the code based on the chained exceptions rendered are a list rather than a tree --- Lib/traceback.py | 53 +++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/Lib/traceback.py b/Lib/traceback.py index 65eabed6ad4defc..d9201782f544094 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -630,29 +630,32 @@ def format(self, *, chain=True): The message indicating which exception occurred is always the last string in the output. """ - def queue_entry_for_exc(exc): - context = exc.__context__ if not exc.__suppress_context__ else None - return [exc.__cause__, context, exc.stack, exc] - - queue = [queue_entry_for_exc(self)] - while queue: - next = queue.pop() - if isinstance(next, str): - yield next + + stack = [] + exc = self + while exc: + if chain: + if exc.__cause__: + chained_msg = _cause_message + chained_exc = exc.__cause__ + elif exc.__context__ and not exc.__suppress_context__: + chained_msg = _context_message + chained_exc = exc.__context__ + else: + chained_msg = None + chained_exc = None + + stack.append((chained_msg, exc)) + exc = chained_exc else: - cause, context, stack, exc = next - if chain: - if cause is not None: - queue.append([None, context, stack, exc]) - queue.append(_cause_message) - queue.append(queue_entry_for_exc(cause)) - continue - elif context is not None: - queue.append([None, None, stack, exc]) - queue.append(_context_message) - queue.append(queue_entry_for_exc(context)) - continue - if stack: - yield 'Traceback (most recent call last):\n' - yield from stack.format() - yield from exc.format_exception_only() + stack.append((None, exc)) + exc = None + + while stack: + msg, exc = stack.pop() + if msg is not None: + yield msg + if exc.stack: + yield 'Traceback (most recent call last):\n' + yield from exc.stack.format() + yield from exc.format_exception_only() From c40a6d577103de9a9bec6028ee10a972a91699bd Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 12 Jan 2021 11:16:13 +0000 Subject: [PATCH 5/7] Update Lib/traceback.py Co-authored-by: Guido van Rossum --- Lib/traceback.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/traceback.py b/Lib/traceback.py index d9201782f544094..9a4fb4ac6adfb26 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -506,7 +506,7 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, self.__suppress_context__ = \ exc_value.__suppress_context__ if exc_value else False - # Convert __cause__ and __context__ to `TracebackExceptions`s, use a + # Convert __cause__ and __context__ to TracebackExceptions, use a # queue to avoid recursion if not _is_chained: queue = [(self, exc_value)] From 40ada8920e84d1e5e0c49119839c8b439d65df78 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 12 Jan 2021 11:40:36 +0000 Subject: [PATCH 6/7] address review comments --- Lib/traceback.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/Lib/traceback.py b/Lib/traceback.py index 9a4fb4ac6adfb26..aef37c9a7af684e 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -481,7 +481,7 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, # permit backwards compat with the existing API, otherwise we # need stub thunk objects just to glue it together. # Handle loops in __cause__ or __context__. - _is_chained = _seen is not None + is_recursive_call = _seen is not None if _seen is None: _seen = set() _seen.add(id(exc_value)) @@ -506,14 +506,12 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, self.__suppress_context__ = \ exc_value.__suppress_context__ if exc_value else False - # Convert __cause__ and __context__ to TracebackExceptions, use a - # queue to avoid recursion - if not _is_chained: + # Convert __cause__ and __context__ to `TracebackExceptions`s, use a + # queue to avoid recursion (only the top-level call gets _seen == None) + if not is_recursive_call: queue = [(self, exc_value)] while queue: te, e = queue.pop() - # Gracefully handle (the way Python 2.4 and earlier did) the - # case of being called with no type or value (None, None, None) if (e and e.__cause__ is not None and id(e.__cause__) not in _seen): cause = TracebackException( @@ -631,28 +629,28 @@ def format(self, *, chain=True): string in the output. """ - stack = [] + output = [] exc = self while exc: if chain: - if exc.__cause__: + if exc.__cause__ is not None: chained_msg = _cause_message chained_exc = exc.__cause__ - elif exc.__context__ and not exc.__suppress_context__: + elif (exc.__context__ is not None and + not exc.__suppress_context__): chained_msg = _context_message chained_exc = exc.__context__ else: chained_msg = None chained_exc = None - stack.append((chained_msg, exc)) + output.append((chained_msg, exc)) exc = chained_exc else: - stack.append((None, exc)) + output.append((None, exc)) exc = None - while stack: - msg, exc = stack.pop() + for msg, exc in reversed(output): if msg is not None: yield msg if exc.stack: From f1591cb17be608d37478beeb5a66338fb6891c45 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 12 Jan 2021 19:34:07 +0000 Subject: [PATCH 7/7] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst diff --git a/Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst b/Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst new file mode 100644 index 000000000000000..4490b6ae3405e5d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst @@ -0,0 +1 @@ +Removed recursion from :class:`~traceback.TracebackException` to allow it to handle long exception chains. \ No newline at end of file