diff options
author | Lluis Sanchez <llsan@microsoft.com> | 2019-05-30 21:09:21 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-05-30 21:09:21 +0300 |
commit | 63d55448ec9385d4cc0977194f4a245444027177 (patch) | |
tree | a447efb25b825d8d87d3bbdc13b4eae9bb3dbdca /main | |
parent | b8f5cbf961afdb6a0ccd1dc70b7ec50469aac461 (diff) | |
parent | 5790889871c9add4be64ef2d26a34a3f6ac5c188 (diff) |
Merge pull request #7733 from mono/exception-handling-better
Handle exceptions coming in from glib and objc in a better way
Diffstat (limited to 'main')
3 files changed, 62 insertions, 30 deletions
diff --git a/main/src/addins/MacPlatform/MacPlatform.cs b/main/src/addins/MacPlatform/MacPlatform.cs index f860333842..0b9a6d51d5 100644 --- a/main/src/addins/MacPlatform/MacPlatform.cs +++ b/main/src/addins/MacPlatform/MacPlatform.cs @@ -249,28 +249,26 @@ namespace MonoDevelop.MacIntegration var nsException = ObjCRuntime.Runtime.GetNSObject<NSException> (exceptionPtr); try { - throw new MarshalledObjCException (nsException, Environment.StackTrace); + throw new MarshalledObjCException (nsException); } catch (MarshalledObjCException e) { + LoggingService.LogInternalError ("Unhandled ObjC Exception", e); // Is there a way to figure out if it's going to crash us? Maybe check MarshalObjectiveCExceptionMode and MarshalManagedExceptionMode? - LoggingService.LogInternalError ("Unhandled ObjC exception", e); } + // Invoke the default xamarin.mac one, so if it bubbles up an exception, the caller receives it. oldHandler?.Invoke (exceptionPtr); } sealed class MarshalledObjCException : ObjCException { - public MarshalledObjCException (NSException exception, string stacktrace) : base (exception) + public MarshalledObjCException (NSException exception) : base (exception) { - StackTrace = stacktrace; - } - - public override string StackTrace { get; } + const BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.SetField; - public override string ToString () - { - // Matches normal exception format: - return GetType () + ": " + Message + Environment.NewLine + StackTrace; + var trace = new [] { new StackTrace (true), }; + //// Otherwise exception stacktrace is not gathered. + typeof (Exception) + .InvokeMember ("captured_traces", flags, null, this, new object [] { trace }); } } diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs index 5f6ef79aff..21a4593f6d 100644 --- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs +++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/GLibLogging.cs @@ -33,6 +33,7 @@ using System.Runtime.InteropServices; using System.Collections;
using System.Runtime.ExceptionServices; using System.Runtime.CompilerServices; +using System.Diagnostics; namespace MonoDevelop.Ide.Gui { @@ -255,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: @@ -271,13 +272,12 @@ namespace MonoDevelop.Ide.Gui case LogLevelFlags.Critical: default: try { - // Otherwise exception info is not gathered. - throw new CriticalGtkException (msg, Environment.StackTrace); + 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,19 +287,42 @@ namespace MonoDevelop.Ide.Gui LoggingService.LogError ("Disabling glib logging for the rest of the session"); } + static bool IsMono = Type.GetType ("Mono.Runtime") != null; + + static string GetStacktraceIfNeeded (LogLevelFlags flags) + { + // If it's an Error or a Critical message, we're going to add the stacktrace via the logged exception property, + // we don't need to append it to the message in the log. But we are only doing so on Mono, nowhere else. + if (IsMono && flags.HasFlag (LogLevelFlags.Error | LogLevelFlags.Critical)) + return string.Empty; + + return "Stack trace: \n" + new StackTrace (1, true); + } + sealed class CriticalGtkException : Exception { - public CriticalGtkException (string message, string stacktrace) : base (message) + readonly StackTrace trace = new StackTrace (1, true); + + public CriticalGtkException (string message) : base (message) { - StackTrace = stacktrace; + const System.Reflection.BindingFlags flags = + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.SetField; + + // 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 + if (IsMono) { + typeof (Exception) + .GetField ("captured_traces", flags) + ?.SetValue (this, new StackTrace [] { trace }); + } } - public override string StackTrace { get; } + public override string StackTrace => trace.ToString (); public override string ToString () { // Matches normal exception format: - return GetType() + ": " + Message + Environment.NewLine + StackTrace; + 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 61ac439b6a..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; } } @@ -91,7 +103,6 @@ namespace MonoDevelop.Ide.Gui Assert.That (stacktrace, Contains.Substring ("at GLib.Log.g_logv")); Assert.That (stacktrace, Contains.Substring ("at MonoDevelop.Ide.Gui.GLibLogging.LoggerMethod")); Assert.That (stacktrace, Contains.Substring ("at MonoDevelop.Ide.Gui.GLibLoggingTests")); - } } } |