Merge pull request #33589 from jasonmalinowski/add-various-documentclose-telemetry

Add various bits of telemetry around DocumentClosed handling

src/Workspaces/Core/Portable/Utilities/EventMap.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using Microsoft.CodeAnalysis.ErrorReporting;
 
 namespace Roslyn.Utilities
 {
@@ -165,13 +166,26 @@ public bool HasHandlers
 
             public void RaiseEvent(Action<TEventHandler> invoker)
             {
-                if (this.HasHandlers)
+                // The try/catch here is to find additional telemetry for https://devdiv.visualstudio.com/DevDiv/_queries/query/71ee8553-7220-4b2a-98cf-20edab701fd1/.
+                // We've realized there's a problem with our eventing, where if an exception is encountered while calling into subscribers to Workspace events,
+                // we won't notify all of the callers. The expectation is such an exception would be thrown to the SafeStartNew in the workspace's event queue that
+                // will raise that as a fatal exception, but OperationCancelledExceptions might be able to propagate through and fault the task we are using in the
+                // chain. I'm choosing to use ReportWithoutCrashAndPropagate, because if our theory here is correct, it seems the first exception isn't actually
+                // causing crashes, and so if it turns out this is a very common situation I don't want to make a often-benign situation fatal.
+                try
                 {
-                    foreach (var registry in _registries)
+                    if (this.HasHandlers)
                     {
-                        registry.Invoke(invoker);
+                        foreach (var registry in _registries)
+                        {
+                            registry.Invoke(invoker);
+                        }
                     }
                 }
+                catch (Exception e) when (FatalError.ReportWithoutCrashAndPropagate(e))
+                {
+                    throw ExceptionUtilities.Unreachable;
+                }
             }
         }
     }

src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
@@ -530,24 +531,35 @@ protected internal void OnDocumentClosed(DocumentId documentId, TextLoader reloa
             this.CheckDocumentIsInCurrentSolution(documentId);
             this.CheckDocumentIsOpen(documentId);
 
-            using (_serializationLock.DisposableWait())
+            // The try/catch here is to find additional telemetry for https://devdiv.visualstudio.com/DevDiv/_queries/query/71ee8553-7220-4b2a-98cf-20edab701fd1/,
+            // where we have one theory that OnDocumentClosed is running but failing somewhere in the middle and thus failing to get to the RaiseDocumentClosedEventAsync() line. 
+            // We are choosing ReportWithoutCrashAndPropagate because this is a public API that has callers outside VS and also non-VisualStudioWorkspace callers inside VS, and
+            // we don't want to be crashing underneath them if they were already handling exceptions or (worse) was using those exceptions for expected code flow.
+            try
             {
-                // forget any open document info
-                ClearOpenDocument(documentId);
+                using (_serializationLock.DisposableWait())
+                {
+                    // forget any open document info
+                    ClearOpenDocument(documentId);
 
-                var oldSolution = this.CurrentSolution;
-                var oldDocument = oldSolution.GetDocument(documentId);
+                    var oldSolution = this.CurrentSolution;
+                    var oldDocument = oldSolution.GetDocument(documentId);
 
-                this.OnDocumentClosing(documentId);
+                    this.OnDocumentClosing(documentId);
 
-                var newSolution = oldSolution.WithDocumentTextLoader(documentId, reloader, PreservationMode.PreserveValue);
-                newSolution = this.SetCurrentSolution(newSolution);
+                    var newSolution = oldSolution.WithDocumentTextLoader(documentId, reloader, PreservationMode.PreserveValue);
+                    newSolution = this.SetCurrentSolution(newSolution);
 
-                var newDoc = newSolution.GetDocument(documentId);
-                this.OnDocumentTextChanged(newDoc);
+                    var newDoc = newSolution.GetDocument(documentId);
+                    this.OnDocumentTextChanged(newDoc);
 
-                this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.DocumentChanged, oldSolution, newSolution, documentId: documentId); // don't wait for this
-                this.RaiseDocumentClosedEventAsync(newDoc); // don't wait for this
+                    this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.DocumentChanged, oldSolution, newSolution, documentId: documentId); // don't wait for this
+                    this.RaiseDocumentClosedEventAsync(newDoc); // don't wait for this
+                }
+            }
+            catch (Exception e) when (FatalError.ReportWithoutCrashAndPropagate(e))
+            {
+                throw ExceptionUtilities.Unreachable;
             }
         }