Merge pull request #18287 from tmeschter/ActuallyThrowExceptions

Ensure non-fatal exceptions are actually thrown

src/Compilers/Core/Portable/InternalUtilities/FatalError.cs

@@ -131,13 +131,34 @@ public static bool Report(Exception exception)
         }
 
         /// <summary>
-        /// Use in an exception filter to report a non fatal error.
+        /// Report a non-fatal error.
         /// Calls <see cref="NonFatalHandler"/> and doesn't pass the exception through (the method returns true).
+        /// This is generally expected to be used within an exception filter as that allows us to
+        /// capture data at the point the exception is thrown rather than when it is handled.
+        /// However, it can also be used outside of an exception filter. If the exception has not
+        /// already been thrown the method will throw and catch it itself to ensure we get a useful
+        /// stack trace.
         /// </summary>
         /// <returns>True to catch the exception.</returns>
         [DebuggerHidden]
         public static bool ReportWithoutCrash(Exception exception)
         {
+            // There have been cases where a new, unthrown exception has been passed to this method.
+            // In these cases the exception won't have a stack trace, which isn't very helpful. We
+            // throw and catch the exception here as that will result in a stack trace that is
+            // better than nothing.
+            if (exception.StackTrace == null)
+            {
+                try
+                {
+                    throw exception;
+                }
+                catch
+                {
+                    // Empty; we just need the exception to have a stack trace.
+                }
+            }
+
             Report(exception, s_nonFatalHandler);
             return true;
         }

src/EditorFeatures/Core/SymbolSearch/SymbolSearchUpdateEngine.Update.cs

@@ -564,15 +564,14 @@ private async Task RepeatIOAsync(Func<Task> action)
                         await action().ConfigureAwait(false);
                         return;
                     }
-                    catch (Exception e)
+                    catch (Exception e) when (IOUtilities.IsNormalIOException(e) || _service._reportAndSwallowException(e))
                     {
-                        // Normal IO exception. Don't bother reporting it.  We don't want to get
-                        // lots of hits just because we couldn't write a file because of something
-                        // like an anti-virus tool lockign the file.
-                        if (!IOUtilities.IsNormalIOException(e))
-                        {
-                            _service._reportAndSwallowException(e);
-                        }
+                        // The exception filter above might be a little funny looking. We always
+                        // want to enter this catch block, but if we ran into a normal IO exception
+                        // we shouldn't bother reporting it. We don't want to get lots of hits just
+                        // because something like an anti-virus tool locked the file and we
+                        // couldn't write to it. The call to IsNormalIOException will shortcut
+                        // around the reporting in this case.
 
                         var delay = _service._delayService.FileWriteDelay;
                         await _service.LogExceptionAsync(e, $"Operation failed. Trying again after {delay}").ConfigureAwait(false);
@@ -587,8 +586,7 @@ private async Task RepeatIOAsync(Func<Task> action)
                 var contentsAttribute = element.Attribute(ContentAttributeName);
                 if (contentsAttribute == null)
                 {
-                    _service._reportAndSwallowException(
-                        new FormatException($"Database element invalid. Missing '{ContentAttributeName}' attribute"));
+                    _service._reportAndSwallowException(new FormatException($"Database element invalid. Missing '{ContentAttributeName}' attribute"));
 
                     return ValueTuple.Create(false, (byte[])null);
                 }
@@ -607,8 +605,7 @@ private async Task RepeatIOAsync(Func<Task> action)
 
                     if (!StringComparer.Ordinal.Equals(expectedChecksum, actualChecksum))
                     {
-                        _service._reportAndSwallowException(
-                            new FormatException($"Checksum mismatch: expected != actual. {expectedChecksum} != {actualChecksum}"));
+                        _service._reportAndSwallowException(new FormatException($"Checksum mismatch: expected != actual. {expectedChecksum} != {actualChecksum}"));
 
                         return ValueTuple.Create(false, (byte[])null);
                     }

src/VisualStudio/Core/Def/Utilities/IVsEditorAdaptersFactoryServiceExtensions.cs

@@ -30,8 +30,9 @@ internal static class IVsEditorAdaptersFactoryServiceExtensions
                 var message = contextDocumentId == null
                     ? $"{nameof(contextDocumentId)} was null."
                     : $"{nameof(contextDocumentId)} was not null.";
-                FatalError.ReportWithoutCrash(new InvalidOperationException(
-                    "Could not retrieve document. " + message));
+
+                FatalError.ReportWithoutCrash(new InvalidOperationException("Could not retrieve document. " + message));
+
                 return null;
             }
 

src/Workspaces/Core/Portable/FindSymbols/DeclaredSymbolInfo.cs

@@ -188,7 +188,9 @@ public ISymbol TryResolve(SemanticModel semanticModel, CancellationToken cancell
 $@"Invalid span in {nameof(DeclaredSymbolInfo)}.
 {nameof(this.Span)} = {this.Span}
 {nameof(root.FullSpan)} = {root.FullSpan}";
+
                 FatalError.ReportWithoutCrash(new InvalidOperationException(message));
+
                 return null;
             }
         }

src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex_Create.cs

@@ -89,6 +89,7 @@ internal sealed partial class SyntaxTreeIndex
 $@"Invalid span in {nameof(declaredSymbolInfo)}.
 {nameof(declaredSymbolInfo.Span)} = {declaredSymbolInfo.Span}
 {nameof(root.FullSpan)} = {root.FullSpan}";
+
                                     FatalError.ReportWithoutCrash(new InvalidOperationException(message));
                                 }
                             }