Display errors from build server as part of MsBuild output

src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs

@@ -148,9 +148,18 @@ namespace Microsoft.AspNetCore.Razor.Tasks
                     }
                     else
                     {
-                        Log.LogMessage(
+                        Log.LogMessage(StandardOutputLoggingImportance, $"Server execution completed with return code {result}. For more info, check the server log file in the location specified by the RAZORBUILDSERVER_LOG environment variable.");
-                            StandardOutputLoggingImportance,
+
-                            $"Server execution completed with return code {result}. For more info, check the server log file in the location specified by the RAZORBUILDSERVER_LOG environment variable.");
+                        if (LogStandardErrorAsError)
+                        {
+                            LogErrors(completedResponse.ErrorOutput);
+                        }
+                        else
+                        {
+                            LogMessages(completedResponse.ErrorOutput, StandardErrorLoggingImportance);
+                        }
+
+                        return true;
                     }
                 }
                 else
@@ -158,9 +167,9 @@ namespace Microsoft.AspNetCore.Razor.Tasks
                     Log.LogMessage(
                         StandardOutputLoggingImportance,
                         $"Server execution failed with response {response.Type}. For more info, check the server log file in the location specified by the RAZORBUILDSERVER_LOG environment variable.");
-                }
 
-                result = -1;
+                    result = -1;
+                }
 
                 if (ForceServer)
                 {
@@ -174,6 +183,32 @@ namespace Microsoft.AspNetCore.Razor.Tasks
             return false;
         }
 
+        private void LogMessages(string output, MessageImportance messageImportance)
+        {
+            var lines = output.Split(new string[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
+            foreach (var line in lines)
+            {
+                if (!string.IsNullOrWhiteSpace(line))
+                {
+                    var trimmedMessage = line.Trim();
+                    Log.LogMessageFromText(trimmedMessage, messageImportance);
+                }
+            }
+        }
+
+        private void LogErrors(string output)
+        {
+            var lines = output.Split(new string[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
+            foreach (var line in lines)
+            {
+                if (!string.IsNullOrWhiteSpace(line))
+                {
+                    var trimmedMessage = line.Trim();
+                    Log.LogError(trimmedMessage);
+                }
+            }
+        }
+
         /// <summary>
         /// Get the current directory that the compiler should run in.
         /// </summary>

src/Microsoft.AspNetCore.Razor.Tools/Application.cs

@@ -13,12 +13,20 @@ namespace Microsoft.AspNetCore.Razor.Tools
 {
     internal class Application : CommandLineApplication
     {
-        public Application(CancellationToken cancellationToken, ExtensionAssemblyLoader loader, ExtensionDependencyChecker checker, Func<string, MetadataReferenceProperties, PortableExecutableReference> assemblyReferenceProvider)
+        public Application(
+            CancellationToken cancellationToken,
+            ExtensionAssemblyLoader loader,
+            ExtensionDependencyChecker checker,
+            Func<string, MetadataReferenceProperties, PortableExecutableReference> assemblyReferenceProvider,
+            TextWriter output = null,
+            TextWriter error = null)
         {
             CancellationToken = cancellationToken;
             Checker = checker;
             Loader = loader;
             AssemblyReferenceProvider = assemblyReferenceProvider;
+            Out = output ?? Out;
+            Error = error ?? Error;
 
             Name = "rzc";
             FullName = "Microsoft ASP.NET Core Razor CLI tool";

src/Microsoft.AspNetCore.Razor.Tools/CommandBase.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.CommandLineUtils;
@@ -20,6 +21,8 @@ namespace Microsoft.AspNetCore.Razor.Tools
 
             base.Parent = parent;
             Name = name;
+            Out = parent.Out ?? Out;
+            Error = parent.Error ?? Error;
 
             Help = HelpOption("-?|-h|--help");
             OnExecute((Func<Task<int>>)ExecuteAsync);

src/Microsoft.AspNetCore.Razor.Tools/CompilerHost.cs

@@ -48,27 +48,27 @@ namespace Microsoft.AspNetCore.Razor.Tools
                 }
 
                 var exitCode = 0;
-                var output = string.Empty;
                 var commandArgs = parsed.args.ToArray();
 
-                var writer = ServerLogger.IsLoggingEnabled ? new StringWriter() : TextWriter.Null;
+                var outputWriter = new StringWriter();
+                var errorWriter = new StringWriter();
 
-                var checker = new DefaultExtensionDependencyChecker(Loader, writer, writer);
+                var checker = new DefaultExtensionDependencyChecker(Loader, outputWriter, errorWriter);
-                var app = new Application(cancellationToken, Loader, checker, AssemblyReferenceProvider)
+                var app = new Application(cancellationToken, Loader, checker, AssemblyReferenceProvider, outputWriter, errorWriter);
-                {
-                    Out = writer,
-                    Error = writer,
-                };
 
                 exitCode = app.Execute(commandArgs);
 
-                if (ServerLogger.IsLoggingEnabled)
+                var output = outputWriter.ToString();
-                {
+                var error = errorWriter.ToString();
-                    output = writer.ToString();
-                    ServerLogger.Log(output);
-                }
 
-                return new CompletedServerResponse(exitCode, utf8output: false, output: string.Empty);
+                outputWriter.Dispose();
+                errorWriter.Dispose();
+
+                // This will no-op if server logging is not enabled.
+                ServerLogger.Log(output);
+                ServerLogger.Log(error);
+
+                return new CompletedServerResponse(exitCode, utf8output: false, output, error);
             }
 
             private bool TryParseArguments(ServerRequest request, out (string workingDirectory, string tempDirectory, string[] args) parsed)

src/Microsoft.AspNetCore.Razor.Tools/DiscoverCommand.cs

@@ -49,7 +49,7 @@ namespace Microsoft.AspNetCore.Razor.Tools
         {
             if (string.IsNullOrEmpty(TagHelperManifest.Value()))
             {
-                Error.WriteLine($"{TagHelperManifest.ValueName} must be specified.");
+                Error.WriteLine($"{TagHelperManifest.Description} must be specified.");
                 return false;
             }
 
@@ -66,24 +66,24 @@ namespace Microsoft.AspNetCore.Razor.Tools
 
             if (string.IsNullOrEmpty(Version.Value()))
             {
-                Error.WriteLine($"{Version.ValueName} must be specified.");
+                Error.WriteLine($"{Version.Description} must be specified.");
                 return false;
             }
             else if (!RazorLanguageVersion.TryParse(Version.Value(), out _))
             {
-                Error.WriteLine($"{Version.ValueName} is not a valid language version.");
+                Error.WriteLine($"Invalid option {Version.Value()} for Razor language version --version; must be Latest or a valid version in range {RazorLanguageVersion.Version_1_0} to {RazorLanguageVersion.Latest}.");
                 return false;
             }
 
             if (string.IsNullOrEmpty(Configuration.Value()))
             {
-                Error.WriteLine($"{Configuration.ValueName} must be specified.");
+                Error.WriteLine($"{Configuration.Description} must be specified.");
                 return false;
             }
 
             if (ExtensionNames.Values.Count != ExtensionFilePaths.Values.Count)
             {
-                Error.WriteLine($"{ExtensionNames.ValueName} and {ExtensionFilePaths.ValueName} should have the same number of values.");
+                Error.WriteLine($"{ExtensionNames.Description} and {ExtensionFilePaths.Description} should have the same number of values.");
             }
 
             foreach (var filePath in ExtensionFilePaths.Values)

src/Microsoft.AspNetCore.Razor.Tools/GenerateCommand.cs

@@ -75,18 +75,18 @@ namespace Microsoft.AspNetCore.Razor.Tools
         {
             if (Sources.Values.Count == 0)
             {
-                Error.WriteLine($"{Sources.ValueName} should have at least one value.");
+                Error.WriteLine($"{Sources.Description} should have at least one value.");
                 return false;
             }
 
             if (Outputs.Values.Count != Sources.Values.Count)
             {
-                Error.WriteLine($"{Sources.ValueName} has {Sources.Values.Count}, but {Outputs.ValueName} has {Outputs.Values.Count}.");
+                Error.WriteLine($"{Sources.Description} has {Sources.Values.Count}, but {Outputs.Description} has {Outputs.Values.Count} values.");
             }
 
             if (RelativePaths.Values.Count != Sources.Values.Count)
             {
-                Error.WriteLine($"{Sources.ValueName} has {Sources.Values.Count}, but {RelativePaths.ValueName} has {RelativePaths.Values.Count}.");
+                Error.WriteLine($"{Sources.Description} has {Sources.Values.Count}, but {RelativePaths.Description} has {RelativePaths.Values.Count} values.");
             }
 
             if (string.IsNullOrEmpty(ProjectDirectory.Value()))
@@ -96,24 +96,24 @@ namespace Microsoft.AspNetCore.Razor.Tools
 
             if (string.IsNullOrEmpty(Version.Value()))
             {
-                Error.WriteLine($"{Version.ValueName} must be specified.");
+                Error.WriteLine($"{Version.Description} must be specified.");
                 return false;
             }
             else if (!RazorLanguageVersion.TryParse(Version.Value(), out _))
             {
-                Error.WriteLine($"{Version.ValueName} is not a valid language version.");
+                Error.WriteLine($"Invalid option {Version.Value()} for Razor language version --version; must be Latest or a valid version in range {RazorLanguageVersion.Version_1_0} to {RazorLanguageVersion.Latest}.");
                 return false;
             }
 
             if (string.IsNullOrEmpty(Configuration.Value()))
             {
-                Error.WriteLine($"{Configuration.ValueName} must be specified.");
+                Error.WriteLine($"{Configuration.Description} must be specified.");
                 return false;
             }
 
             if (ExtensionNames.Values.Count != ExtensionFilePaths.Values.Count)
             {
-                Error.WriteLine($"{ExtensionNames.ValueName} and {ExtensionFilePaths.ValueName} should have the same number of values.");
+                Error.WriteLine($"{ExtensionNames.Description} and {ExtensionFilePaths.Description} should have the same number of values.");
             }
 
             foreach (var filePath in ExtensionFilePaths.Values)
@@ -163,12 +163,22 @@ namespace Microsoft.AspNetCore.Razor.Tools
 
             foreach (var result in results)
             {
-                if (result.CSharpDocument.Diagnostics.Count > 0)
+                var errorCount = result.CSharpDocument.Diagnostics.Count;
+                if (errorCount > 0)
                 {
                     success = false;
-                    foreach (var error in result.CSharpDocument.Diagnostics)
+
+                    for (var i = 0; i < errorCount; i++)
                     {
-                        Console.Error.WriteLine(error.ToString());
+                        var error = result.CSharpDocument.Diagnostics[i];
+                        Error.WriteLine(error.ToString());
+
+                        // Only show the first 100 errors to prevent massive string allocations.
+                        if (i == 99)
+                        {
+                            Error.WriteLine($"And {errorCount - i + 1} more errors.");
+                            break;
+                        }
                     }
                 }
 

src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/CompletedServerResponse.cs

@@ -27,16 +27,12 @@ namespace Microsoft.AspNetCore.Razor.Tools
         public readonly string Output;
         public readonly string ErrorOutput;
 
-        public CompletedServerResponse(int returnCode, bool utf8output, string output)
+        public CompletedServerResponse(int returnCode, bool utf8output, string output, string error)
         {
             ReturnCode = returnCode;
             Utf8Output = utf8output;
             Output = output;
-
+            ErrorOutput = error;
-            // This field existed to support writing to Console.Error.  The compiler doesn't ever write to 
-            // this field or Console.Error.  This field is only kept around in order to maintain the existing
-            // protocol semantics.
-            ErrorOutput = string.Empty;
         }
 
         public override ResponseType Type => ResponseType.Completed;
@@ -47,12 +43,8 @@ namespace Microsoft.AspNetCore.Razor.Tools
             var utf8Output = reader.ReadBoolean();
             var output = ServerProtocol.ReadLengthPrefixedString(reader);
             var errorOutput = ServerProtocol.ReadLengthPrefixedString(reader);
-            if (!string.IsNullOrEmpty(errorOutput))
-            {
-                throw new InvalidOperationException();
-            }
 
-            return new CompletedServerResponse(returnCode, utf8Output, output);
+            return new CompletedServerResponse(returnCode, utf8Output, output, errorOutput);
         }
 
         protected override void AddResponseBody(BinaryWriter writer)

src/Microsoft.AspNetCore.Razor.Tools/ShutdownCommand.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Diagnostics;
+using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.CommandLineUtils;

test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs

@@ -3,6 +3,7 @@
 
 using System.IO;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.AspNetCore.Testing.xunit;
 using Xunit;
 
@@ -84,5 +85,23 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests
             Assert.FileExists(result, IntermediateOutputPath, "Whitespace in name.RazorCoreGenerate.cache");
             Assert.FileExists(result, RazorIntermediateOutputPath, "Views", "Home", "Index.cs");
         }
+
+        [Fact]
+        [InitializeTestProject("SimpleMvc")]
+        public async Task Build_ErrorInServer_DisplaysErrorInMsBuildOutput()
+        {
+            var result = await DotnetMSBuild(
+                "Build",
+                "/p:_RazorForceBuildServer=true /p:RazorLangVersion=5.0");
+
+            Assert.BuildFailed(result);
+            Assert.BuildOutputContainsLine(
+                result,
+                $"Invalid option 5.0 for Razor language version --version; must be Latest or a valid version in range {RazorLanguageVersion.Version_1_0} to {RazorLanguageVersion.Latest}.");
+
+            // Compilation failed without creating the views assembly
+            Assert.FileExists(result, IntermediateOutputPath, "SimpleMvc.dll");
+            Assert.FileDoesNotExist(result, IntermediateOutputPath, "SimpleMvc.Views.dll");
+        }
     }
 }

test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/RazorGenerateIntegrationTest.cs

@@ -4,6 +4,7 @@
 using System;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Text;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Testing.xunit;
 using Xunit;
@@ -72,6 +73,28 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests
             Assert.FileExists(result, RazorIntermediateOutputPath, "Views", "Home", "Index.cs");
         }
 
+        [Fact]
+        [InitializeTestProject("SimpleMvc")]
+        public async Task RazorGenerate_LotOfErrorsInRazorFile_ReportsFirstHundredErrors()
+        {
+            // Introducing a syntax error, an unclosed brace
+            var content = new StringBuilder().Insert(0, "@{ ", 100).ToString();
+            ReplaceContent(content, "Views", "Home", "Index.cshtml");
+
+            var result = await DotnetMSBuild(RazorGenerateTarget);
+
+            Assert.BuildFailed(result);
+
+            Assert.BuildOutputContainsLine(result, "And 101 more errors.");
+
+            // RazorGenerate should compile the assembly, but not the views.
+            Assert.FileExists(result, IntermediateOutputPath, "SimpleMvc.dll");
+            Assert.FileDoesNotExist(result, IntermediateOutputPath, "SimpleMvc.Views.dll");
+
+            // The file should still be generated even if we had a Razor syntax error.
+            Assert.FileExists(result, RazorIntermediateOutputPath, "Views", "Home", "Index.cs");
+        }
+
         [ConditionalFact]
         [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "aspnet/Razor#1888")]
         [InitializeTestProject("SimpleMvc")]

test/Microsoft.AspNetCore.Razor.Tools.Test/DefaultRequestDispatcherTest.cs

@@ -19,7 +19,8 @@ namespace Microsoft.AspNetCore.Razor.Tools
         private static ServerResponse EmptyServerResponse => new CompletedServerResponse(
             returnCode: 0,
             utf8output: false,
-            output: string.Empty);
+            output: string.Empty,
+            error: string.Empty);
 
         [Fact]
         public async Task AcceptConnection_ReadingRequestFails_ClosesConnection()

test/Microsoft.AspNetCore.Razor.Tools.Test/ServerLifecycleTest.cs

@@ -20,7 +20,8 @@ namespace Microsoft.AspNetCore.Razor.Tools
         private static ServerResponse EmptyServerResponse => new CompletedServerResponse(
             returnCode: 0,
             utf8output: false,
-            output: string.Empty);
+            output: string.Empty,
+            error: string.Empty);
 
         [Fact]
         public void ServerStartup_MutexAlreadyAcquired_Fails()

test/Microsoft.AspNetCore.Razor.Tools.Test/ServerProtocol/ServerProtocolTest.cs

@@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Razor.Tools
         public async Task ServerResponse_WriteRead_RoundtripsProperly()
         {
             // Arrange
-            var response = new CompletedServerResponse(42, utf8output: false, output: "a string");
+            var response = new CompletedServerResponse(42, utf8output: false, output: "a string", error: "an error");
             var memoryStream = new MemoryStream();
 
             // Act
@@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Razor.Tools
             Assert.Equal(42, result.ReturnCode);
             Assert.False(result.Utf8Output);
             Assert.Equal("a string", result.Output);
-            Assert.Equal("", result.ErrorOutput);
+            Assert.Equal("an error", result.ErrorOutput);
         }
 
         [Fact]