Add unit tests to ensure stacktraces in non-errors

2 files changed, 46 insertions, 15 deletions

diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs
index 2604d036e7..9aaa7ceddc 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs
@@ -256,7 +256,7 @@ namespace MonoDevelop.Ide.Gui
 				LoggingService.LogError (message, e);
 			}
 
-			string msg = string.Format ("{0}-{1}: {2}", logDomain, logLevel, message);
+			string msg = string.Format ("{0}-{1}: {2}\n{3}", logDomain, logLevel, message, GetStacktraceIfNeeded (logLevel));
 
 			switch (logLevel) {
 			case LogLevelFlags.Debug:
@@ -275,9 +275,9 @@ namespace MonoDevelop.Ide.Gui
 					throw new CriticalGtkException (msg);
 				} catch (CriticalGtkException e) {
 					if (logLevel.HasFlag (LogLevelFlags.FlagFatal))
-						LoggingService.LogFatalError ("Fatal GLib error", e);
+						LoggingService.LogFatalError ($"Fatal {logDomain} error", e);
 					else
-						LoggingService.LogInternalError ("Critical GLib error", e);
+						LoggingService.LogInternalError ($"Critical {logDomain} error", e);
 				}
 				break;
 			}
@@ -287,17 +287,36 @@ namespace MonoDevelop.Ide.Gui
 				LoggingService.LogError ("Disabling glib logging for the rest of the session");
 		}
 
+		static string GetStacktraceIfNeeded (LogLevelFlags flags)
+		{
+			if (flags.HasFlag (LogLevelFlags.Error | LogLevelFlags.Critical))
+				return string.Empty;
+
+			return "Stack trace: \n" + new StackTrace (1, true);
+		}
+
 		sealed class CriticalGtkException : Exception
 		{
+			readonly StackTrace trace = new StackTrace (1, true);
+
 			public CriticalGtkException (string message) : base (message)
 			{
 				const System.Reflection.BindingFlags flags =
 					 System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.SetField;
 
-				var trace = new [] { new StackTrace (true), };
-				//// Otherwise exception stacktrace is not gathered.
+				// HACK: We need to somehow inject our stacktrace, and this is the only way we can
+				// This is not to transform every glib error into a managed exception
 				typeof (Exception)
-					.InvokeMember ("captured_traces", flags, null, this, new object [] { trace } );
+					.GetField ("captured_traces", flags)
+					?.SetValue (this, new StackTrace [] { trace });
+			}
+
+			public override string StackTrace => trace.ToString ();
+
+			public override string ToString ()
+			{
+				// Matches normal exception format:
+				return GetType () + ": " + Message + Environment.NewLine + trace;
 			}
 		}
 	}
diff --git a/main/tests/Ide.Tests/MonoDevelop.Ide.Gui/GLibLoggingTests.cs b/main/tests/Ide.Tests/MonoDevelop.Ide.Gui/GLibLoggingTests.cs
index b49711ed74..b75242e373 100644
--- a/main/tests/Ide.Tests/MonoDevelop.Ide.Gui/GLibLoggingTests.cs
+++ b/main/tests/Ide.Tests/MonoDevelop.Ide.Gui/GLibLoggingTests.cs
@@ -27,6 +27,7 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using MonoDevelop.Core;
+using MonoDevelop.Core.Logging;
 using MonoDevelop.Core.LogReporting;
 using NUnit.Framework;
 using UnitTests;
@@ -65,22 +66,33 @@ namespace MonoDevelop.Ide.Gui
 		}
 
 		[Test]
-		public void CriticalErrorsExceptionsHaveFullStacktracesInLog ()
+		public void GLibLoggingHaveFullStacktracesInLog ()
 		{
 			var old = GLibLogging.Enabled;
-			var logger = new CapturingLogger {
-				EnabledLevel = Core.Logging.EnabledLoggingLevel.Error,
-			};
 
 			try {
-				LoggingService.AddLogger (logger);
 				GLibLogging.Enabled = true;
 
-				GLib.Log.Write ("Gtk", GLib.LogLevelFlags.Critical, "{0}", "critical should be captured");
-				var (_, message) = logger.LogMessages.Single (x => x.Level == Core.Logging.LogLevel.Error);
-				AssertGLibStackTrace (message);
+				var flagsToCheck = new [] {
+					(GLib.LogLevelFlags.Critical, LogLevel.Error),
+					(GLib.LogLevelFlags.Debug, LogLevel.Debug),
+					(GLib.LogLevelFlags.Info, LogLevel.Info),
+					//(GLib.LogLevelFlags.Error, LogLevel.Fatal),
+				};
+
+				foreach (var (glibLevel, coreLevel) in flagsToCheck) {
+					var logger = new CapturingLogger ();
+
+					LoggingService.AddLogger (logger);
+					try {
+						GLib.Log.Write ("Gtk", glibLevel, "{0}: should be captured", glibLevel);
+						var (_, message) = logger.LogMessages.Single (x => x.Level == coreLevel);
+						AssertGLibStackTrace (message);
+					} finally {
+						LoggingService.RemoveLogger (logger.Name);
+					}
+				}
 			} finally {
-				LoggingService.RemoveLogger (logger.Name);
 				GLibLogging.Enabled = old;
 			}
 		}