From a12eff96c05956a442fab8075dea7c2461bb142d Mon Sep 17 00:00:00 2001 From: Sergei Vorobev Date: Thu, 28 Jul 2016 18:18:18 -0700 Subject: [PATCH 1/3] Change formatter for ErrorRecord Fix #1465 On Linux, it's common to use error stream (2) as an information stream. There is no errors in this case, it's just a way to report info. When PowerShell redirects error stream, it wraps all output from stream 2 in NativeCommandError ErrorRecord. That doesn't happen for interactive experience (when there is no redirection), but it shows up in logs, scripting and non-console powershell hosts (i.e. PowerShell ISE). Here we change the formatter, so we are keeping the ErrorRecords objects, but we are changing their default representation. If it's a NativeCommandError, we don't use the regular ErrorRecord formatter. It allows us keep ErrorRecord object, but in redirection case it will look exectly like on the screen. Also, we are change NativeCommandErrorMessage to use one line per ErrorRecord to enable this scenario. --- .../DefaultFormatters/PowerShellCore_format_ps1xml.cs | 4 ++-- .../engine/NativeCommandProcessor.cs | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/System.Management.Automation/commands/utility/FormatAndOutput/common/DefaultFormatters/PowerShellCore_format_ps1xml.cs b/src/System.Management.Automation/commands/utility/FormatAndOutput/common/DefaultFormatters/PowerShellCore_format_ps1xml.cs index 84c2d371a..47dd76fab 100644 --- a/src/System.Management.Automation/commands/utility/FormatAndOutput/common/DefaultFormatters/PowerShellCore_format_ps1xml.cs +++ b/src/System.Management.Automation/commands/utility/FormatAndOutput/common/DefaultFormatters/PowerShellCore_format_ps1xml.cs @@ -720,7 +720,7 @@ namespace System.Management.Automation.Runspaces CustomControl.Create(outOfBand: true) .StartEntry() .AddScriptBlockExpressionBinding(@" - if ($_.FullyQualifiedErrorId -ne ""NativeCommandErrorMessage"" -and $ErrorView -ne ""CategoryView"") + if (($_.FullyQualifiedErrorId -ne ""NativeCommandErrorMessage"" -and $_.FullyQualifiedErrorId -ne ""NativeCommandError"") -and $ErrorView -ne ""CategoryView"") { $myinv = $_.InvocationInfo if ($myinv -and $myinv.MyCommand) @@ -767,7 +767,7 @@ namespace System.Management.Automation.Runspaces } ") .AddScriptBlockExpressionBinding(@" - if ($_.FullyQualifiedErrorId -eq ""NativeCommandErrorMessage"") { + if ($_.FullyQualifiedErrorId -eq ""NativeCommandErrorMessage"" -or $_.FullyQualifiedErrorId -eq ""NativeCommandError"") { $_.Exception.Message } else diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 507a5bb7b..8971dd644 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -1880,17 +1880,11 @@ namespace System.Management.Automation // // Wrap the rest of the output in ErrorRecords with the "NativeCommandErrorMessage" error ID // - char[] buffer = new char[4096]; - - int read = 0; - - while ((read = _streamReader.Read(buffer, 0, buffer.Length)) != 0) + while ((line = _streamReader.ReadLine()) != null) { - StringBuilder errorMessage = new StringBuilder().Append(buffer, 0, read); - AddObjectToWriter( new ErrorRecord( - new RemoteException(errorMessage.ToString()), + new RemoteException(line), "NativeCommandErrorMessage", ErrorCategory.NotSpecified, null), From 161940c9aae8aab88324cc1f30d61385bd98991a Mon Sep 17 00:00:00 2001 From: Sergei Vorobev Date: Thu, 28 Jul 2016 20:09:57 -0700 Subject: [PATCH 2/3] Don't apply red color to error stream output --- .../engine/MshCommandRuntime.cs | 6 ++++-- .../engine/NativeCommandProcessor.cs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/MshCommandRuntime.cs b/src/System.Management.Automation/engine/MshCommandRuntime.cs index 489389319..361b1b55f 100644 --- a/src/System.Management.Automation/engine/MshCommandRuntime.cs +++ b/src/System.Management.Automation/engine/MshCommandRuntime.cs @@ -2841,7 +2841,7 @@ namespace System.Management.Automation /// but the command failure will ultimately be /// , /// - internal void _WriteErrorSkipAllowCheck(ErrorRecord errorRecord, ActionPreference? actionPreference = null) + internal void _WriteErrorSkipAllowCheck(ErrorRecord errorRecord, ActionPreference? actionPreference = null, bool isNativeError = false) { ThrowIfStopping(); @@ -2923,7 +2923,9 @@ namespace System.Management.Automation PSObject errorWrap = PSObject.AsPSObject(errorRecord); // It's possible we've already added the member (this method is recursive sometimes // when tracing), so don't add the member again. - if (errorWrap.Members["writeErrorStream"] == null) + + // We don't add a note property on messages that comes from stderr stream. + if (!isNativeError && errorWrap.Members["writeErrorStream"] == null) { PSNoteProperty note = new PSNoteProperty("writeErrorStream", true); errorWrap.Properties.Add(note); diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 8971dd644..3c927cc1e 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -947,7 +947,7 @@ namespace System.Management.Automation ErrorRecord record = outputValue.Data as ErrorRecord; Dbg.Assert(record != null, "ProcessReader should ensure that data is ErrorRecord"); record.SetInvocationInfo(this.Command.MyInvocation); - this.commandRuntime._WriteErrorSkipAllowCheck(record); + this.commandRuntime._WriteErrorSkipAllowCheck(record, isNativeError: true); } else if (outputValue.Stream == MinishellStream.Output) { From 5690b1a0d9d7502a27fe12c2c41f5beea967a4aa Mon Sep 17 00:00:00 2001 From: Sergei Vorobev Date: Mon, 1 Aug 2016 18:39:30 -0700 Subject: [PATCH 3/3] Add tests for Error stream --- .../NativeExecution/NativeStreams.Tests.ps1 | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 test/powershell/Scripting/NativeExecution/NativeStreams.Tests.ps1 diff --git a/test/powershell/Scripting/NativeExecution/NativeStreams.Tests.ps1 b/test/powershell/Scripting/NativeExecution/NativeStreams.Tests.ps1 new file mode 100644 index 000000000..7c191802e --- /dev/null +++ b/test/powershell/Scripting/NativeExecution/NativeStreams.Tests.ps1 @@ -0,0 +1,53 @@ +Describe "Native streams behavior with PowerShell" -Tags 'CI' { + $powershell = Join-Path -Path $PsHome -ChildPath "powershell" + + Context "Error stream" { + # we are using powershell itself as an example of a native program. + # we can create a behavior we want on the fly and test complex scenarios. + + $error.Clear() + + $command = [string]::Join('', @( + '[Console]::Error.Write(\"foo`n`nbar`n`nbaz\"); ', + '[Console]::Error.Write(\"middle\"); ', + '[Console]::Error.Write(\"foo`n`nbar`n`nbaz\")' + )) + + $out = & $powershell -noprofile -command $command 2>&1 + + # this check should be the first one, because $error is a global shared variable + It 'should not add records to $error variable' { + # we are keeping existing Windows PS v5.1 behavior for $error variable + $error.Count | Should Be 9 + } + + It 'uses ErrorRecord object to return stderr output' { + ($out | measure).Count | Should BeGreaterThan 1 + + $out[0] | Should BeOfType 'System.Management.Automation.ErrorRecord' + $out[0].FullyQualifiedErrorId | Should Be 'NativeCommandError' + + $out | Select-Object -Skip 1 | % { + $_ | Should BeOfType 'System.Management.Automation.ErrorRecord' + $_.FullyQualifiedErrorId | Should Be 'NativeCommandErrorMessage' + } + } + + It 'uses correct exception messages for error stream' { + ($out | measure).Count | Should Be 9 + $out[0].Exception.Message | Should Be 'foo' + $out[1].Exception.Message | Should Be '' + $out[2].Exception.Message | Should Be 'bar' + $out[3].Exception.Message | Should Be '' + $out[4].Exception.Message | Should Be 'bazmiddlefoo' + $out[5].Exception.Message | Should Be '' + $out[6].Exception.Message | Should Be 'bar' + $out[7].Exception.Message | Should Be '' + $out[8].Exception.Message | Should Be 'baz' + } + + It 'preserves error stream as is with Out-String' { + ($out | Out-String).Replace("`r", '') | Should Be "foo`n`nbar`n`nbazmiddlefoo`n`nbar`n`nbaz`n" + } + } +}