Throw an exception in OOP if an error is logged. (#12376)

Fixes the first part of #12223

We used to log messages to the output window in VS when formatting was
being abandoned due to bad code. With cohosting that got lost, as the
log messages are now in the service hub log files only. This resurrects
them and makes them into an info bar, by virtue of the exception
handling in Roslyn, so users will know what is going on.

Author: davidwengier

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/HtmlDocumentServices/HtmlDocumentSynchronizer.cs

@@ -174,6 +174,11 @@ private async Task<SynchronizationResult> PublishHtmlDocumentAsync(TextDocument
 
             return result;
         }
+        catch (OperationCanceledException)
+        {
+            _logger.LogDebug($"Not publishing Html text for {document.FilePath} as the request was cancelled.");
+            return default;
+        }
         catch (Exception ex)
         {
             _logger.LogError(ex, $"Error publishing Html text for {document.FilePath}. Html document contents will be stale");

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingContentValidationPass.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Razor.PooledObjects;
 using Microsoft.AspNetCore.Razor.Threading;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Text;
@@ -29,28 +30,37 @@ public Task<bool> IsValidAsync(FormattingContext context, ImmutableArray<TextCha
             // Looks like we removed some non-whitespace content as part of formatting. Oops.
             // Discard this formatting result.
 
-            _logger.LogWarning($"{SR.Format_operation_changed_nonwhitespace}");
-
-            foreach (var change in changes)
-            {
-                if (change.NewText?.Any(c => !char.IsWhiteSpace(c)) ?? false)
-                {
-                    _logger.LogWarning($"{SR.FormatEdit_at_adds(text.GetLinePositionSpan(change.Span), change.NewText)}");
-                }
-                else if (text.TryGetFirstNonWhitespaceOffset(change.Span, out _))
-                {
-                    _logger.LogWarning($"{SR.FormatEdit_at_deletes(text.GetLinePositionSpan(change.Span), text.ToString(change.Span))}");
-                }
-            }
+            var message = GetLogMessage(changes, text);
+            _logger.LogError(message);
 
             if (DebugAssertsEnabled)
             {
-                Debug.Fail("A formatting result was rejected because it was going to change non-whitespace content in the document.");
+                Debug.Fail(message);
             }
 
             return SpecializedTasks.False;
         }
 
         return SpecializedTasks.True;
     }
+
+    private static string GetLogMessage(ImmutableArray<TextChange> changes, SourceText text)
+    {
+        using var _1 = StringBuilderPool.GetPooledObject(out var builder);
+        builder.AppendLine(SR.Format_operation_changed_nonwhitespace);
+
+        foreach (var change in changes)
+        {
+            if (change.NewText?.Any(c => !char.IsWhiteSpace(c)) ?? false)
+            {
+                builder.AppendLine(SR.FormatEdit_at_adds(text.GetLinePositionSpan(change.Span), change.NewText));
+            }
+            else if (text.TryGetFirstNonWhitespaceOffset(change.Span, out _))
+            {
+                builder.AppendLine(SR.FormatEdit_at_deletes(text.GetLinePositionSpan(change.Span), text.ToString(change.Span)));
+            }
+        }
+
+        return builder.ToString();
+    }
 }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingDiagnosticValidationPass.cs

@@ -9,6 +9,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.Language;
+using Microsoft.AspNetCore.Razor.PooledObjects;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Text;
 
@@ -37,22 +38,12 @@ public async Task<bool> IsValidAsync(FormattingContext context, ImmutableArray<T
         // at all possible). Also worth noting the order has to be maintained in that case.
         if (!originalDiagnostics.SequenceEqual(changedDiagnostics, LocationIgnoringDiagnosticComparer.Instance))
         {
-            _logger.LogWarning($"{SR.Format_operation_changed_diagnostics}");
-            _logger.LogWarning($"{SR.Diagnostics_before}");
-            foreach (var diagnostic in originalDiagnostics)
-            {
-                _logger.LogWarning($"{diagnostic}");
-            }
-
-            _logger.LogWarning($"{SR.Diagnostics_after}");
-            foreach (var diagnostic in changedDiagnostics)
-            {
-                _logger.LogWarning($"{diagnostic}");
-            }
+            var message = GetLogMessage(originalDiagnostics, changedDiagnostics);
+            _logger.LogError(message);
 
             if (DebugAssertsEnabled)
             {
-                Debug.Fail("A formatting result was rejected because the formatted text produced different diagnostics compared to the original text.");
+                Debug.Fail(message);
             }
 
             return false;
@@ -61,6 +52,26 @@ public async Task<bool> IsValidAsync(FormattingContext context, ImmutableArray<T
         return true;
     }
 
+    private static string GetLogMessage(ImmutableArray<RazorDiagnostic> originalDiagnostics, ImmutableArray<RazorDiagnostic> changedDiagnostics)
+    {
+        using var _ = StringBuilderPool.GetPooledObject(out var builder);
+
+        builder.AppendLine(SR.Format_operation_changed_diagnostics);
+        builder.AppendLine(SR.Diagnostics_before);
+        foreach (var diagnostic in originalDiagnostics)
+        {
+            builder.AppendLine(diagnostic.ToString());
+        }
+
+        builder.AppendLine(SR.Diagnostics_after);
+        foreach (var diagnostic in changedDiagnostics)
+        {
+            builder.AppendLine(diagnostic.ToString());
+        }
+
+        return builder.ToString();
+    }
+
     private class LocationIgnoringDiagnosticComparer : IEqualityComparer<RazorDiagnostic>
     {
         public static IEqualityComparer<RazorDiagnostic> Instance = new LocationIgnoringDiagnosticComparer();

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Logging/RemoteLoggerFactory.cs

@@ -13,13 +13,20 @@ internal sealed class RemoteLoggerFactory : ILoggerFactory
 {
     private ILoggerFactory _targetLoggerFactory = EmptyLoggerFactory.Instance;
 
-    internal void SetTargetLoggerFactory(ILoggerFactory loggerFactory)
+    /// <summary>
+    /// Sets the logger factory to use in OOP
+    /// </summary>
+    /// <returns>True if the factory was set</returns>
+    internal bool SetTargetLoggerFactory(ILoggerFactory loggerFactory)
     {
         // We only set the target logger factory if the current factory is empty.
         if (_targetLoggerFactory is EmptyLoggerFactory)
         {
             _targetLoggerFactory = loggerFactory;
+            return true;
         }
+
+        return false;
     }
 
     public void AddLoggerProvider(ILoggerProvider provider)

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RazorBrokeredServiceBase.FactoryBase`1.cs

@@ -94,7 +94,7 @@ protected virtual async Task<object> CreateInternalAsync(
             // Note that this means that the first non-empty ILoggerFactory that we use
             // will be used for MEF component logging for the lifetime of all services.
             var remoteLoggerFactory = exportProvider.GetExportedValue<RemoteLoggerFactory>();
-            remoteLoggerFactory.SetTargetLoggerFactory(targetLoggerFactory);
+            var didSetLoggerFactory = remoteLoggerFactory.SetTargetLoggerFactory(targetLoggerFactory);
 
             // In proc services don't use any service hub infra
             if (stream is null)
@@ -103,6 +103,13 @@ protected virtual async Task<object> CreateInternalAsync(
                 return CreateService(in inProcArgs);
             }
 
+            // At this point, we know we're in a remote scenario where we're on the end of a service hub connection, so we want
+            // logged errors to be thrown, so they're bubbled up to the client.
+            if (didSetLoggerFactory)
+            {
+                remoteLoggerFactory.AddLoggerProvider(new ThrowingErrorLoggerProvider());
+            }
+
             var serverConnection = CreateServerConnection(stream, traceSource);
 
             var args = new ServiceArgs(serviceBroker.AssumeNotNull(), exportProvider, targetLoggerFactory, workspaceProvider, serverConnection, brokeredServiceData?.Interceptor);

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ThrowingErrorLoggerProvider.cs

@@ -0,0 +1,43 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Microsoft.CodeAnalysis.Razor.Logging;
+
+namespace Microsoft.CodeAnalysis.Remote.Razor;
+
+/// <summary>
+/// An error logger provider that provides a logger that simple throws an exception for any LogError call.
+/// </summary>
+internal class ThrowingErrorLoggerProvider : ILoggerProvider
+{
+    public ILogger CreateLogger(string categoryName)
+    {
+        return Logger.Instance;
+    }
+
+    private class Logger : ILogger
+    {
+        public static readonly Logger Instance = new();
+
+        public bool IsEnabled(LogLevel logLevel)
+        {
+            return logLevel == LogLevel.Error;
+        }
+
+        public void Log(LogLevel logLevel, string message, Exception? exception)
+        {
+            if (logLevel != LogLevel.Error)
+            {
+                return;
+            }
+
+            if (exception is not null)
+            {
+                throw exception;
+            }
+
+            throw new InvalidOperationException(message);
+        }
+    }
+}