Better exception presentation

Trap and route as many exceptions as possible to the central handler.

For exceptions coming from the main thread it is possible to ignore those
and continue the app's execution, but for unhandled exceptions coming
from background threads the app will terminate.

The main focus of this work is to intercept exceptions originated from
`Executable` (i.e. executing git commands, etc.), and user scripts.
However it is not limited in this scope.

Resolves gitextensions#7795

GitCommands/Git/Executable.cs

@@ -98,9 +98,18 @@ public ProcessWrapper(string fileName, string arguments, string workDir, bool cr
 
                 _process.Exited += OnProcessExit;
 
-                _process.Start();
+                try
+                {
+                    _process.Start();
+                    _logOperation.SetProcessId(_process.Id);
+                }
+                catch (Exception ex)
+                {
+                    Dispose();
 
-                _logOperation.SetProcessId(_process.Id);
+                    _logOperation.LogProcessEnd(ex);
+                    throw new ExternalOperationException(fileName, arguments, workDir, ex);
+                }
             }
 
             private void OnProcessExit(object sender, EventArgs eventArgs)

GitCommands/Git/ExternalOperationException.cs

@@ -0,0 +1,44 @@
+using System;
+
+#nullable enable
+
+namespace GitCommands
+{
+    /// <summary>
+    /// Represents errors that occur during execution of an external operation,
+    /// e.g. running a git operation or launching an external process.
+    /// </summary>
+    public class ExternalOperationException : Exception
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ExternalOperationException"/> class with a specified parameters
+        /// and a reference to the inner exception that is the cause of this exception.
+        /// </summary>
+        /// <param name="command">The command that led to the exception.</param>
+        /// <param name="arguments">The command arguments.</param>
+        /// <param name="workingDirectory">The working directory.</param>
+        /// <param name="innerException">The exception that is the cause of the current exception.</param>
+        public ExternalOperationException(string command, string arguments, string workingDirectory, Exception innerException)
+            : base(innerException?.Message, innerException)
+        {
+            Command = command;
+            Arguments = arguments;
+            WorkingDirectory = workingDirectory;
+        }
+
+        /// <summary>
+        /// The command that led to the exception.
+        /// </summary>
+        public string Command { get; }
+
+        /// <summary>
+        /// The command arguments.
+        /// </summary>
+        public string Arguments { get; }
+
+        /// <summary>
+        /// The working directory.
+        /// </summary>
+        public string WorkingDirectory { get; }
+    }
+}

GitCommands/Git/UserExternalOperationException.cs

@@ -0,0 +1,25 @@
+using System;
+
+#nullable enable
+
+namespace GitCommands
+{
+    /// <summary>
+    /// Represents errors that occur during execution of user-configured operation, e.g. a script.
+    /// </summary>
+    public class UserExternalOperationException : ExternalOperationException
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="UserExternalOperationException"/> class with a specified parameters
+        /// and a reference to the inner exception that is the cause of this exception.
+        /// </summary>
+        /// <param name="command">The command that led to the exception.</param>
+        /// <param name="arguments">The command arguments.</param>
+        /// <param name="workingDirectory">The working directory.</param>
+        /// <param name="innerException">The exception that is the cause of the current exception.</param>
+        public UserExternalOperationException(string command, string arguments, string workingDirectory, Exception innerException)
+            : base(command, arguments, workingDirectory, innerException)
+        {
+        }
+    }
+}

GitExtensions/Program.cs

@@ -37,6 +37,11 @@ private static void Main()
             Application.EnableVisualStyles();
             Application.SetCompatibleTextRenderingDefault(false);
 
+            // If an error happens before we had a chance to init the environment information
+            // the call to GetInformation() from BugReporter.ShowNBug() will fail.
+            // There's no perf hit calling Initialise() multiple times.
+            UserEnvironmentInformation.Initialise(ThisAssembly.Git.Sha, ThisAssembly.Git.IsDirty);
+
             ThemeModule.Load();
             Application.ApplicationExit += (s, e) => ThemeModule.Unload();
 
@@ -48,8 +53,8 @@ private static void Main()
 
                 if (!Debugger.IsAttached)
                 {
-                    AppDomain.CurrentDomain.UnhandledException += (s, e) => ReportBug((Exception)e.ExceptionObject);
-                    Application.ThreadException += (s, e) => ReportBug(e.Exception);
+                    AppDomain.CurrentDomain.UnhandledException += (s, e) => BugReporter.Report((Exception)e.ExceptionObject, e.IsTerminating);
+                    Application.ThreadException += (s, e) => BugReporter.Report(e.Exception, isTerminating: false);
                 }
             }
             catch (TypeInitializationException tie)
@@ -356,22 +361,5 @@ private static bool LocateMissingGit()
                     }
             }
         }
-
-        private static void ReportBug(Exception ex)
-        {
-            // if the error happens before we had a chance to init the environment information
-            // the call to GetInformation() will fail. A double Initialise() call is safe.
-            UserEnvironmentInformation.Initialise(ThisAssembly.Git.Sha, ThisAssembly.Git.IsDirty);
-            var envInfo = UserEnvironmentInformation.GetInformation();
-
-            using (var form = new GitUI.NBugReports.BugReportForm())
-            {
-                var result = form.ShowDialog(ex, envInfo);
-                if (result == DialogResult.Abort)
-                {
-                    Environment.Exit(-1);
-                }
-            }
-        }
     }
 }

GitUI/MessageBoxes.cs

@@ -56,10 +56,6 @@ internal MessageBoxes()
 
         private static MessageBoxes Instance => instance ?? (instance = new MessageBoxes());
 
-        public static void FailedToExecuteScript(IWin32Window owner, string scriptKey, Exception ex)
-            => ShowError(owner, $"{Instance._failedToExecuteScript.Text} {scriptKey.Quote()}.{Environment.NewLine}"
-                                + $"{Instance._reason.Text}: {ex.Message}");
-
         public static void FailedToRunShell(IWin32Window owner, string shell, Exception ex)
             => ShowError(owner, $"{Instance._failedToRunShell.Text} {shell.Quote()}.{Environment.NewLine}"
                                 + $"{Instance._reason.Text}: {ex.Message}");

GitUI/NBugReports/BugReportForm.cs

@@ -71,7 +71,7 @@ public BugReportForm()
             mainTabs.TabPages.Remove(mainTabs.TabPages["reportContentsTabPage"]);
         }
 
-        public DialogResult ShowDialog(Exception exception, string environmentInfo)
+        public DialogResult ShowDialog(IWin32Window owner, Exception exception, string environmentInfo)
         {
             _lastException = new SerializableException(exception);
             _lastReport = new Report(_lastException);
@@ -95,7 +95,7 @@ public DialogResult ShowDialog(Exception exception, string environmentInfo)
             DialogResult = DialogResult.None;
 
             // ToDo: Fill in the 'Report Contents' tab);
-            var result = ShowDialog();
+            var result = ShowDialog(owner);
 
             // Write back the user description (as we passed 'report' as a reference since it is a reference object anyway)
             _lastReport.GeneralInfo.UserDescription = descriptionTextBox.Text;
@@ -172,4 +172,4 @@ public TestAccessor(BugReportForm form)
             public bool CheckContainsInfo(string input) => _form.CheckContainsInfo(input);
         }
     }
-}
+}

GitUI/NBugReports/BugReporter.cs

@@ -0,0 +1,152 @@
+#nullable enable
+
+using System;
+using System.Text;
+using System.Windows.Forms;
+using GitCommands;
+using GitUI;
+using Microsoft.WindowsAPICodePack.Dialogs;
+
+namespace GitExtensions
+{
+    public static class BugReporter
+    {
+        private static Form? OwnerForm =>
+            Form.ActiveForm ?? (Application.OpenForms.Count > 0 ? Application.OpenForms[0] : null);
+
+        private static IntPtr OwnerFormHandle
+            => OwnerForm?.Handle ?? IntPtr.Zero;
+
+        private static string FormatText(ExternalOperationException ex, bool canRaiseBug)
+        {
+            StringBuilder sb = new StringBuilder();
+
+            // Command: <command>
+            sb.AppendLine($"{Strings.Command}: {ex.Command}");
+
+            // Arguments: <args>
+            if (!string.IsNullOrWhiteSpace(ex.Arguments))
+            {
+                sb.AppendLine($"{Strings.Arguments}: {ex.Arguments}");
+            }
+
+            // Working directory: <working dir>
+            sb.AppendLine($"{Strings.WorkingDirectory}: {ex.WorkingDirectory}");
+
+            if (canRaiseBug)
+            {
+                // Directions to raise a bug
+                sb.AppendLine();
+                sb.AppendLine(Strings.ReportBug);
+            }
+
+            return sb.ToString();
+        }
+
+        public static void Report(Exception ex, bool isTerminating)
+        {
+            if (ex is UserExternalOperationException userExternalException)
+            {
+                // Something happened that was likely caused by the user
+                ReportUserException(userExternalException, isTerminating);
+                return;
+            }
+
+            if (ex is ExternalOperationException externalException)
+            {
+                // Something happened either in the app - can submit the bug to GitHub
+                ReportAppException(externalException, isTerminating);
+                return;
+            }
+
+            // Report any other exceptions
+            ReportGenericException(ex, isTerminating);
+        }
+
+        private static void ReportAppException(ExternalOperationException ex, bool isTerminating)
+        {
+            // UserExternalOperationException wraps an actual exception, but be cautious just in case
+            string instructionText = ex.InnerException?.Message ?? Strings.InstructionOperationFailed;
+
+            ReportException(FormatText(ex, canRaiseBug: true), instructionText, ex, isTerminating);
+        }
+
+        private static void ReportException(string text, string instructionText, Exception ex, bool isTerminating)
+        {
+            using var dialog = new TaskDialog
+            {
+                OwnerWindowHandle = OwnerFormHandle,
+                Text = text,
+                InstructionText = instructionText,
+                Caption = Strings.Error,
+                Icon = TaskDialogStandardIcon.Error,
+                Cancelable = true,
+            };
+            var btnReport = new TaskDialogCommandLink("Report", Strings.ButtonReportBug);
+            btnReport.Click += (s, e) =>
+            {
+                dialog.Close();
+                ShowNBug(OwnerForm, ex, isTerminating);
+            };
+            var btnIgnoreOrClose = new TaskDialogCommandLink("IgnoreOrClose", isTerminating ? Strings.ButtonCloseApp : Strings.ButtonIgnore);
+            btnIgnoreOrClose.Click += (s, e) =>
+            {
+                dialog.Close();
+            };
+            dialog.Controls.Add(btnReport);
+            dialog.Controls.Add(btnIgnoreOrClose);
+
+            dialog.Show();
+        }
+
+        private static void ReportGenericException(Exception ex, bool isTerminating)
+        {
+            // This exception is arbitrary, see if there's additional information
+            string? moreInfo = ex.InnerException?.Message;
+            if (moreInfo != null)
+            {
+                moreInfo += Environment.NewLine + Environment.NewLine;
+            }
+
+            ReportException($"{moreInfo}{Strings.ReportBug}", ex.Message, ex, isTerminating);
+        }
+
+        private static void ReportUserException(UserExternalOperationException ex, bool isTerminating)
+        {
+            // UserExternalOperationException wraps an actual exception, but be cautious just in case
+            string instructionText = ex.InnerException?.Message ?? Strings.InstructionOperationFailed;
+
+            using var dialog = new TaskDialog
+            {
+                OwnerWindowHandle = OwnerFormHandle,
+                Text = FormatText(ex, canRaiseBug: false),
+                InstructionText = instructionText,
+                Caption = Strings.CaptionFailedExecute,
+                Icon = TaskDialogStandardIcon.Error,
+                Cancelable = true,
+            };
+            var btnIgnoreOrClose = new TaskDialogCommandLink("IgnoreOrClose", isTerminating ? Strings.ButtonCloseApp : Strings.ButtonIgnore);
+            btnIgnoreOrClose.Click += (s, e) =>
+            {
+                dialog.Close();
+            };
+            dialog.Controls.Add(btnIgnoreOrClose);
+
+            dialog.Show();
+        }
+
+        private static void ShowNBug(IWin32Window? owner, Exception ex, bool isTerminating)
+        {
+            var envInfo = UserEnvironmentInformation.GetInformation();
+
+            using (var form = new GitUI.NBugReports.BugReportForm())
+            {
+                var result = form.ShowDialog(owner, ex, envInfo);
+                if (isTerminating || result == DialogResult.Abort)
+                {
+                    Environment.Exit(-1);
+                }
+            }
+        }
+    }
+}

GitUI/Script/ScriptRunner.cs

@@ -37,10 +37,9 @@ public static CommandStatus RunScript(IWin32Window owner, IGitModule module, str
                 return RunScript(owner, module, scriptKey, uiCommands, revisionGrid,
                     msg => MessageBox.Show(owner, msg, Strings.Error, MessageBoxButtons.OK, MessageBoxIcon.Error));
             }
-            catch (Exception ex)
+            catch (ExternalOperationException ex)
             {
-                MessageBoxes.FailedToExecuteScript(owner, scriptKey, ex);
-                return new CommandStatus(false, false);
+                throw new UserExternalOperationException(ex.Command, ex.Arguments, ex.WorkingDirectory, ex.InnerException ?? ex);
             }
         }
 

GitUI/Strings.cs

@@ -15,7 +15,13 @@ internal sealed class Strings : Translate
 
         private readonly TranslationString _buttonCheckoutBranch = new TranslationString("Checkout branch");
         private readonly TranslationString _buttonContinue = new TranslationString("Continue");
+        private readonly TranslationString _buttonCloseApp = new TranslationString("Close application");
         private readonly TranslationString _buttonCreateBranch = new TranslationString("Create branch");
+        private readonly TranslationString _buttonIgnore = new TranslationString("Ignore");
+        private readonly TranslationString _buttonReportBug = new TranslationString("Report bug!");
+
+        private readonly TranslationString _captionFailedExecute = new TranslationString("Failed to execute");
+        private readonly TranslationString _instructionOperationFailed = new TranslationString("Operation failed");
 
         private readonly TranslationString _containedInCurrentCommitText = new TranslationString("'{0}' is contained in the currently selected commit");
         private readonly TranslationString _containedInBranchesText = new TranslationString("Contained in branches:");
@@ -93,6 +99,11 @@ internal sealed class Strings : Translate
 
         private readonly TranslationString _rotInactive = new TranslationString("[ Inactive ]");
 
+        private readonly TranslationString _argumentsText = new TranslationString("Arguments");
+        private readonly TranslationString _commandText = new TranslationString("Command");
+        private readonly TranslationString _workingDirectoryText = new TranslationString("Working directory");
+        private readonly TranslationString _reportBugText = new TranslationString("If you think this was caused by Git Extensions, you can report a bug for the team to investigate.");
+
         // public only because of FormTranslate
         public Strings()
         {
@@ -118,7 +129,13 @@ public static void Reinitialize()
 
         public static string ButtonContinue => _instance.Value._buttonContinue.Text;
         public static string ButtonCheckoutBranch => _instance.Value._buttonCheckoutBranch.Text;
+        public static string ButtonCloseApp => _instance.Value._buttonCloseApp.Text;
         public static string ButtonCreateBranch => _instance.Value._buttonCreateBranch.Text;
+        public static string ButtonIgnore => _instance.Value._buttonIgnore.Text;
+        public static string ButtonReportBug => _instance.Value._buttonReportBug.Text;
+
+        public static string CaptionFailedExecute => _instance.Value._captionFailedExecute.Text;
+        public static string InstructionOperationFailed => _instance.Value._instructionOperationFailed.Text;
 
         public static string ContainedInCurrentCommit => _instance.Value._containedInCurrentCommitText.Text;
         public static string ContainedInBranches => _instance.Value._containedInBranchesText.Text;
@@ -192,5 +209,9 @@ public static void Reinitialize()
         public static string RevertSelectedLines => _instance.Value._revertSelectedLines.Text;
         public static string ResetSelectedLinesConfirmation => _instance.Value._resetSelectedLinesConfirmation.Text;
         public static string ResetChangesCaption => _instance.Value._resetChangesCaption.Text;
+        public static string Arguments => _instance.Value._argumentsText.Text;
+        public static string Command => _instance.Value._commandText.Text;
+        public static string WorkingDirectory => _instance.Value._workingDirectoryText.Text;
+        public static string ReportBug => _instance.Value._reportBugText.Text;
     }
 }