fix: Better error logging when processes fail

src/GitHub.Api/Helpers/Constants.cs

@@ -4,13 +4,20 @@ namespace GitHub.Unity
 {
     static class Constants
     {
+        // settings keys
         public const string GuidKey = "Guid";
         public const string MetricsKey = "MetricsEnabled";
         public const string UsageFile = "metrics.json";
         public const string GitInstallPathKey = "GitInstallPath";
         public const string TraceLoggingKey = "EnableTraceLogging";
         public const string WebTimeoutKey = "WebTimeout";
         public const string GitTimeoutKey = "GitTimeout";
+        public const string SkipVersionKey = "SkipVersion";
+        public const string GitInstallationState = "GitInstallationState";
+        public const string DisableGCMKey = "DisableGCM"; // don't let GCM prompt for credentials
+        public const string EnableGitAuthPromptsKey = "EnableGitAuthPrompts"; // let git prompt for credentials on the command line (might hang processes)
+        public const string DisableSetEnvironmentKey = "DisableSetEnvironment"; // don't set environment variables when spawning processes
+
         public const string Iso8601Format = @"yyyy-MM-dd\THH\:mm\:ss.fffzzz";
         public const string Iso8601FormatZ = @"yyyy-MM-dd\THH\:mm\:ss\Z";
         public static readonly string[] Iso8601Formats = {
@@ -32,8 +39,6 @@ static class Constants
             @"yyyy-MM-dd\THH\:mm\:ss.f\Z",
         };
         public const DateTimeStyles DateTimeStyle = DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal;
-        public const string SkipVersionKey = "SkipVersion";
-        public const string GitInstallationState = "GitInstallationState";
 
         public static readonly TheVersion MinimumGitVersion = TheVersion.Parse("2.0");
         public static readonly TheVersion MinimumGitLfsVersion = TheVersion.Parse("2.0");

src/GitHub.Api/Platform/ProcessEnvironment.cs

@@ -19,6 +19,11 @@ public ProcessEnvironment(IEnvironment environment)
         public void Configure(ProcessStartInfo psi, NPath workingDirectory, bool dontSetupGit = false)
         {
             psi.WorkingDirectory = workingDirectory;
+            if (Environment.UserSettings.Get<bool>(Constants.DisableSetEnvironmentKey))
+            {
+                // don't do anything beyond setting the working directory;
+                return;
+            }
             psi.EnvironmentVariables["HOME"] = NPath.HomeDirectory;
             psi.EnvironmentVariables["TMP"] = psi.EnvironmentVariables["TEMP"] = NPath.SystemTemp;
 
@@ -109,6 +114,11 @@ public void Configure(ProcessStartInfo psi, NPath workingDirectory, bool dontSet
             if (!String.IsNullOrEmpty(httpsProxy))
                 psi.EnvironmentVariables["HTTPS_PROXY"] = httpsProxy;
             psi.EnvironmentVariables["DISPLAY"] = "0";
+            if (!Environment.UserSettings.Get<bool>(Constants.EnableGitAuthPromptsKey))
+                psi.EnvironmentVariables["GIT_TERMINAL_PROMPT"] = "0";
+
+            if (Environment.UserSettings.Get<bool>(Constants.DisableGCMKey))
+                psi.EnvironmentVariables["GCM_INTERACTIVE"] = "NEVER";
         }
     }
 }

src/GitHub.Api/Tasks/ProcessTask.cs

@@ -4,6 +4,7 @@
 using System.ComponentModel;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Text;
 using System.Threading;
 
@@ -61,6 +62,15 @@ interface IProcessTask<TData, T> : ITask<TData, T>, IProcess
 
     class ProcessWrapper
     {
+        private readonly HashSet<string> traceEnvKeys = new HashSet<string> {
+            "TMP", "TEMP",
+            "APPDATA", "LOCALAPPDATA", "PROGRAMDATA",
+            "TERM", "PLINK_PROTOCOL", "DISPLAY",
+            "GITLFSLOCKSENABLED", "GITHUB_UNITY_DISABLE",
+            "GIT_EXEC_PATH", "PATH", "GHU_FULLPATH", "GHU_WORKINGDIR",
+            "HTTP_PROXY", "HTTPS_PROXY"
+        };
+
         private readonly string taskName;
         private readonly IOutputProcessor outputProcessor;
         private readonly Action onStart;
@@ -108,7 +118,6 @@ public void Run()
                     {
                         var line = Encoding.UTF8.GetString(Encoding.UTF8.GetBytes(e.Data));
                         errors.Add(line.TrimEnd('\r', '\n'));
-                        Logger.Trace(line);
                     }
                 };
             }
@@ -156,8 +165,10 @@ public void Run()
                             // process is done and we haven't seen output, we're done
                             done = !gotOutput.WaitOne(100);
                         }
-                        else if (token.IsCancellationRequested || (taskName.Contains("git lfs") && lastOutput.AddMilliseconds(ApplicationConfiguration.DefaultGitTimeout) < DateTimeOffset.UtcNow))
-                        // if we're exiting or we haven't had output for a while
+                        else if (token.IsCancellationRequested || (taskName.Contains("git lfs") &&
+                                lastOutput.AddMilliseconds(ApplicationConfiguration.DefaultGitTimeout) <
+                                DateTimeOffset.UtcNow))
+                            // if we're exiting or we haven't had output for a while
                         {
                             Stop(true);
                             token.ThrowIfCancellationRequested();
@@ -167,17 +178,21 @@ public void Run()
 
                     if (Process.ExitCode != 0 && errors.Count > 0)
                     {
-                        thrownException = new ProcessException(Process.ExitCode, string.Join(Environment.NewLine, errors.ToArray()));
+                        throw new ProcessException(Process.ExitCode, string.Join(Environment.NewLine, errors.ToArray()));
                     }
                 }
             }
             catch (Exception ex)
             {
                 var errorCode = -42;
-                if (ex is Win32Exception)
-                    errorCode = ((Win32Exception)ex).NativeErrorCode;
-
-                StringBuilder sb = new StringBuilder();
+                var win32Exception = ex as Win32Exception;
+                if (win32Exception != null)
+                    errorCode = win32Exception.NativeErrorCode;
+                var processException = ex as ProcessException;
+                if (processException != null)
+                    errorCode = processException.ErrorCode;
+
+                var sb = new StringBuilder();
                 sb.AppendLine($"Error code {errorCode}");
                 sb.AppendLine(ex.Message);
                 if (Process.StartInfo.Arguments.Contains("-credential"))
@@ -186,12 +201,12 @@ public void Run()
                     sb.AppendLine($"'{Process.StartInfo.FileName} {Process.StartInfo.Arguments}'");
                 if (errorCode == 2)
                     sb.AppendLine("The system cannot find the file specified.");
-                foreach (string env in Process.StartInfo.EnvironmentVariables.Keys)
-                {
-                    sb.AppendFormat("{0}:{1}", env, Process.StartInfo.EnvironmentVariables[env]);
-                    sb.AppendLine();
-                }
-                thrownException = new ProcessException(errorCode, sb.ToString(), ex);
+
+                thrownException = new ProcessException(errorCode, sb.ToString(), ex) {
+                    EnvironmentVariables = Process.StartInfo.EnvironmentVariables.Keys.Cast<string>()
+                                                  .Where(x => traceEnvKeys.Contains(x)).Select(x =>
+                                                      $"{x}={Process.StartInfo.EnvironmentVariables[x]}").ToArray()
+                };
             }
 
             if (thrownException != null || errors.Count > 0)
@@ -348,25 +363,14 @@ protected override T RunWithReturn(bool success)
                 () => OnStartProcess?.Invoke(this),
                 () =>
                 {
-                    try
-                    {
-                        if (outputProcessor != null)
-                            result = outputProcessor.Result;
-
-                        if (typeof(T) == typeof(string) && result == null && !Process.StartInfo.CreateNoWindow)
-                            result = (T)(object)"Process running";
+                    if (outputProcessor != null)
+                        result = outputProcessor.Result;
 
-                        if (!String.IsNullOrEmpty(Errors))
-                            OnErrorData?.Invoke(Errors);
-                    }
-                    catch (Exception ex)
-                    {
-                        if (thrownException == null)
-                            thrownException = new ProcessException(ex.Message, ex);
-                        else
-                            thrownException = new ProcessException(thrownException.GetExceptionMessage(), ex);
-                    }
+                    if (typeof(T) == typeof(string) && result == null && !Process.StartInfo.CreateNoWindow)
+                        result = (T)(object)"Process running";
 
+                    if (!String.IsNullOrEmpty(Errors))
+                        OnErrorData?.Invoke(Errors);
                     if (thrownException != null && !RaiseFaultHandlers(thrownException))
                         throw thrownException;
                 },
@@ -388,9 +392,9 @@ public override string ToString()
         }
 
         public Process Process { get; set; }
-        public int ProcessId { get { return Process.Id; } }
-        public override bool Successful { get { return base.Successful && Process.ExitCode == 0; } }
-        public StreamWriter StandardInput { get { return wrapper?.Input; } }
+        public int ProcessId => Process.Id;
+        public override bool Successful => base.Successful && Process.ExitCode == 0;
+        public StreamWriter StandardInput => wrapper?.Input;
         public virtual string ProcessName { get; protected set; }
         public virtual string ProcessArguments { get; }
     }

src/GitHub.Api/Tasks/TaskCanceledExceptions.cs

@@ -19,30 +19,4 @@ protected DependentTaskFailedException(SerializationInfo info, StreamingContext
         public DependentTaskFailedException(ITask task, Exception ex) : this(ex.InnerException != null ? ex.InnerException.Message : ex.Message, ex.InnerException ?? ex)
         {}
     }
-
-    [Serializable]
-    class ProcessException : TaskCanceledException
-    {
-        public int ErrorCode { get; }
-
-        protected ProcessException() : base()
-        { }
-        public ProcessException(int errorCode, string message) : base(message)
-        {
-            ErrorCode = errorCode;
-        }
-        public ProcessException(int errorCode, string message, Exception innerException) : base(message, innerException)
-        {
-            ErrorCode = errorCode;
-        }
-        public ProcessException(string message) : base(message)
-        { }
-        public ProcessException(string message, Exception innerException) : base(message, innerException)
-        { }
-        protected ProcessException(SerializationInfo info, StreamingContext context) : base(info, context)
-        { }
-
-        public ProcessException(ITask process) : this(process.Errors)
-        { }
-    }
-}
+}

src/GitHub.Logging/Extensions/ExceptionExtensions.cs

@@ -1,17 +1,30 @@
 using System;
 using System.Linq;
+using GitHub.Unity;
 
 namespace GitHub.Logging
 {
     public static class ExceptionExtensions
     {
-        public static string GetExceptionMessage(this Exception ex)
+        public static string GetExceptionMessage(this Exception ex, bool includeEnvironment = false)
         {
             var message = ex.ToString();
+
+            var processException = ex as ProcessException;
+            if (includeEnvironment && processException != null)
+            {
+                message += Environment.NewLine + String.Join(Environment.NewLine, ((ProcessException)ex).EnvironmentVariables);
+            }
+
             var inner = ex.InnerException;
             while (inner != null)
             {
                 message += Environment.NewLine + inner.ToString();
+                processException = inner as ProcessException;
+                if (includeEnvironment && processException != null)
+                {
+                    message += Environment.NewLine + String.Join(Environment.NewLine, processException.EnvironmentVariables);
+                }
                 inner = inner.InnerException;
             }
             var caller = Environment.StackTrace;
@@ -33,4 +46,4 @@ public static string GetExceptionMessageShort(this Exception ex)
             return message;
         }
     }
-}
+}

src/GitHub.Logging/Extensions/ProcessExceptions.cs

@@ -0,0 +1,25 @@
+using System;
+using System.Runtime.Serialization;
+
+namespace GitHub.Unity
+{
+    [Serializable]
+    class ProcessException : OperationCanceledException
+    {
+        public int ErrorCode { get; }
+        public string[] EnvironmentVariables { get; set; }
+
+        public ProcessException(int errorCode, string message) : base(message)
+        {
+            ErrorCode = errorCode;
+        }
+        public ProcessException(int errorCode, string message, Exception innerException) : base(message, innerException)
+        {
+            ErrorCode = errorCode;
+        }
+        public ProcessException(string message, Exception innerException) : base(message, innerException)
+        { }
+        protected ProcessException(SerializationInfo info, StreamingContext context) : base(info, context)
+        { }
+    }
+}

src/GitHub.Logging/GitHub.Logging.csproj

@@ -56,6 +56,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="Extensions\ExceptionExtensions.cs" />
+    <Compile Include="Extensions\ProcessExceptions.cs" />
     <Compile Include="FileLogAdapter.cs" />
     <Compile Include="GlobalSuppressions.cs" />
     <Compile Include="ILogging.cs" />

src/GitHub.Logging/LogFacade.cs

@@ -59,7 +59,7 @@ public void Debug(string format, params object[] objects)
         public void Debug(Exception ex, string message)
         {
 #if DEBUG
-            Debug(String.Concat(message, Environment.NewLine, ex.GetExceptionMessage()));
+            Debug(String.Concat(message, Environment.NewLine, ex.GetExceptionMessage(true)));
 #endif
         }
 
@@ -87,8 +87,7 @@ public void Trace(string format, params object[] objects)
         public void Trace(Exception ex, string message)
         {
             if (!LogHelper.TracingEnabled) return;
-
-            Trace(String.Concat(message, Environment.NewLine, ex.GetExceptionMessage()));
+            Trace(String.IsNullOrEmpty(message) ? ex.GetExceptionMessage(true) : String.Concat(message, Environment.NewLine, ex.GetExceptionMessage(true)));
         }
 
         public void Trace(Exception ex)
@@ -155,4 +154,4 @@ public void Error(Exception ex, string format, params object[] objects)
             Error(ex, String.Format(format, objects));
         }
     }
-}
+}

src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Styles.cs

@@ -220,6 +220,36 @@ public static GUIStyle Label
             }
         }
 
+        private static GUIStyle toggle;
+        public static GUIStyle Toggle
+        {
+            get
+            {
+                if (toggle == null)
+                {
+                    toggle = new GUIStyle(GUI.skin.toggle);
+                    toggle.name = "CustomToggle";
+                    toggle.wordWrap = true;
+                }
+                return toggle;
+            }
+        }
+
+        private static GUIStyle toggleNoWrap;
+        public static GUIStyle ToggleNoWrap
+        {
+            get
+            {
+                if (toggleNoWrap == null)
+                {
+                    toggleNoWrap = new GUIStyle(GUI.skin.toggle);
+                    toggleNoWrap.name = "CustomToggleNoWrap";
+                    toggleNoWrap.wordWrap = false;
+                }
+                return toggleNoWrap;
+            }
+        }
+
         public static GUIStyle LabelNoWrap
         {
             get
@@ -1136,5 +1166,19 @@ public static GUIStyle LockMetaDataStyle
                 return lockMetaDataStyle;
             }
         }
+
+        private static GUIStyle horizontalLine;
+        public static GUIStyle HorizontalLine {
+            get {
+                if (horizontalLine == null)
+                {
+                    horizontalLine = new GUIStyle(GUI.skin.box);
+                    horizontalLine.border.top = horizontalLine.border.bottom = 1;
+                    horizontalLine.margin.top = horizontalLine.margin.bottom = 0;
+                    horizontalLine.padding.top = horizontalLine.padding.bottom = 0;
+                }
+                return horizontalLine;
+            }
+        }
     }
 }