Don't wrap stderr as ErrorRecord#5190
Conversation
.vscode/launch.json
Outdated
There was a problem hiding this comment.
This is being done in #5189. It shouldn't also be in this PR, I don't think.
There was a problem hiding this comment.
I'll remove it, had to change it locally to debug this.
updated tests
What exactly do you mean? Would any |
| this.OutputPipe.Add(sendToPipeline); | ||
| } | ||
|
|
||
| internal void _WriteErrorSkipAllowCheck(object sendToPipeline) |
| Dbg.Assert(record != null, "ProcessReader should ensure that data is ErrorRecord"); | ||
| record.SetInvocationInfo(this.Command.MyInvocation); | ||
| this.commandRuntime._WriteErrorSkipAllowCheck(record, isNativeError: true); | ||
| this.commandRuntime._WriteErrorSkipAllowCheck(outputValue.Data); |
There was a problem hiding this comment.
Please add a comment stating that it is the intent to not wrap native stderror in an ErrorRecord.
| { | ||
| $ErrorActionPreference = $origEA | ||
| } | ||
| It "Verify Validate Dollar Error Populated should be in stderr" { |
There was a problem hiding this comment.
Should we also have a test for stream redirection to output?
$err = & $powershell -noprofile -command { wgwg-wrwrhqwrhrh35h3h3} 2>&1
| /// The Cmdlet should generally just allow PipelineStoppedException | ||
| /// to percolate up to the caller of ProcessRecord etc. | ||
| /// </exception> | ||
| internal void _WriteErrorSkipAllowCheck(object sendToPipeline) |
There was a problem hiding this comment.
Maybe name this WriteNativeCommandErrorSkipAllowCheck instead?
| record.SetInvocationInfo(this.Command.MyInvocation); | ||
| this.commandRuntime._WriteErrorSkipAllowCheck(record, isNativeError: true); | ||
| // write stderr as string instead of as ErrorRecord | ||
| this.commandRuntime._WriteErrorSkipAllowCheck(outputValue.Data); |
There was a problem hiding this comment.
outputValue.Data is still an ErrorRecord - right?
There was a problem hiding this comment.
You're right, outputValue.Data is still an ErrorRecord. I'll need to rework this.
Keep stderr as string and don't wrap as ErrorRecord.
This also means that errors from running pwsh.exe within pwsh are now just stderr strings even though they visually look like ErrorRecords so tests had to be changed.
Most customers are likely not running powershell within powershell and expecting errors, but it does mean that try{}catch{} won't work (until we add the enhancement to throw on any stderr).
Fix #3813