Fix exception string (#1360)

filmor · web-flow · commit 1afae4cf1396 · 2021-01-29T07:50:08.000+01:00
Fix exception stacktrace formatting Ensure that the stacktrace list is joined into a single string and adjust the formatting slightly such that the .NET stacktrace is separated from the one from Python. The Python traceback is also reversed to match the order used in .NET. Fixes #1359.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,22 +39,23 @@ or the DLL must be loaded in advance. This must be done before calling any other
 
 ### Fixed
 
--    Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
--    Fix incorrect dereference in params array handling
--    Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097])
--    Fix `object[]` parameters taking precedence when should not in overload resolution
--    Fixed a bug where all .NET class instances were considered Iterable
--    Fix incorrect choice of method to invoke when using keyword arguments.
--    Fix non-delegate types incorrectly appearing as callable.
--    Indexers can now be used with interface objects
--    Fixed a bug where indexers could not be used if they were inherited
--    Made it possible to use `__len__` also on `ICollection<>` interface objects
--    Fixed issue when calling PythonException.Format where another exception would be raise for unnormalized exceptions
--    Made it possible to call `ToString`, `GetHashCode`, and `GetType` on inteface objects
--    Fixed objects returned by enumerating `PyObject` being disposed too soon
--    Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException ([#1325][i1325])
--    `import` may now raise errors with more detail than "No module named X"
--    Providing an invalid type parameter to a generic type or method produces a helpful Python error
+-   Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
+-   Fix incorrect dereference in params array handling
+-   Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097])
+-   Fix `object[]` parameters taking precedence when should not in overload resolution
+-   Fixed a bug where all .NET class instances were considered Iterable
+-   Fix incorrect choice of method to invoke when using keyword arguments.
+-   Fix non-delegate types incorrectly appearing as callable.
+-   Indexers can now be used with interface objects
+-   Fixed a bug where indexers could not be used if they were inherited
+-   Made it possible to use `__len__` also on `ICollection<>` interface objects
+-   Fixed issue when calling PythonException.Format where another exception would be raise for unnormalized exceptions
+-   Made it possible to call `ToString`, `GetHashCode`, and `GetType` on inteface objects
+-   Fixed objects returned by enumerating `PyObject` being disposed too soon
+-   Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException
+-   `import` may now raise errors with more detail than "No module named X"
+-   Exception stacktraces on `PythonException.StackTrace` are now properly formatted
+-   Providing an invalid type parameter to a generic type or method produces a helpful Python error
 
 ### Removed
 
diff --git a/src/embed_tests/TestPythonException.cs b/src/embed_tests/TestPythonException.cs
@@ -65,7 +65,21 @@ public void TestPythonExceptionFormat()
             }
             catch (PythonException ex)
             {
-                Assert.That(ex.Format(), Does.Contain("Traceback").And.Contains("(most recent call last):").And.Contains("ValueError: Error!"));
+                // Console.WriteLine($"Format: {ex.Format()}");
+                // Console.WriteLine($"Stacktrace: {ex.StackTrace}");
+                Assert.That(
+                    ex.Format(),
+                    Does.Contain("Traceback")
+                    .And.Contains("(most recent call last):")
+                    .And.Contains("ValueError: Error!")
+                );
+
+                // Check that the stacktrace is properly formatted
+                Assert.That(
+                    ex.StackTrace,
+                    Does.Not.StartWith("[")
+                    .And.Not.Contain("\\n")
+                );
             }
         }
 
diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Text;
 
@@ -13,9 +14,9 @@ public class PythonException : System.Exception, IDisposable
         private IntPtr _pyType = IntPtr.Zero;
         private IntPtr _pyValue = IntPtr.Zero;
         private IntPtr _pyTB = IntPtr.Zero;
-        private string _tb = "";
-        private string _message = "";
-        private string _pythonTypeName = "";
+        private readonly string _tb = "";
+        private readonly string _message = "";
+        private readonly string _pythonTypeName = "";
         private bool disposed = false;
         private bool _finalized = false;
 
@@ -44,16 +45,23 @@ public PythonException()
                 }
                 _message = type + " : " + message;
             }
+
             if (_pyTB != IntPtr.Zero)
             {
-                using (PyObject tb_module = PythonEngine.ImportModule("traceback"))
-                {
-                    Runtime.XIncref(_pyTB);
-                    using (var pyTB = new PyObject(_pyTB))
-                    {
-                        _tb = tb_module.InvokeMethod("format_tb", pyTB).ToString();
-                    }
+                using PyObject tb_module = PythonEngine.ImportModule("traceback");
+
+                Runtime.XIncref(_pyTB);
+                using var pyTB = new PyObject(_pyTB);
+
+                using var tbList = tb_module.InvokeMethod("format_tb", pyTB);
+
+                var sb = new StringBuilder();
+                // Reverse Python's traceback list to match the order used in C#
+                // stacktraces
+                foreach (var line in tbList.Reverse()) {
+                    sb.Append(line.ToString());
                 }
+                _tb = sb.ToString();
             }
             PythonEngine.ReleaseLock(gs);
         }
@@ -136,10 +144,7 @@ public override string Message
         /// <remarks>
         /// A string representing the python exception stack trace.
         /// </remarks>
-        public override string StackTrace
-        {
-            get { return _tb + base.StackTrace; }
-        }
+        public override string StackTrace => $"{_tb}===\n{base.StackTrace}";
 
         /// <summary>
         /// Python error type name.

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,21 @@ public void TestPythonExceptionFormat()`
`65`	`65`	`}`
`66`	`66`	`catch (PythonException ex)`
`67`	`67`	`{`
`68`		`- Assert.That(ex.Format(), Does.Contain("Traceback").And.Contains("(most recent call last):").And.Contains("ValueError: Error!"));`
	`68`	`+ // Console.WriteLine($"Format: {ex.Format()}");`
	`69`	`+ // Console.WriteLine($"Stacktrace: {ex.StackTrace}");`
	`70`	`+ Assert.That(`
	`71`	`+ ex.Format(),`
	`72`	`+ Does.Contain("Traceback")`
	`73`	`+ .And.Contains("(most recent call last):")`
	`74`	`+ .And.Contains("ValueError: Error!")`
	`75`	`+ );`
	`76`	`+`
	`77`	`+ // Check that the stacktrace is properly formatted`
	`78`	`+ Assert.That(`
	`79`	`+ ex.StackTrace,`
	`80`	`+ Does.Not.StartWith("[")`
	`81`	`+ .And.Not.Contain("\\n")`
	`82`	`+ );`
`69`	`83`	`}`
`70`	`84`	`}`
`71`	`85`