Issue #22836: Merge exception reporting from 3.5

vadmium · vadmium · commit 8cacc6300258 · 2016-02-28T03:29:51.000Z
diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst
@@ -74,8 +74,8 @@ Printing and clearing
    :meth:`__del__` method.
 
    The function is called with a single argument *obj* that identifies the context
-   in which the unraisable exception occurred. The repr of *obj* will be printed in
-   the warning message.
+   in which the unraisable exception occurred. If possible,
+   the repr of *obj* will be printed in the warning message.
 
 
 Raising exceptions
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
@@ -7,7 +7,7 @@
 import weakref
 import errno
 
-from test.support import (TESTFN, captured_output, check_impl_detail,
+from test.support import (TESTFN, captured_stderr, check_impl_detail,
                           check_warnings, cpython_only, gc_collect, run_unittest,
                           no_tracing, unlink, import_module)
 
@@ -20,6 +20,10 @@ class SlottedNaiveException(Exception):
     def __init__(self, x):
         self.x = x
 
+class BrokenStrException(Exception):
+    def __str__(self):
+        raise Exception("str() is broken")
+
 # XXX This is not really enough, each *operation* should be tested!
 
 class ExceptionTests(unittest.TestCase):
@@ -882,7 +886,7 @@ def __subclasscheck__(cls, subclass):
         class MyException(Exception, metaclass=Meta):
             pass
 
-        with captured_output("stderr") as stderr:
+        with captured_stderr() as stderr:
             try:
                 raise KeyError()
             except MyException as e:
@@ -1011,6 +1015,66 @@ def test_errno_ENOTDIR(self):
             os.listdir(__file__)
         self.assertEqual(cm.exception.errno, errno.ENOTDIR, cm.exception)
 
+    def test_unraisable(self):
+        # Issue #22836: PyErr_WriteUnraisable() should give sensible reports
+        class BrokenDel:
+            def __del__(self):
+                exc = ValueError("del is broken")
+                # The following line is included in the traceback report:
+                raise exc
+
+        class BrokenRepr(BrokenDel):
+            def __repr__(self):
+                raise AttributeError("repr() is broken")
+
+        class BrokenExceptionDel:
+            def __del__(self):
+                exc = BrokenStrException()
+                # The following line is included in the traceback report:
+                raise exc
+
+        for test_class in (BrokenDel, BrokenRepr, BrokenExceptionDel):
+            with self.subTest(test_class):
+                obj = test_class()
+                with captured_stderr() as stderr:
+                    del obj
+                report = stderr.getvalue()
+                self.assertIn("Exception ignored", report)
+                if test_class is BrokenRepr:
+                    self.assertIn("<object repr() failed>", report)
+                else:
+                    self.assertIn(test_class.__del__.__qualname__, report)
+                self.assertIn("test_exceptions.py", report)
+                self.assertIn("raise exc", report)
+                if test_class is BrokenExceptionDel:
+                    self.assertIn("BrokenStrException", report)
+                    self.assertIn("<exception str() failed>", report)
+                else:
+                    self.assertIn("ValueError", report)
+                    self.assertIn("del is broken", report)
+                self.assertTrue(report.endswith("\n"))
+
+    def test_unhandled(self):
+        # Check for sensible reporting of unhandled exceptions
+        for exc_type in (ValueError, BrokenStrException):
+            with self.subTest(exc_type):
+                try:
+                    exc = exc_type("test message")
+                    # The following line is included in the traceback report:
+                    raise exc
+                except exc_type:
+                    with captured_stderr() as stderr:
+                        sys.__excepthook__(*sys.exc_info())
+                report = stderr.getvalue()
+                self.assertIn("test_exceptions.py", report)
+                self.assertIn("raise exc", report)
+                self.assertIn(exc_type.__name__, report)
+                if exc_type is BrokenStrException:
+                    self.assertIn("<exception str() failed>", report)
+                else:
+                    self.assertIn("test message", report)
+                self.assertTrue(report.endswith("\n"))
+
 
 class ImportErrorTests(unittest.TestCase):
 
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -10,6 +10,11 @@ Release date: tba
 Core and Builtins
 -----------------
 
+- Issue #22836: Ensure exception reports from PyErr_Display() and
+  PyErr_WriteUnraisable() are sensible even when formatting them produces
+  secondary errors.  This affects the reports produced by
+  sys.__excepthook__() and when __del__() raises an exception.
+
 - Issue #26302: Correct behavior to reject comma as a legal character for
   cookie names.
 
diff --git a/Python/errors.c b/Python/errors.c
@@ -900,8 +900,12 @@ PyErr_WriteUnraisable(PyObject *obj)
     if (obj) {
         if (PyFile_WriteString("Exception ignored in: ", f) < 0)
             goto done;
-        if (PyFile_WriteObject(obj, f, 0) < 0)
-            goto done;
+        if (PyFile_WriteObject(obj, f, 0) < 0) {
+            PyErr_Clear();
+            if (PyFile_WriteString("<object repr() failed>", f) < 0) {
+                goto done;
+            }
+        }
         if (PyFile_WriteString("\n", f) < 0)
             goto done;
     }
@@ -946,8 +950,12 @@ PyErr_WriteUnraisable(PyObject *obj)
     if (v && v != Py_None) {
         if (PyFile_WriteString(": ", f) < 0)
             goto done;
-        if (PyFile_WriteObject(v, f, Py_PRINT_RAW) < 0)
-            goto done;
+        if (PyFile_WriteObject(v, f, Py_PRINT_RAW) < 0) {
+            PyErr_Clear();
+            if (PyFile_WriteString("<exception str() failed>", f) < 0) {
+                goto done;
+            }
+        }
     }
     if (PyFile_WriteString("\n", f) < 0)
         goto done;
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
@@ -766,8 +766,11 @@ print_exception(PyObject *f, PyObject *value)
         /* only print colon if the str() of the
            object is not the empty string
         */
-        if (s == NULL)
+        if (s == NULL) {
+            PyErr_Clear();
             err = -1;
+            PyFile_WriteString(": <exception str() failed>", f);
+        }
         else if (!PyUnicode_Check(s) ||
             PyUnicode_GetLength(s) != 0)
             err = PyFile_WriteString(": ", f);
@@ -776,6 +779,9 @@ print_exception(PyObject *f, PyObject *value)
         Py_XDECREF(s);
     }
     /* try to write a newline in any case */
+    if (err < 0) {
+        PyErr_Clear();
+    }
     err += PyFile_WriteString("\n", f);
     Py_XDECREF(tb);
     Py_DECREF(value);
