Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash handlers for Dynamo #14826

Merged
merged 24 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,8 @@ public static DynamoModel Start(IStartConfiguration configuration)
/// <param name="config">Start configuration</param>
protected DynamoModel(IStartConfiguration config)
{
DynamoModel.IsCrashing = false;

if (config is DefaultStartConfiguration defaultStartConfig)
{
// This is not exposed in IStartConfiguration to avoid a breaking change.
Expand Down
4 changes: 2 additions & 2 deletions src/DynamoCoreWpf/UI/Prompts/CrashPrompt.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ public CrashPrompt(CrashPromptArgs args, DynamoViewModel dynamoViewModel)
{
InitializeComponent();

var packageLoader = dynamoViewModel.Model.GetPackageManagerExtension()?.PackageLoader;
var packageLoader = dynamoViewModel?.Model?.GetPackageManagerExtension()?.PackageLoader;
markdownPackages = Wpf.Utilities.CrashUtilities.PackagesToMakrdown(packageLoader);

productName = dynamoViewModel.BrandingResourceProvider.ProductName;
productName = dynamoViewModel?.BrandingResourceProvider.ProductName ?? Process.GetCurrentProcess().ProcessName;
Title = string.Format(Wpf.Properties.Resources.CrashPromptDialogTitle, productName);
TitleTextBlock.Text = string.Format(Wpf.Properties.Resources.CrashPromptDialogTitle, productName);
txtOverridingText.Text = string.Format(Wpf.Properties.Resources.CrashPromptDialogCrashMessage, productName);
Expand Down
1 change: 1 addition & 0 deletions src/DynamoCoreWpf/Utilities/CerDLL.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public void Dispose()
DLL.FreeLibrary(m_dll);
m_dll = IntPtr.Zero;
}
Initialized = false;
}

private static bool Initialized;
Expand Down
15 changes: 15 additions & 0 deletions src/DynamoCoreWpf/Utilities/CrashReportTool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,21 @@ private static string FindCERToolInInstallLocations()
}
}

internal static void ShowCrashWindow(object sender, CrashPromptArgs args)
{
var viewModel = sender as DynamoViewModel;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a null check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually can be null. ShowCrashWindow could be called like ShowCrashWindow(null, new CrashPromptArgs());


if (CrashReportTool.ShowCrashErrorReportWindow(viewModel,
(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);
prompt.ShowDialog();
}

/// <summary>
/// Calls external CER tool (with UI)
/// </summary>
Expand Down
84 changes: 75 additions & 9 deletions src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Forms;
Expand Down Expand Up @@ -690,14 +691,20 @@ public struct StartConfiguration

protected DynamoViewModel(StartConfiguration startConfiguration)
{
// CurrentDomain_UnhandledException - catches unhandled exceptions that are fatal to the current process. These exceptions cannot be handled and process termination is guaranteed
AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException;
Copy link
Contributor

@aparajit-pratap aparajit-pratap Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was initially added to DynamoModel. Can you explain why it has been added here instead of DynamoModel and the same for TaskScheduler.UnobservedException? Since DynamoModel is instantiated before the view model, there could be code that's uncaught by any of these handlers.

Copy link
Contributor Author

@pinzart90 pinzart90 Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would require more refactor. CrashTool is in the DynamoCoreWPF project. It (along with other CrashPromptArgs) uses DynamoViewModel ..which I cannot access from DynamoCore and also may not exist yet depending on when the crash happens. Also if DynamoViewModel is created...then we would want all exception handlers to start using the DynamoViewModel. and stop the DynamoModel handler ?
I would leave that for another PR...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Maybe we should decouple the CER crash prompt from DynamoViewModel. Currently, it needs the view model to show the dialog. Would be good to remove the dependency on the view model for this dialog if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying that now, but there are too many changes for this PR
I will create a followup

// Dispatcher.CurrentDispatcher.UnhandledException - catches unhandled exceptions from the UI thread. Can mark exceptions as handled (and close Dynamo) so that host apps can continue running normally even though Dynamo crashed
Dispatcher.CurrentDispatcher.UnhandledException += CurrentDispatcher_UnhandledException;
// TaskScheduler.UnobservedTaskException - catches unobserved Task exceptions from all threads. Does not crash Dynamo, we only log the exceptions and do not call CER or close Dynamo
TaskScheduler.UnobservedTaskException += TaskScheduler_UnobservedTaskException;

this.ShowLogin = startConfiguration.ShowLogin;

// initialize core data structures
this.model = startConfiguration.DynamoModel;
this.model.CommandStarting += OnModelCommandStarting;
this.model.CommandCompleted += OnModelCommandCompleted;
this.model.RequestsCrashPrompt += CrashReportTool.ShowCrashWindow;

this.HideReportOptions = startConfiguration.HideReportOptions;
UsageReportingManager.Instance.InitializeCore(this);
Expand Down Expand Up @@ -772,28 +779,84 @@ protected DynamoViewModel(StartConfiguration startConfiguration)
MLDataPipelineExtension = model.ExtensionManager.Extensions.OfType<DynamoMLDataPipelineExtension>().FirstOrDefault();
}

private void TaskScheduler_UnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e)
{
try
{
var crashData = new CrashErrorReportArgs(e.Exception);
Model?.Logger?.LogError($"Unobserved task exception: {crashData.Details}");
Analytics.TrackException(e.Exception, true);
}
catch
{ }
}

private void CurrentDispatcher_UnhandledException(object sender, DispatcherUnhandledExceptionEventArgs e)
{
if (e.Handled)
if (e.Handled || DynamoModel.IsCrashing)
{
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;

CrashGracefully(e.Exception);
}

private void CrashGracefully(Exception ex)
private void CurrentDomain_UnhandledException(object sender, UnhandledExceptionEventArgs e)
{
try
if (!DynamoModel.IsCrashing)//Avoid duplicate CER reports
{
Model?.Logger?.LogError($"Unhandled exception {ex.Message}");
CrashGracefully(e.ExceptionObject as Exception, fatal: true);
}
}

private void CrashGracefully(Exception ex, bool fatal = false)
{
try
{
DynamoModel.IsCrashing = true;
var crashData = new CrashErrorReportArgs(ex);
Model?.Logger?.LogError($"Unhandled exception: {crashData.Details} ");
Analytics.TrackException(ex, true);
Model?.OnRequestsCrashPrompt(new CrashErrorReportArgs(ex));
Model?.OnRequestsCrashPrompt(crashData);

if (fatal)
{
// Fatal exception. Close Dynamo but do not terminate the process.

Exit(false); // don't allow cancellation
// Run the Dynamo exit code in the UI thread since CrashGracefully could be called in other threads too.
TryDispatcherInvoke(() => {
try
{
Exit(false);
}
catch { }
});

// Do not terminate the process in the plugin, because other AppDomain.UnhandledException events will not get a chance to get called
// ex. A host app (like Revit) could have an AppDomain.UnhandledException too.
// If we terminate the process here, the host app will not get a chance to gracefully shut down.
// Environment.Exit(1);
}
else
{
// Non fatal exception.

// We run the Dynamo exit call asyncronously in the dispatcher to ensure that any continuation of code
// manages to run to completion before we start shutting down Dynamo.
TryDispatcherBeginInvoke(() => {
try
{
Exit(false);
}
catch
{ }
});
}
}
catch
{ }
Expand Down Expand Up @@ -3490,8 +3553,6 @@ public ShutdownParams(
///
public bool PerformShutdownSequence(ShutdownParams shutdownParams)
{
Dispatcher.CurrentDispatcher.UnhandledException -= CurrentDispatcher_UnhandledException;

if (shutdownSequenceInitiated)
{
// There was a prior call to shutdown. This could happen for example
Expand All @@ -3511,14 +3572,18 @@ public bool PerformShutdownSequence(ShutdownParams shutdownParams)
// that the shutdown may not be stopped.
shutdownSequenceInitiated = true;

AppDomain.CurrentDomain.UnhandledException -= CurrentDomain_UnhandledException;
Dispatcher.CurrentDispatcher.UnhandledException -= CurrentDispatcher_UnhandledException;
TaskScheduler.UnobservedTaskException -= TaskScheduler_UnobservedTaskException;
this.Model.RequestsCrashPrompt -= CrashReportTool.ShowCrashWindow;

// Request the View layer to close its window (see
// ShutdownParams.CloseDynamoView member for details).
if (shutdownParams.CloseDynamoView)
{
OnRequestClose(this, EventArgs.Empty);
}


BackgroundPreviewViewModel.Dispose();
foreach (var wsvm in workspaces)
{
Expand All @@ -3528,6 +3593,7 @@ public bool PerformShutdownSequence(ShutdownParams shutdownParams)

model.ShutDown(shutdownParams.ShutdownHost);
UsageReportingManager.DestroyInstance();

this.model.CommandStarting -= OnModelCommandStarting;
this.model.CommandCompleted -= OnModelCommandCompleted;
BackgroundPreviewViewModel.PropertyChanged -= Watch3DViewModelPropertyChanged;
Expand Down
15 changes: 0 additions & 15 deletions src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,6 @@ private void DynamoView_Loaded(object sender, EventArgs e)
dynamoViewModel.RequestSave3DImage += DynamoViewModelRequestSave3DImage;
dynamoViewModel.SidebarClosed += DynamoViewModelSidebarClosed;

dynamoViewModel.Model.RequestsCrashPrompt += Controller_RequestsCrashPrompt;
dynamoViewModel.Model.RequestTaskDialog += Controller_RequestTaskDialog;

DynamoSelection.Instance.Selection.CollectionChanged += Selection_CollectionChanged;
Expand Down Expand Up @@ -1606,19 +1605,6 @@ private void Selection_CollectionChanged(object sender, NotifyCollectionChangedE
dynamoViewModel.NodeFromSelectionCommand.RaiseCanExecuteChanged();
}

private void Controller_RequestsCrashPrompt(object sender, CrashPromptArgs args)
{
if (CrashReportTool.ShowCrashErrorReportWindow(dynamoViewModel,
(args is CrashErrorReportArgs cerArgs) ? cerArgs :
new CrashErrorReportArgs(args.Details)))
{
return;
}
// Backup crash reporting dialog (in case ADSK CER is not found)
var prompt = new CrashPrompt(args, dynamoViewModel);
prompt.ShowDialog();
}

private void Controller_RequestTaskDialog(object sender, TaskDialogEventArgs e)
{
var taskDialog = new UI.Prompts.GenericTaskDialog(e);
Expand Down Expand Up @@ -1982,7 +1968,6 @@ private void WindowClosed(object sender, EventArgs e)

if (dynamoViewModel.Model != null)
{
dynamoViewModel.Model.RequestsCrashPrompt -= Controller_RequestsCrashPrompt;
dynamoViewModel.Model.RequestTaskDialog -= Controller_RequestTaskDialog;
dynamoViewModel.Model.ClipBoard.CollectionChanged -= ClipBoard_CollectionChanged;
}
Expand Down
27 changes: 21 additions & 6 deletions src/DynamoCoreWpf/Views/SplashScreen/SplashScreen.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,30 @@ private void DynamoModel_LanguageDetected()
}

private void WebView_NavigationCompleted(object sender, CoreWebView2NavigationCompletedEventArgs e)
{
if (webView != null)
{
try
{
// Exceptions thrown in WebView_NavigationCompleted seem to be silenced somewhere in the webview2 callstack.
// If we catch an exceptions here, we log it and close the spash screen.
//
if (webView != null)
{
webView.NavigationCompleted -= WebView_NavigationCompleted;
webView.Focus();
System.Windows.Forms.SendKeys.SendWait("{TAB}");
}
OnRequestDynamicSplashScreen();
}
catch (Exception ex)
{
webView.NavigationCompleted -= WebView_NavigationCompleted;
webView.Focus();
System.Windows.Forms.SendKeys.SendWait("{TAB}");
if (!DynamoModel.IsCrashing && !IsClosing)
{
CrashReportTool.ShowCrashWindow(viewModel, new CrashErrorReportArgs(ex));
Close();// Close the SpashScreen
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added Try Catch in the WebView_NavigationCompleted body.
Looks like all exceptions thrown in WebView_NavigationCompleted are silenced by webview2.

SplashScreen.WebView_NavigationCompleted is particularly sensitive because Dynamo views are initialzied inside this method and if an exceptions is thrown, SplashScreen will seem like it hangs.

}
}
OnRequestDynamicSplashScreen();
}

/// <summary>
/// Request to close SplashScreen.
/// </summary>
Expand Down
1 change: 0 additions & 1 deletion src/DynamoSandbox/DynamoCoreSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Dynamo.Logging;
using Dynamo.Models;
using Dynamo.ViewModels;
using Dynamo.Wpf.UI;
using Dynamo.Wpf.Utilities;
using Dynamo.Wpf.ViewModels.Watch3D;

Expand Down
Loading