diff --git a/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs b/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs index 2539d3e5ed..45e4a2019c 100644 --- a/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs +++ b/src/Microsoft.AspNetCore.Razor.Tasks/DotnetToolTask.cs @@ -148,9 +148,18 @@ namespace Microsoft.AspNetCore.Razor.Tasks } else { - 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."); + 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."); + + 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); + } + } + } + /// /// Get the current directory that the compiler should run in. /// diff --git a/src/Microsoft.AspNetCore.Razor.Tools/Application.cs b/src/Microsoft.AspNetCore.Razor.Tools/Application.cs index c9f569747a..ba043a2b40 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/Application.cs +++ b/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 assemblyReferenceProvider) + public Application( + CancellationToken cancellationToken, + ExtensionAssemblyLoader loader, + ExtensionDependencyChecker checker, + Func 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"; diff --git a/src/Microsoft.AspNetCore.Razor.Tools/CommandBase.cs b/src/Microsoft.AspNetCore.Razor.Tools/CommandBase.cs index a25a382212..d391337cab 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/CommandBase.cs +++ b/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>)ExecuteAsync); diff --git a/src/Microsoft.AspNetCore.Razor.Tools/CompilerHost.cs b/src/Microsoft.AspNetCore.Razor.Tools/CompilerHost.cs index ce1b0791f2..0525beed06 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/CompilerHost.cs +++ b/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 app = new Application(cancellationToken, Loader, checker, AssemblyReferenceProvider) - { - Out = writer, - Error = writer, - }; + var checker = new DefaultExtensionDependencyChecker(Loader, outputWriter, errorWriter); + var app = new Application(cancellationToken, Loader, checker, AssemblyReferenceProvider, outputWriter, errorWriter); exitCode = app.Execute(commandArgs); - if (ServerLogger.IsLoggingEnabled) - { - output = writer.ToString(); - ServerLogger.Log(output); - } + var output = outputWriter.ToString(); + var error = errorWriter.ToString(); - 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) diff --git a/src/Microsoft.AspNetCore.Razor.Tools/DiscoverCommand.cs b/src/Microsoft.AspNetCore.Razor.Tools/DiscoverCommand.cs index 5ff3f07d52..0ea45724d3 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/DiscoverCommand.cs +++ b/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) diff --git a/src/Microsoft.AspNetCore.Razor.Tools/GenerateCommand.cs b/src/Microsoft.AspNetCore.Razor.Tools/GenerateCommand.cs index b1db811079..da2af99f91 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/GenerateCommand.cs +++ b/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; + } } } diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/CompletedServerResponse.cs b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/CompletedServerResponse.cs index 49a9c5a33a..98a64dba8f 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/CompletedServerResponse.cs +++ b/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; - - // 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; + ErrorOutput = error; } 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) diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ShutdownCommand.cs b/src/Microsoft.AspNetCore.Razor.Tools/ShutdownCommand.cs index 3e0907774b..a2bedee889 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ShutdownCommand.cs +++ b/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; diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs index 7ffbbc27e9..9caed65993 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs +++ b/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"); + } } } diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/RazorGenerateIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/RazorGenerateIntegrationTest.cs index 0834a37974..2566375769 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/RazorGenerateIntegrationTest.cs +++ b/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")] diff --git a/test/Microsoft.AspNetCore.Razor.Tools.Test/DefaultRequestDispatcherTest.cs b/test/Microsoft.AspNetCore.Razor.Tools.Test/DefaultRequestDispatcherTest.cs index 495649bd3e..c852648dc2 100644 --- a/test/Microsoft.AspNetCore.Razor.Tools.Test/DefaultRequestDispatcherTest.cs +++ b/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() diff --git a/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerLifecycleTest.cs b/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerLifecycleTest.cs index f91fbc5fdd..d2e46631ec 100644 --- a/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerLifecycleTest.cs +++ b/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() diff --git a/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerProtocol/ServerProtocolTest.cs b/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerProtocol/ServerProtocolTest.cs index 9ce69a6f54..86a7beb8be 100644 --- a/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerProtocol/ServerProtocolTest.cs +++ b/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]