gh-110721: Use the traceback module for PyErr_Display() and fallback to the C implementation (#110702)

Author: pablogsal

Include/internal/pycore_traceback.h

@@ -95,9 +95,8 @@ extern PyObject* _PyTraceBack_FromFrame(
 
 /* Write the traceback tb to file f. Prefix each line with
    indent spaces followed by the margin (if it is not NULL). */
-extern int _PyTraceBack_Print_Indented(
-    PyObject *tb, int indent, const char* margin,
-    const char *header_margin, const char *header, PyObject *f);
+extern int _PyTraceBack_Print(
+    PyObject *tb, const char *header, PyObject *f);
 extern int _Py_WriteIndentedMargin(int, const char*, PyObject *);
 extern int _Py_WriteIndent(int, PyObject *);
 

Lib/test/test_sys.py

@@ -175,7 +175,8 @@ def test_excepthook_bytes_filename(self):
 
     def test_excepthook(self):
         with test.support.captured_output("stderr") as stderr:
-            sys.excepthook(1, '1', 1)
+            with test.support.catch_unraisable_exception():
+                sys.excepthook(1, '1', 1)
         self.assertTrue("TypeError: print_exception(): Exception expected for " \
                          "value, str found" in stderr.getvalue())
 

Lib/test/test_traceback.py

@@ -963,7 +963,9 @@ class CPythonTracebackLegacyErrorCaretTests(
     Same set of tests as above but with Python's legacy internal traceback printing.
     """
 
-class TracebackFormatTests(unittest.TestCase):
+
+class TracebackFormatMixin:
+    DEBUG_RANGES = True
 
     def some_exception(self):
         raise KeyError('blah')
@@ -1137,6 +1139,8 @@ def g(count=10):
         )
         expected = (tb_line + result_g).splitlines()
         actual = stderr_g.getvalue().splitlines()
+        if not self.DEBUG_RANGES:
+            expected = [line for line in expected if not set(line.strip()) == {"^"}]
         self.assertEqual(actual, expected)
 
         # Check 2 different repetitive sections
@@ -1173,6 +1177,8 @@ def h(count=10):
         )
         expected = (result_h + result_g).splitlines()
         actual = stderr_h.getvalue().splitlines()
+        if not self.DEBUG_RANGES:
+            expected = [line for line in expected if not set(line.strip()) == {"^"}]
         self.assertEqual(actual, expected)
 
         # Check the boundary conditions. First, test just below the cutoff.
@@ -1199,11 +1205,13 @@ def h(count=10):
         )
         tb_line = (
             'Traceback (most recent call last):\n'
-            f'  File "{__file__}", line {lineno_g+77}, in _check_recursive_traceback_display\n'
+            f'  File "{__file__}", line {lineno_g+81}, in _check_recursive_traceback_display\n'
             '    g(traceback._RECURSIVE_CUTOFF)\n'
         )
         expected = (tb_line + result_g).splitlines()
         actual = stderr_g.getvalue().splitlines()
+        if not self.DEBUG_RANGES:
+            expected = [line for line in expected if not set(line.strip()) == {"^"}]
         self.assertEqual(actual, expected)
 
         # Second, test just above the cutoff.
@@ -1231,24 +1239,24 @@ def h(count=10):
         )
         tb_line = (
             'Traceback (most recent call last):\n'
-            f'  File "{__file__}", line {lineno_g+108}, in _check_recursive_traceback_display\n'
+            f'  File "{__file__}", line {lineno_g+114}, in _check_recursive_traceback_display\n'
             '    g(traceback._RECURSIVE_CUTOFF + 1)\n'
         )
         expected = (tb_line + result_g).splitlines()
         actual = stderr_g.getvalue().splitlines()
+        if not self.DEBUG_RANGES:
+            expected = [line for line in expected if not set(line.strip()) == {"^"}]
         self.assertEqual(actual, expected)
 
     @requires_debug_ranges()
-    def test_recursive_traceback_python(self):
-        self._check_recursive_traceback_display(traceback.print_exc)
-
-    @cpython_only
-    @requires_debug_ranges()
-    def test_recursive_traceback_cpython_internal(self):
-        from _testcapi import exception_print
-        def render_exc():
-            exception_print(sys.exception())
-        self._check_recursive_traceback_display(render_exc)
+    def test_recursive_traceback(self):
+        if self.DEBUG_RANGES:
+            self._check_recursive_traceback_display(traceback.print_exc)
+        else:
+            from _testcapi import exception_print
+            def render_exc():
+                exception_print(sys.exception())
+            self._check_recursive_traceback_display(render_exc)
 
     def test_format_stack(self):
         def fmt():
@@ -1321,7 +1329,8 @@ def test_exception_group_deep_recursion_traceback(self):
     def test_print_exception_bad_type_capi(self):
         from _testcapi import exception_print
         with captured_output("stderr") as stderr:
-            exception_print(42)
+            with support.catch_unraisable_exception():
+                exception_print(42)
         self.assertEqual(
             stderr.getvalue(),
             ('TypeError: print_exception(): '
@@ -1345,6 +1354,24 @@ def test_print_exception_bad_type_python(self):
 boundaries = re.compile(
     '(%s|%s)' % (re.escape(cause_message), re.escape(context_message)))
 
+class TestTracebackFormat(unittest.TestCase, TracebackFormatMixin):
+    pass
+
+@cpython_only
+class TestFallbackTracebackFormat(unittest.TestCase, TracebackFormatMixin):
+    DEBUG_RANGES = False
+    def setUp(self) -> None:
+        self.original_unraisable_hook = sys.unraisablehook
+        sys.unraisablehook = lambda *args: None
+        self.original_hook = traceback._print_exception_bltin
+        traceback._print_exception_bltin = lambda *args: 1/0
+        return super().setUp()
+
+    def tearDown(self) -> None:
+        traceback._print_exception_bltin = self.original_hook
+        sys.unraisablehook = self.original_unraisable_hook
+        return super().tearDown()
+
 class BaseExceptionReportingTests:
 
     def get_exception(self, exception_or_callable):

Lib/traceback.py

@@ -125,6 +125,14 @@ def print_exception(exc, /, value=_sentinel, tb=_sentinel, limit=None, \
     te.print(file=file, chain=chain)
 
 
+BUILTIN_EXCEPTION_LIMIT = object()
+
+
+def _print_exception_bltin(exc, /):
+    file = sys.stderr if sys.stderr is not None else sys.__stderr__
+    return print_exception(exc, limit=BUILTIN_EXCEPTION_LIMIT, file=file)
+
+
 def format_exception(exc, /, value=_sentinel, tb=_sentinel, limit=None, \
                      chain=True):
     """Format a stack trace and the exception information.
@@ -406,12 +414,16 @@ def _extract_from_extended_frame_gen(klass, frame_gen, *, limit=None,
         # (frame, (lineno, end_lineno, colno, end_colno)) in the stack.
         # Only lineno is required, the remaining fields can be None if the
         # information is not available.
-        if limit is None:
+        builtin_limit = limit is BUILTIN_EXCEPTION_LIMIT
+        if limit is None or builtin_limit:
             limit = getattr(sys, 'tracebacklimit', None)
             if limit is not None and limit < 0:
                 limit = 0
         if limit is not None:
-            if limit >= 0:
+            if builtin_limit:
+                frame_gen = tuple(frame_gen)
+                frame_gen = frame_gen[len(frame_gen) - limit:]
+            elif limit >= 0:
                 frame_gen = itertools.islice(frame_gen, limit)
             else:
                 frame_gen = collections.deque(frame_gen, maxlen=-limit)
@@ -741,9 +753,9 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None,
                 wrong_name = getattr(exc_value, "name", None)
                 if wrong_name is not None and wrong_name in sys.stdlib_module_names:
                     if suggestion:
-                        self._str += f" Or did you forget to import '{wrong_name}'"
+                        self._str += f" Or did you forget to import '{wrong_name}'?"
                     else:
-                        self._str += f". Did you forget to import '{wrong_name}'"
+                        self._str += f". Did you forget to import '{wrong_name}'?"
         if lookup_lines:
             self._load_lines()
         self.__suppress_context__ = \
@@ -904,7 +916,11 @@ def _format_syntax_error(self, stype):
             if self.offset is not None:
                 offset = self.offset
                 end_offset = self.end_offset if self.end_offset not in {None, 0} else offset
-                if offset == end_offset or end_offset == -1:
+                if self.text and offset > len(self.text):
+                    offset = len(self.text) + 1
+                if self.text and end_offset > len(self.text):
+                    end_offset = len(self.text) + 1
+                if offset >= end_offset or end_offset < 0:
                     end_offset = offset + 1
 
                 # Convert 1-based column offset to 0-based index into stripped text

Misc/NEWS.d/next/Core and Builtins/2023-10-11-16-56-54.gh-issue-110721.afcSsH.rst

@@ -0,0 +1,2 @@
+Use the :mod:`traceback` implementation for the default
+:c:func:`PyErr_Display` functionality. Patch by Pablo Galindo