Issue #22836: Keep exception reports sensible despite errors

5 files changed, 87 insertions, 9 deletions

diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst
index 65249e1..10e2c4c 100644
--- a/Doc/c-api/exceptions.rst
+++ b/Doc/c-api/exceptions.rst
@@ -404,8 +404,8 @@ is a separate error indicator for each thread.
    :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.
 
 
 .. _unicodeexceptions:
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
index b950031..bc8944d 100644
--- a/Lib/test/test_exceptions.py
+++ b/Lib/test/test_exceptions.py
@@ -5,10 +5,15 @@ import sys
 import unittest
 import pickle, cPickle
 
-from test.test_support import (TESTFN, unlink, run_unittest, captured_output,
+from test.test_support import (TESTFN, unlink, run_unittest, captured_stderr,
                                check_warnings, cpython_only)
 from test.test_pep352 import ignore_deprecation_warnings
 
+class BrokenStrException(Exception):
+    def __str__(self):
+        raise Exception("str() is broken")
+    __repr__ = __str__  # Python 2's PyErr_WriteUnraisable() uses repr()
+
 # XXX This is not really enough, each *operation* should be tested!
 
 class ExceptionTests(unittest.TestCase):
@@ -375,7 +380,7 @@ class ExceptionTests(unittest.TestCase):
         # The test prints an unraisable recursion error when
         # doing "except ValueError", this is because subclass
         # checking has recursion checking too.
-        with captured_output("stderr"):
+        with captured_stderr():
             try:
                 g()
             except RuntimeError:
@@ -448,7 +453,7 @@ class ExceptionTests(unittest.TestCase):
             __metaclass__ = Meta
             pass
 
-        with captured_output("stderr") as stderr:
+        with captured_stderr() as stderr:
             try:
                 raise KeyError()
             except MyException, e:
@@ -460,7 +465,7 @@ class ExceptionTests(unittest.TestCase):
             else:
                 self.fail("Should have raised KeyError")
 
-        with captured_output("stderr") as stderr:
+        with captured_stderr() as stderr:
             def g():
                 try:
                     return g()
@@ -644,6 +649,62 @@ class TestSameStrAndUnicodeMsg(unittest.TestCase):
         self.assertEqual(error5.a, 1)
         self.assertEqual(error5.__doc__, "")
 
+    def test_unraisable(self):
+        # Issue #22836: PyErr_WriteUnraisable() should give sensible reports
+        class BrokenDel:
+            def __del__(self):
+                exc = ValueError("del is broken")
+                # In Python 3, the following line would be in the report:
+                raise exc
+
+        class BrokenRepr(BrokenDel):
+            def __repr__(self):
+                raise AttributeError("repr() is broken")
+
+        class BrokenExceptionDel:
+            def __del__(self):
+                exc = BrokenStrException()
+                # In Python 3, the following line would be in the report:
+                raise exc
+
+        for test_class in (BrokenDel, BrokenRepr, BrokenExceptionDel):
+            obj = test_class()
+            with captured_stderr() as stderr:
+                del obj
+            report = stderr.getvalue()
+            self.assertRegexpMatches(report, "Exception.* ignored")
+            if test_class is BrokenRepr:
+                self.assertIn("<object repr() failed>", report)
+            else:
+                self.assertIn("__del__", report)
+            if test_class is BrokenExceptionDel:
+                self.assertIn("BrokenStrException", report)
+                self.assertIn("<exception repr() 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):
+            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"))
+
 
 def test_main():
     run_unittest(ExceptionTests, TestSameStrAndUnicodeMsg)
diff --git a/Misc/NEWS b/Misc/NEWS
index 49d3fd1..012f7d7 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,11 @@ What's New in Python 2.7.12?
 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 #22847: Improve method cache efficiency.
 
 - Issue #25843: When compiling code, don't merge constants if they are equal
diff --git a/Python/errors.c b/Python/errors.c
index 5cbf5c9..e7c221c 100644
--- a/Python/errors.c
+++ b/Python/errors.c
@@ -696,12 +696,18 @@ PyErr_WriteUnraisable(PyObject *obj)
                 PyFile_WriteString(className, f);
             if (v && v != Py_None) {
                 PyFile_WriteString(": ", f);
-                PyFile_WriteObject(v, f, 0);
+                if (PyFile_WriteObject(v, f, 0) < 0) {
+                    PyErr_Clear();
+                    PyFile_WriteString("<exception repr() failed>", f);
+                }
             }
             Py_XDECREF(moduleName);
         }
         PyFile_WriteString(" in ", f);
-        PyFile_WriteObject(obj, f, 0);
+        if (PyFile_WriteObject(obj, f, 0) < 0) {
+            PyErr_Clear();
+            PyFile_WriteString("<object repr() failed>", f);
+        }
         PyFile_WriteString(" ignored\n", f);
         PyErr_Clear(); /* Just in case */
     }
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index ece709c..9b5f62d 100644
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -1299,8 +1299,11 @@ PyErr_Display(PyObject *exception, PyObject *value, PyObject *tb)
             /* 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 (!PyString_Check(s) ||
                      PyString_GET_SIZE(s) != 0)
                 err = PyFile_WriteString(": ", f);
@@ -1309,6 +1312,9 @@ PyErr_Display(PyObject *exception, PyObject *value, PyObject *tb)
             Py_XDECREF(s);
         }
         /* try to write a newline in any case */
+        if (err < 0) {
+            PyErr_Clear();
+        }
         err += PyFile_WriteString("\n", f);
     }
     Py_DECREF(value);