crash report fixes (#15147)

pinzart90 · web-flow · commit 59e22d86a732 · 2024-04-29T15:38:55.000-04:00
* crash report fixes

* update

* update

* Update DynamoModel.cs

* Update PackageManagerViewExtensionTests.cs

* update

* update

* Update PackageManagerViewExtensionTests.cs

* update

* update

* Update PackageManagerViewExtensionTests.cs

* more logs

* Update DynamoViewModel.cs

* Update DynamoViewModel.cs

* update

* Update DynamoViewModel.cs

* update

* Update DynamoViewModel.cs

* Create TestCrash.dll
diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs
@@ -974,6 +974,7 @@ protected DynamoModel(IStartConfiguration config)
             LogWarningMessageEvents.LogWarningMessage += LogWarningMessage;
             LogWarningMessageEvents.LogInfoMessage += LogInfoMessage;
             DynamoConsoleLogger.LogMessageToDynamoConsole += LogMessageWrapper;
+            DynamoConsoleLogger.LogErrorToDynamoConsole += LogErrorMessageWrapper;
             StartBackupFilesTimer();
 
             TraceReconciliationProcessor = this;
@@ -1425,6 +1426,7 @@ public void Dispose()
             LogWarningMessageEvents.LogWarningMessage -= LogWarningMessage;
             LogWarningMessageEvents.LogInfoMessage -= LogInfoMessage;
             DynamoConsoleLogger.LogMessageToDynamoConsole -= LogMessageWrapper;
+            DynamoConsoleLogger.LogErrorToDynamoConsole -= LogErrorMessageWrapper;
             foreach (var ws in _workspaces)
             {
                 ws.Dispose();
@@ -3296,11 +3298,17 @@ private void LogMessage(ILogMessage obj)
         {
             Logger.Log(obj);
         }
+
         private void LogMessageWrapper(string m)
         {
             LogMessage(Dynamo.Logging.LogMessage.Info(m));
         }
 
+        private void LogErrorMessageWrapper(string m)
+        {
+            LogMessage(Dynamo.Logging.LogMessage.Error(m));
+        }
+
 #if DEBUG_LIBRARY
         private void DumpLibrarySnapshot(IEnumerable<Engine.FunctionGroup> functionGroups)
         {
diff --git a/src/DynamoCoreWpf/UI/Prompts/CrashPrompt.xaml.cs b/src/DynamoCoreWpf/UI/Prompts/CrashPrompt.xaml.cs
@@ -11,6 +11,7 @@
 using System.Windows.Markup;
 using System.Xml;
 using Dynamo.Core;
+using Dynamo.Models;
 using Dynamo.PackageManager;
 using Dynamo.ViewModels;
 
@@ -47,11 +48,25 @@ public CrashPrompt(DynamoViewModel dynamoViewModel)
             txtOverridingText.Text = string.Format(Wpf.Properties.Resources.CrashPromptDialogCrashMessage, productName);
         }
 
-        public CrashPrompt(CrashPromptArgs args, DynamoViewModel dynamoViewModel)
+        public CrashPrompt(CrashPromptArgs args, DynamoViewModel dynamoViewModel) : this(dynamoViewModel, args)
+        {}
+
+        internal CrashPrompt(object sender, CrashPromptArgs args)
         {
+            DynamoModel model = null;
+            var dynamoViewModel = sender as DynamoViewModel;
+            if (dynamoViewModel != null)
+            {
+                model = dynamoViewModel.Model;
+            }
+            else if (sender is DynamoModel dm)
+            {
+                model = dm;
+            }
+
             InitializeComponent();
 
-            var packageLoader = dynamoViewModel?.Model?.GetPackageManagerExtension()?.PackageLoader;
+            var packageLoader = model?.GetPackageManagerExtension()?.PackageLoader;
             markdownPackages = Wpf.Utilities.CrashUtilities.PackagesToMakrdown(packageLoader);
 
             productName = dynamoViewModel?.BrandingResourceProvider.ProductName ?? Process.GetCurrentProcess().ProcessName;
diff --git a/src/DynamoCoreWpf/Utilities/CrashReportTool.cs b/src/DynamoCoreWpf/Utilities/CrashReportTool.cs
@@ -164,33 +164,35 @@ private static string FindCERToolInInstallLocations()
 
         internal static void ShowCrashWindow(object sender, CrashPromptArgs args)
         {
-            var viewModel = sender as DynamoViewModel;
-
-            if (CrashReportTool.ShowCrashErrorReportWindow(viewModel,
+            if (CrashReportTool.ShowCrashErrorReportWindow(sender,
                 (args is CrashErrorReportArgs cerArgs) ? cerArgs :
                 new CrashErrorReportArgs(args.Details)))
             {
                 return;
             }
             // Backup crash reporting dialog (in case ADSK CER is not found)
-            var prompt = new Nodes.Prompts.CrashPrompt(args, viewModel);
+            var prompt = new Nodes.Prompts.CrashPrompt(sender, args);
             prompt.ShowDialog();
         }
 
         /// <summary>
         /// Calls external CER tool (with UI)
         /// </summary>
-        /// <param name="viewModel">The Dynamo view model</param>
-        /// <param name="args"></param>
+        /// <param name="sender">Either a DynamoViewModel or a DynamoModel object</param>
+        /// <param name="args">Arguments consumed by the Crash report UI</param>
         /// <returns>True if the CER tool process was successfully started. False otherwise</returns>
-        internal static bool ShowCrashErrorReportWindow(DynamoViewModel viewModel, CrashErrorReportArgs args)
+        private static bool ShowCrashErrorReportWindow(object sender, CrashErrorReportArgs args)
         {
-            if (DynamoModel.FeatureFlags?.CheckFeatureFlag("CER_v2", false) == false)
+            DynamoModel model = null;
+            var viewModel = sender as DynamoViewModel;
+            if (viewModel != null)
             {
-                return false;
+                model = viewModel.Model;
+            }
+            else if (sender is DynamoModel dm)
+            {
+                model = dm;
             }
-
-            DynamoModel model = viewModel?.Model;
 
             string cerToolDir = !string.IsNullOrEmpty(model?.CERLocation) ?
                 model?.CERLocation : FindCERToolInInstallLocations();
diff --git a/src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs b/src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs
@@ -8,6 +8,7 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Runtime.Loader;
 using System.Threading.Tasks;
 using System.Windows;
 using System.Windows.Controls;
@@ -46,8 +47,10 @@
 using Dynamo.Wpf.ViewModels.FileTrust;
 using Dynamo.Wpf.ViewModels.Watch3D;
 using DynamoMLDataPipeline;
+using DynamoServices;
 using DynamoUtilities;
 using ICSharpCode.AvalonEdit;
+using J2N.Text;
 using Newtonsoft.Json;
 using PythonNodeModels;
 using ISelectable = Dynamo.Selection.ISelectable;
@@ -848,10 +851,18 @@ private void CurrentDispatcher_UnhandledException(object sender, DispatcherUnhan
                 return;
             }
 
-            // Try to handle the exception so that the host app can continue (in most cases).
-            // In some cases Dynamo code might still crash after this handler kicks in. In these edge cases
-            // we might see 2 CER windows (the extra one from the host app) - CER tool might handle this in the future.
-            e.Handled = true;
+            if (DynamoModel.IsTestMode)
+            {
+                // Do not handle the exception in test mode.
+                // Let the test host handle it.
+            }
+            else
+            {
+                // Try to handle the exception so that the host app can continue (in most cases).
+                // In some cases Dynamo code might still crash after this handler kicks in. In these edge cases
+                // we might see 2 CER windows (the extra one from the host app) - CER tool might handle this in the future.
+                e.Handled = true;
+            }
 
             CrashGracefully(e.Exception);
         }
@@ -864,14 +875,45 @@ private void CurrentDomain_UnhandledException(object sender, UnhandledExceptionE
             }
         }
 
-        private void CrashGracefully(Exception ex, bool fatal = false)
+        // CrashGracefully should only be used in the DynamoViewModel class or within tests.
+        internal void CrashGracefully(Exception ex, bool fatal = false)
         {
             try
             {
+                var exceptionAssembly = ex.TargetSite?.Module?.Assembly;
+                // TargetInvocationException is the exception that is thrown by methods invoked through reflection
+                // The inner exception contains the originating exception.
+                // https://learn.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/7.0/reflection-invoke-exceptions
+                if (ex is TargetInvocationException && ex.InnerException != null)
+                {
+                    exceptionAssembly = ex.InnerException?.TargetSite?.Module?.Assembly;
+                }
+                
+                // Do not crash if the exception is coming from a 3d party package; 
+                if (!fatal && exceptionAssembly != null)
+                {
+                    // Check if the exception might be coming from a loaded package assembly.
+                    var faultyPkg = Model.GetPackageManagerExtension()?.PackageLoader?.LocalPackages?.FirstOrDefault(p => exceptionAssembly.Location.StartsWith(p.RootDirectory, StringComparison.OrdinalIgnoreCase));
+                    if (faultyPkg != null)
+                    {
+                        var crashDetails = new CrashErrorReportArgs(ex);
+                        DynamoConsoleLogger.OnLogErrorToDynamoConsole($"Unhandled exception coming from package {faultyPkg.Name} was handled: {crashDetails.Details}");
+                        Analytics.TrackException(ex, false);
+                        return;
+                    }
+                }
+
                 DynamoModel.IsCrashing = true;
                 var crashData = new CrashErrorReportArgs(ex);
-                Model?.Logger?.LogError($"Unhandled exception: {crashData.Details} ");
+                DynamoConsoleLogger.OnLogErrorToDynamoConsole($"Unhandled exception: {crashData.Details} ");
                 Analytics.TrackException(ex, true);
+
+                if (DynamoModel.IsTestMode)
+                {
+                    // Do not show the crash UI during tests.
+                    return;
+                }
+
                 Model?.OnRequestsCrashPrompt(crashData);
 
                 if (fatal)
diff --git a/src/DynamoCoreWpf/Views/SplashScreen/SplashScreen.xaml.cs b/src/DynamoCoreWpf/Views/SplashScreen/SplashScreen.xaml.cs
@@ -205,6 +205,12 @@ private void WebView_NavigationCompleted(object sender, CoreWebView2NavigationCo
             }
             catch (Exception ex)
             {
+                if (DynamoModel.IsTestMode)
+                {
+                    // Rethrow exception during testing.
+                    throw;
+                }
+
                 if (!DynamoModel.IsCrashing && !IsClosing)
                 {
                     CrashReportTool.ShowCrashWindow(viewModel, new CrashErrorReportArgs(ex));
diff --git a/src/DynamoPackages/Package.cs b/src/DynamoPackages/Package.cs
@@ -23,7 +23,7 @@ public class PackageAssembly
     {
         public bool IsNodeLibrary { get; set; }
         public Assembly Assembly { get; set; }
-        public string LocalFilePath {get;set;} 
+        public string LocalFilePath {get;set;}
 
         public string Name
         {
@@ -358,7 +358,6 @@ internal IEnumerable<PackageAssembly> EnumerateAndLoadAssembliesInBinDirectory()
             // Use the pkg header to determine which assemblies to load and prevent multiple enumeration
             // In earlier packages, this field could be null, which is correctly handled by IsNodeLibrary
             var nodeLibraries = Header.node_libraries;
-            
             foreach (var assemFile in new DirectoryInfo(BinaryDirectory).EnumerateFiles("*.dll"))
             {
                 Assembly assem;
diff --git a/src/NodeServices/MessageEvents.cs b/src/NodeServices/MessageEvents.cs
@@ -66,9 +66,16 @@ internal static class DynamoConsoleLogger
     {
         public static event Action<string> LogMessageToDynamoConsole;
 
+        public static event Action<string> LogErrorToDynamoConsole;
+
         public static void OnLogMessageToDynamoConsole(string message)
         {
             LogMessageToDynamoConsole?.Invoke(message);
         }
+
+        public static void OnLogErrorToDynamoConsole(string message)
+        {
+            LogErrorToDynamoConsole?.Invoke(message);
+        }
     }
 }
diff --git a/test/DynamoCoreWpfTests/PackageManager/PackageManagerViewExtensionTests.cs b/test/DynamoCoreWpfTests/PackageManager/PackageManagerViewExtensionTests.cs
@@ -12,6 +12,8 @@
 using Dynamo.PackageManager.UI;
 using Dynamo.Scheduler;
 using Dynamo.Wpf.Extensions;
+using DynamoCoreWpfTests.Utility;
+using DynamoServices;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Moq;
@@ -314,7 +316,49 @@ void PackageManagerViewExtensionTests_NotificationLogged(NotificationMessage obj
             }
         }
 
+        [Test]
+        public void TestCrashInPackage()
+        {
+            var pkgDir = Path.Combine(PackagesDirectory, "SampleViewExtension_Crash");
+            var currentDynamoModel = ViewModel.Model;
+            var loader = currentDynamoModel.GetPackageManagerExtension().PackageLoader;
+
+            var pkg = loader.ScanPackageDirectory(pkgDir);
+            Assert.IsNotNull(pkg);
+
+            loader.LoadPackages(new List<Package>() { pkg });
+     
+            Exception expectedEx = null;
+            var assem = AppDomain.CurrentDomain.GetAssemblies().Where(x => x.GetName().Name == "TestCrash").FirstOrDefault();
+            Assert.IsNotNull(assem);
+
+            try
+            {
+                assem.GetType("TestCrash.Class1").GetMethod("TestCrash").Invoke(null, new object[] { });
+            }
+            catch (Exception ex)
+            {
+                expectedEx = ex;
+            }
 
+            int count = 0;
+            void DynamoConsoleLogger_LogErrorToDynamoConsole(string obj)
+            {
+                if (obj.Contains($"Unhandled exception coming from package {pkg.Name} was handled"))
+                {
+                    count++;
+                }
+            }
+
+            DynamoConsoleLogger.LogErrorToDynamoConsole += DynamoConsoleLogger_LogErrorToDynamoConsole;
+
+            ViewModel.CrashGracefully(expectedEx);
+
+            DynamoConsoleLogger.LogErrorToDynamoConsole -= DynamoConsoleLogger_LogErrorToDynamoConsole;
+
+            ViewModel.CrashGracefully(expectedEx);
+            Assert.AreEqual(1, count);
+        }
 
         [OneTimeTearDown]
         /// <summary>
diff --git a/test/pkgs/SampleViewExtension_Crash/bin/SampleViewExtensionCrash.dll b/test/pkgs/SampleViewExtension_Crash/bin/SampleViewExtensionCrash.dll
diff --git a/test/pkgs/SampleViewExtension_Crash/bin/TestCrash.dll b/test/pkgs/SampleViewExtension_Crash/bin/TestCrash.dll
diff --git a/test/pkgs/SampleViewExtension_Crash/extra/SampleViewExtension_ViewExtensionDefinition.xml b/test/pkgs/SampleViewExtension_Crash/extra/SampleViewExtension_ViewExtensionDefinition.xml
@@ -0,0 +1,4 @@
+﻿<ViewExtensionDefinition>
+  <AssemblyPath>..\bin\SampleViewExtensionCrash.dll</AssemblyPath>
+  <TypeName>SampleViewExtension.SampleViewExtension</TypeName>
+</ViewExtensionDefinition>
diff --git a/test/pkgs/SampleViewExtension_Crash/pkg.json b/test/pkgs/SampleViewExtension_Crash/pkg.json
@@ -0,0 +1,17 @@
+{
+  "file_hash":null,
+  "name":"Sample View Extension",
+  "version":"2.0.1",
+  "description":"Dynamo sample view extension.",
+  "group":"",
+  "keywords":["dynamo","samples, view, extension, nodes"],
+  "dependencies":[],
+  "license":"MIT",
+  "contents":"",
+  "engine_version":"2.0.1",
+  "engine_metadata":"",
+  "engine":"dynamo",
+  "site_url": "https://dynamobim.org/",
+  "repository_url": "https://github.com/DynamoDS/DynamoSamples",
+  "node_libraries": [ ]
+}

Original file line number	Diff line number	Diff line change
`@@ -974,6 +974,7 @@ protected DynamoModel(IStartConfiguration config)`
`974`	`974`	`LogWarningMessageEvents.LogWarningMessage += LogWarningMessage;`
`975`	`975`	`LogWarningMessageEvents.LogInfoMessage += LogInfoMessage;`
`976`	`976`	`DynamoConsoleLogger.LogMessageToDynamoConsole += LogMessageWrapper;`
	`977`	`+ DynamoConsoleLogger.LogErrorToDynamoConsole += LogErrorMessageWrapper;`
`977`	`978`	`StartBackupFilesTimer();`
`978`	`979`
`979`	`980`	`TraceReconciliationProcessor = this;`
`@@ -1425,6 +1426,7 @@ public void Dispose()`
`1425`	`1426`	`LogWarningMessageEvents.LogWarningMessage -= LogWarningMessage;`
`1426`	`1427`	`LogWarningMessageEvents.LogInfoMessage -= LogInfoMessage;`
`1427`	`1428`	`DynamoConsoleLogger.LogMessageToDynamoConsole -= LogMessageWrapper;`
	`1429`	`+ DynamoConsoleLogger.LogErrorToDynamoConsole -= LogErrorMessageWrapper;`
`1428`	`1430`	`foreach (var ws in _workspaces)`
`1429`	`1431`	`{`
`1430`	`1432`	`ws.Dispose();`
`@@ -3296,11 +3298,17 @@ private void LogMessage(ILogMessage obj)`
`3296`	`3298`	`{`
`3297`	`3299`	`Logger.Log(obj);`
`3298`	`3300`	`}`
	`3301`	`+`
`3299`	`3302`	`private void LogMessageWrapper(string m)`
`3300`	`3303`	`{`
`3301`	`3304`	`LogMessage(Dynamo.Logging.LogMessage.Info(m));`
`3302`	`3305`	`}`
`3303`	`3306`
	`3307`	`+ private void LogErrorMessageWrapper(string m)`
	`3308`	`+ {`
	`3309`	`+ LogMessage(Dynamo.Logging.LogMessage.Error(m));`
	`3310`	`+ }`
	`3311`	`+`
`3304`	`3312`	`#if DEBUG_LIBRARY`
`3305`	`3313`	`private void DumpLibrarySnapshot(IEnumerable<Engine.FunctionGroup> functionGroups)`
`3306`	`3314`	`{`
Original file line number	Diff line number	Diff line change
`@@ -205,6 +205,12 @@ private void WebView_NavigationCompleted(object sender, CoreWebView2NavigationCo`
`205`	`205`	`}`
`206`	`206`	`catch (Exception ex)`
`207`	`207`	`{`
	`208`	`+ if (DynamoModel.IsTestMode)`
	`209`	`+ {`
	`210`	`+ // Rethrow exception during testing.`
	`211`	`+ throw;`
	`212`	`+ }`
	`213`	`+`
`208`	`214`	`if (!DynamoModel.IsCrashing && !IsClosing)`
`209`	`215`	`{`
`210`	`216`	`CrashReportTool.ShowCrashWindow(viewModel, new CrashErrorReportArgs(ex));`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public class PackageAssembly`
`23`	`23`	`{`
`24`	`24`	`public bool IsNodeLibrary { get; set; }`
`25`	`25`	`public Assembly Assembly { get; set; }`
`26`		`- public string LocalFilePath {get;set;}`
	`26`	`+ public string LocalFilePath {get;set;}`
`27`	`27`
`28`	`28`	`public string Name`
`29`	`29`	`{`
`@@ -358,7 +358,6 @@ internal IEnumerable<PackageAssembly> EnumerateAndLoadAssembliesInBinDirectory()`
`358`	`358`	`// Use the pkg header to determine which assemblies to load and prevent multiple enumeration`
`359`	`359`	`// In earlier packages, this field could be null, which is correctly handled by IsNodeLibrary`
`360`	`360`	`var nodeLibraries = Header.node_libraries;`
`361`		`-`
`362`	`361`	`foreach (var assemFile in new DirectoryInfo(BinaryDirectory).EnumerateFiles("*.dll"))`
`363`	`362`	`{`
`364`	`363`	`Assembly assem;`
Original file line number	Diff line number	Diff line change
`@@ -66,9 +66,16 @@ internal static class DynamoConsoleLogger`
`66`	`66`	`{`
`67`	`67`	`public static event Action<string> LogMessageToDynamoConsole;`
`68`	`68`
	`69`	`+ public static event Action<string> LogErrorToDynamoConsole;`
	`70`	`+`
`69`	`71`	`public static void OnLogMessageToDynamoConsole(string message)`
`70`	`72`	`{`
`71`	`73`	`LogMessageToDynamoConsole?.Invoke(message);`
`72`	`74`	`}`
	`75`	`+`
	`76`	`+ public static void OnLogErrorToDynamoConsole(string message)`
	`77`	`+ {`
	`78`	`+ LogErrorToDynamoConsole?.Invoke(message);`
	`79`	`+ }`
`73`	`80`	`}`
`74`	`81`	`}`