mark NFW from OOP critical (#36156)

* mark NFW from OOP critical

currently, whenever OOP throws an exception, we show infobar saying "restart VS". whenever that happens, we report NFW. now those NFW will be marked as critical and we will stop reporting NFW after that in VS.

basically making critical NFW behavior same as fatal watson in VS. and management will treat critical NFW same as fatal watson.

also, now we will show callstack like code fix exception when info bar is shown

* xml document fix

* fix build break.

* changed button to hyper-link to follow same pattern as other info bar (code fix)

show remote exception in callstack view.

* introduced enum for Watson Severity

eng/Versions.props

@@ -113,7 +113,7 @@
     <MicrosoftVisualStudioProgressionInterfacesVersion>15.8.27812-alpha</MicrosoftVisualStudioProgressionInterfacesVersion>
     <MicrosoftVisualStudioProjectSystemVersion>15.3.178-pre-g209fb07c2e</MicrosoftVisualStudioProjectSystemVersion>
     <MicrosoftVisualStudioProjectSystemManagedVersion>2.3.6152103</MicrosoftVisualStudioProjectSystemManagedVersion>
-    <MicrosoftVisualStudioRemoteControlVersion>14.0.249-master2E2DC10C</MicrosoftVisualStudioRemoteControlVersion>
+    <MicrosoftVisualStudioRemoteControlVersion>14.1.10</MicrosoftVisualStudioRemoteControlVersion>
     <MicrosoftVisualStudioSDKAnalyzersVersion>15.7.7</MicrosoftVisualStudioSDKAnalyzersVersion>
     <MicrosoftVisualStudioSetupConfigurationInteropVersion>1.15.103</MicrosoftVisualStudioSetupConfigurationInteropVersion>
     <MicrosoftVisualStudioShell150Version>16.0.28226-pre</MicrosoftVisualStudioShell150Version>
@@ -127,7 +127,7 @@
     <MicrosoftVisualStudioShellInterop121DesignTimeVersion>12.1.30328</MicrosoftVisualStudioShellInterop121DesignTimeVersion>
     <MicrosoftVisualStudioShellInterop157DesignTimeVersion>15.7.1</MicrosoftVisualStudioShellInterop157DesignTimeVersion>
     <MicrosoftVisualStudioShellInterop80Version>8.0.50728</MicrosoftVisualStudioShellInterop80Version>
-    <MicrosoftVisualStudioTelemetryVersion>15.8.27812-alpha</MicrosoftVisualStudioTelemetryVersion>
+    <MicrosoftVisualStudioTelemetryVersion>16.0.233</MicrosoftVisualStudioTelemetryVersion>
     <MicrosoftVisualStudioTemplateWizardInterfaceVersion>8.0.0.0-alpha</MicrosoftVisualStudioTemplateWizardInterfaceVersion>
     <MicrosoftVisualStudioTextDataVersion>16.0.467</MicrosoftVisualStudioTextDataVersion>
     <MicrosoftVisualStudioTextInternalVersion>16.0.467</MicrosoftVisualStudioTextInternalVersion>

src/EditorFeatures/TestUtilities/Remote/RemoteHostCrashInfoBar.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System;
 using Microsoft.CodeAnalysis;
 
 namespace Microsoft.VisualStudio.LanguageServices.Remote
@@ -9,12 +10,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Remote
     /// </summary>
     internal static class RemoteHostCrashInfoBar
     {
-        public static void ShowInfoBar()
-        {
-            // do nothing
-        }
-
-        public static void ShowInfoBar(Workspace workspace)
+        public static void ShowInfoBar(Workspace workspace, Exception ex = null)
         {
             // do nothing
         }

src/VisualStudio/Core/Def/Implementation/Remote/JsonRpcClient.cs

@@ -153,7 +153,7 @@ private bool ReportUnlessCanceled(Exception ex, CancellationToken cancellationTo
 
         private void ThrowSoftCrashException(Exception ex, CancellationToken token)
         {
-            RemoteHostCrashInfoBar.ShowInfoBar(Workspace);
+            RemoteHostCrashInfoBar.ShowInfoBar(Workspace, ex);
 
             // log disconnect information before throw
             LogDisconnectInfo(_debuggingLastDisconnectReason, _debuggingLastDisconnectCallstack);

src/VisualStudio/Core/Def/Implementation/Remote/RemoteHostClientServiceFactory.RemoteHostClientService.cs

@@ -282,7 +282,7 @@ private void OnStatusChanged(object sender, bool started)
 
                     // s_lastRemoteClientTask info should be saved in the dump
                     // report NFW when connection is closed unless it is proper shutdown
-                    WatsonReporter.Report(new Exception("Connection to remote host closed"));
+                    WatsonReporter.Report(new Exception("Connection to remote host closed"), WatsonSeverity.Critical);
 
                     RemoteHostCrashInfoBar.ShowInfoBar(_workspace);
                 }

src/VisualStudio/Core/Def/Implementation/Remote/RemoteHostCrashInfoBar.cs

@@ -20,7 +20,7 @@ internal static class RemoteHostCrashInfoBar
 
         private static bool s_infoBarReported = false;
 
-        public static void ShowInfoBar(Workspace workspace)
+        public static void ShowInfoBar(Workspace workspace, Exception exception = null)
         {
             // use info bar to show warning to users
             if (workspace == null || s_infoBarReported)
@@ -52,6 +52,14 @@ public static void ShowInfoBar(Workspace workspace)
                     }, closeAfterAction: true));
             }
 
+            if (exception != null)
+            {
+                var errorReportingService = workspace.Services.GetService<IErrorReportingService>();
+                infoBarUIs.Add(
+                    new InfoBarUI(WorkspacesResources.Show_Stack_Trace, InfoBarUI.UIKind.HyperLink, () =>
+                        errorReportingService.ShowDetailedErrorInfo(exception), closeAfterAction: true));
+            }
+
             workspace.Services.GetService<IErrorReportingService>().ShowGlobalErrorInfo(
                 ServicesVSResources.Unfortunately_a_process_used_by_Visual_Studio_has_encountered_an_unrecoverable_error_We_recommend_saving_your_work_and_then_closing_and_restarting_Visual_Studio,
                 infoBarUIs.ToArray());

src/VisualStudio/Core/Def/Implementation/Remote/ServiceHubRemoteHostClient.Connections.cs

@@ -122,7 +122,7 @@ private static class Connections
                     await Task.Delay(retry_delayInMS, cancellationToken).ConfigureAwait(false);
                 }
 
-                RemoteHostCrashInfoBar.ShowInfoBar(workspace);
+                RemoteHostCrashInfoBar.ShowInfoBar(workspace, lastException);
 
                 // raise soft crash exception rather than doing hard crash.
                 // we had enough feedback from users not to crash VS on servicehub failure

src/VisualStudio/Core/Def/Implementation/Remote/ServiceHubRemoteHostClient.cs

@@ -233,7 +233,7 @@ private async Task RpcInvokeAsync(string targetName, params object[] arguments)
             {
                 if (!_shutdownCancellationTokenSource.IsCancellationRequested)
                 {
-                    RemoteHostCrashInfoBar.ShowInfoBar(Workspace);
+                    RemoteHostCrashInfoBar.ShowInfoBar(Workspace, ex);
                 }
             }
         }
@@ -341,7 +341,8 @@ private bool ReportUnlessCanceled(Exception ex)
                 return true;
             }
 
-            return FatalError.ReportWithoutCrash(ex);
+            ex.ReportServiceHubNFW("JsonRpc invoke Failed");
+            return true;
         }
     }
 }

src/VisualStudio/Core/Def/Implementation/Watson/WatsonReporter.cs

@@ -33,7 +33,17 @@ internal static class WatsonReporter
         /// <param name="exception">Exception that triggered this non-fatal error</param>
         public static void Report(Exception exception)
         {
-            Report("Roslyn NonFatal Watson", exception);
+            Report("Roslyn NonFatal Watson", exception, WatsonSeverity.Default);
+        }
+
+        /// <summary>
+        /// Report Non-Fatal Watson
+        /// </summary>
+        /// <param name="exception">Exception that triggered this non-fatal error</param>
+        /// <param name="severity">indicate <see cref="WatsonSeverity"/> of NFW</param>
+        public static void Report(Exception exception, WatsonSeverity severity)
+        {
+            Report("Roslyn NonFatal Watson", exception, severity);
         }
 
         /// <summary>
@@ -41,9 +51,10 @@ public static void Report(Exception exception)
         /// </summary>
         /// <param name="description">any description you want to save with this watson report</param>
         /// <param name="exception">Exception that triggered this non-fatal error</param>
-        public static void Report(string description, Exception exception)
+        /// <param name="severity">indicate <see cref="WatsonSeverity"/> of NFW</param>
+        public static void Report(string description, Exception exception, WatsonSeverity severity = WatsonSeverity.Default)
         {
-            Report(description, exception, s_defaultCallback);
+            Report(description, exception, s_defaultCallback, severity);
         }
 
         /// <summary>
@@ -54,8 +65,10 @@ public static void Report(string description, Exception exception)
         /// <param name="callback">Callback to include extra data with the NFW. Note that we always collect
         /// a dump of the current process, but this can be used to add further information or files to the
         /// CAB.</param>
-        public static void Report(string description, Exception exception, Func<IFaultUtility, int> callback)
+        /// <param name="severity">indicate <see cref="WatsonSeverity"/> of NFW</param>
+        public static void Report(string description, Exception exception, Func<IFaultUtility, int> callback, WatsonSeverity severity = WatsonSeverity.Default)
         {
+            var critical = severity == WatsonSeverity.Critical;
             var emptyCallstack = exception.SetCallstackIfEmpty();
 
             if (!WatsonDisabled.s_reportWatson ||
@@ -67,6 +80,7 @@ public static void Report(string description, Exception exception, Func<IFaultUt
             var faultEvent = new FaultEvent(
                 eventName: FunctionId.NonFatalWatson.GetEventName(),
                 description: description,
+                critical ? FaultSeverity.Critical : FaultSeverity.Diagnostic,
                 exceptionObject: exception,
                 gatherEventDetails: arg =>
                 {
@@ -83,13 +97,27 @@ public static void Report(string description, Exception exception, Func<IFaultUt
 
             TelemetryService.DefaultSession.PostEvent(faultEvent);
 
-            if (exception is OutOfMemoryException)
+            if (exception is OutOfMemoryException || critical)
             {
-                // Once we've encountered one OOM we're likely to see more. There will probably be other
-                // failures as a direct result of the OOM, as well. These aren't helpful so we should just
-                // stop reporting failures.
+                // Once we've encountered one OOM or Critial NFW, 
+                // we're likely to see more. There will probably be other
+                // failures as a direct result of the OOM or critical NFW, as well. 
+                // These aren't helpful so we should just stop reporting failures.
                 WatsonDisabled.s_reportWatson = false;
             }
         }
     }
+
+    internal enum WatsonSeverity
+    {
+        /// <summary>
+        /// Indicate that this watson is informative and not urgent
+        /// </summary>
+        Default,
+
+        /// <summary>
+        /// Indicate that this watson is critical and need to be addressed soon
+        /// </summary>
+        Critical,
+    }
 }

src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioErrorReportingService.ExceptionFormatting.cs

@@ -4,6 +4,9 @@
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Text;
+using Newtonsoft.Json.Linq;
+using StreamJsonRpc;
+using StreamJsonRpc.Protocol;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation
 {
@@ -16,16 +19,32 @@ private static string GetFormattedExceptionStack(Exception exception)
                 return GetStackForAggregateException(exception, aggregate);
             }
 
+            if (exception is RemoteInvocationException remoteException)
+            {
+                return GetStackForRemoteException(remoteException);
+            }
+
             return GetStackForException(exception, includeMessageOnly: false);
         }
 
+        private static string GetStackForRemoteException(RemoteInvocationException remoteException)
+        {
+            var text = GetStackForException(remoteException, includeMessageOnly: true);
+            if (remoteException.ErrorData == null)
+            {
+                return text;
+            }
+
+            text = $"{text}{Environment.NewLine}---> (Remote Exception) {remoteException.ErrorData.ToString()} <--- {Environment.NewLine}";
+            return text;
+        }
+
         private static string GetStackForAggregateException(Exception exception, AggregateException aggregate)
         {
             var text = GetStackForException(exception, includeMessageOnly: true);
             for (int i = 0; i < aggregate.InnerExceptions.Count; i++)
             {
-                text = string.Format("{0}{1}---> (Inner Exception #{2}) {3}{4}{5}", text,
-                    Environment.NewLine, i, GetFormattedExceptionStack(aggregate.InnerExceptions[i]), "<---", Environment.NewLine);
+                text = $"{text}{Environment.NewLine}---> (Inner Exception #{i}) {GetFormattedExceptionStack(aggregate.InnerExceptions[i])} <--- {Environment.NewLine}";
             }
 
             return text;

src/Workspaces/Remote/Razor/Shared/WatsonReporter.cs

@@ -19,14 +19,27 @@ namespace Microsoft.CodeAnalysis.ErrorReporting
     /// </summary>
     internal class WatsonReporter
     {
-        public static void Report(string description, Exception exception)
+        public static void Report(string description, Exception exception, WatsonSeverity severity = WatsonSeverity.Default)
         {
             // do nothing
         }
 
-        public static void Report(string description, Exception exception, Func<IFaultUtility, int> callback)
+        public static void Report(string description, Exception exception, Func<IFaultUtility, int> callback, WatsonSeverity severity = WatsonSeverity.Default)
         {
             // do nothing
         }
     }
+
+    internal enum WatsonSeverity
+    {
+        /// <summary>
+        /// Indicate that this watson is informative and not urgent
+        /// </summary>
+        Default,
+
+        /// <summary>
+        /// Indicate that this watson is critical and need to be addressed soon
+        /// </summary>
+        Critical,
+    }
 }

src/Workspaces/Remote/ServiceHub/Services/RemoteHostService.cs

@@ -202,8 +202,8 @@ private void SetGlobalContext(int uiCultureLCID, int cultureLCID, string seriali
             RoslynLogger.SetLogger(AggregateLogger.Create(new VSTelemetryLogger(session), RoslynLogger.GetLogger()));
 
             // set both handler as NFW
-            FatalError.Handler = WatsonReporter.Report;
-            FatalError.NonFatalHandler = WatsonReporter.Report;
+            FatalError.Handler = ex => WatsonReporter.Report(ex, WatsonSeverity.Critical);
+            FatalError.NonFatalHandler = ex => WatsonReporter.Report(ex);
 
             // start performance reporter
             var diagnosticAnalyzerPerformanceTracker = SolutionService.PrimaryWorkspace.Services.GetService<IPerformanceTrackerService>();

src/Workspaces/Remote/ServiceHub/Shared/Extensions.cs

@@ -271,7 +271,7 @@ public static void ReportServiceHubNFW(this Exception exception, string message)
                 return;
             }
 
-            WatsonReporter.Report(message, exception, ReportDetailServiceHubLogs);
+            WatsonReporter.Report(message, exception, ReportDetailServiceHubLogs, WatsonSeverity.Critical);
         }
 
         private static int ReportDetailServiceHubLogs(IFaultUtility faultUtility)

src/Workspaces/Remote/ServiceHub/Telemetry/WatsonReporter.cs

@@ -39,9 +39,10 @@ public static void SetTelemetrySession(TelemetrySession session)
         /// Report Non-Fatal Watson
         /// </summary>
         /// <param name="exception">Exception that triggered this non-fatal error</param>
-        public static void Report(Exception exception)
+        /// <param name="severity">indicate <see cref="WatsonSeverity"/> of NFW</param>
+        public static void Report(Exception exception, WatsonSeverity severity = WatsonSeverity.Default)
         {
-            Report("Roslyn NonFatal Watson", exception);
+            Report("Roslyn NonFatal Watson", exception, severity);
         }
 
         /// <summary>
@@ -49,9 +50,10 @@ public static void Report(Exception exception)
         /// </summary>
         /// <param name="description">any description you want to save with this watson report</param>
         /// <param name="exception">Exception that triggered this non-fatal error</param>
-        public static void Report(string description, Exception exception)
+        /// <param name="severity">indicate <see cref="WatsonSeverity"/> of NFW</param>
+        public static void Report(string description, Exception exception, WatsonSeverity severity = WatsonSeverity.Default)
         {
-            Report(description, exception, s_defaultCallback);
+            Report(description, exception, s_defaultCallback, severity);
         }
 
         /// <summary>
@@ -62,8 +64,10 @@ public static void Report(string description, Exception exception)
         /// <param name="callback">Callback to include extra data with the NFW. Note that we always collect
         /// a dump of the current process, but this can be used to add further information or files to the
         /// CAB.</param>
-        public static void Report(string description, Exception exception, Func<IFaultUtility, int> callback)
+        /// <param name="severity">indicate <see cref="WatsonSeverity"/> of NFW</param>
+        public static void Report(string description, Exception exception, Func<IFaultUtility, int> callback, WatsonSeverity severity = WatsonSeverity.Default)
         {
+            var critical = severity == WatsonSeverity.Critical;
             var emptyCallstack = exception.SetCallstackIfEmpty();
 
             // if given exception is non recoverable exception,
@@ -83,9 +87,18 @@ public static void Report(string description, Exception exception, Func<IFaultUt
                 return;
             }
 
+            // in OOP, we don't fire Critical NFW, rather we fire General which is 1 level higher than Diagnostic
+            // and we keep fire NFW even after critical report. 
+            // critical NFW regarding OOP from VS side will let us to prioritize NFW to fix, and NFW from here should provide
+            // extra dump/info to take a look.
+            // whenever there is an exception in OOP, we fire NFW in both VS and OOP side. VS will report it as critical NFW
+            // and OOP will fire normal NFW. we only mark VS side critical since we don't want to double report same issue
+            // and don't want to shutdown NFW in OOP
+            // one can correlate NFW from VS and OOP through remote callstack in VS NFW
             var faultEvent = new FaultEvent(
                 eventName: FunctionId.NonFatalWatson.GetEventName(),
                 description: description,
+                critical ? FaultSeverity.General : FaultSeverity.Diagnostic,
                 exceptionObject: exception,
                 gatherEventDetails: arg =>
                 {
@@ -108,4 +121,17 @@ private static bool IsNonRecoverableException(Exception exception)
             return exception is OutOfMemoryException;
         }
     }
+
+    internal enum WatsonSeverity
+    {
+        /// <summary>
+        /// Indicate that this watson is informative and not urgent
+        /// </summary>
+        Default,
+
+        /// <summary>
+        /// Indicate that this watson is critical and need to be addressed soon
+        /// </summary>
+        Critical,
+    }
 }