Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error formatting for pipeline enumeration exceptions #20211

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Fix error formatting for pipeline enumeration exceptions #20211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0599079
Fix error view for pipeline enumeration exceptions
SeeminglyScience Sep 6, 2023
8e8e3a5
Fix test failure
SeeminglyScience Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 49 additions & 34 deletions ...m.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,8 @@ private static IEnumerable<FormatViewDefinition> ViewsOf_System_Management_Autom
yield return new FormatViewDefinition("ErrorInstance",
CustomControl.Create(outOfBand: true)
.StartEntry()
.AddScriptBlockExpressionBinding(@"
.AddScriptBlockExpressionBinding(
"""
$errorColor = ''
$commandPrefix = ''
if (@('NativeCommandErrorMessage','NativeCommandError') -notcontains $_.FullyQualifiedErrorId -and @('CategoryView','ConciseView','DetailedView') -notcontains $ErrorView)
Expand Down Expand Up @@ -1076,8 +1077,9 @@ private static IEnumerable<FormatViewDefinition> ViewsOf_System_Management_Autom
}

$errorColor + $commandPrefix
")
.AddScriptBlockExpressionBinding(@"
""")
.AddScriptBlockExpressionBinding(
"""
Set-StrictMode -Off
$ErrorActionPreference = 'Stop'
trap { 'Error found in error view definition: ' + $_.Exception.Message }
Expand Down Expand Up @@ -1111,34 +1113,47 @@ function Get-ConciseViewPositionMessage {
$message = ''
$prefix = ''

# Handle case where there is a TargetObject from a Pester `Should` assertion failure and we can show the error at the target rather than the script source
# Note that in some versions, this is a Dictionary<,> and in others it's a hashtable. So we explicitly cast to a shared interface in the method invocation
# to force using `IDictionary.Contains`. Hashtable does have it's own `ContainKeys` as well, but if they ever opt to use a custom `IDictionary`, that may not.
$useTargetObject = $null -ne $err.TargetObject -and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find where $err is defined before this use. Can you please point me to it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find where $err is defined before this use. Can you please point me to it?

Yeah it's a little confusing. This is inside the nested function Get-ConciseViewPositionMessage. Then look at this

image

As labeled:

  1. is the ending bracket for the function definition of Get-ConciseViewPositionMessage
  2. is where $err is set
  3. is where the function is called in the main body of the format view

I kept with the existing style, but at some point it may be worth rewriting some parts of this for maintainability.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

$err.TargetObject -is [System.Collections.IDictionary] -and
([System.Collections.IDictionary]$err.TargetObject).Contains('Line') -and
([System.Collections.IDictionary]$err.TargetObject).Contains('LineText')

# The checks here determine if we show line detailed error information:
# - check if `ParserError` and comes from PowerShell which eventually results in a ParseException, but during this execution it's an ErrorRecord
$isParseError = $err.CategoryInfo.Category -eq 'ParserError' -and
$err.Exception -is [System.Management.Automation.ParentContainsErrorRecordException]

# - check if invocation is a script or multiple lines in the console
$isMultiLineOrExternal = $myinv.ScriptName -or $myinv.ScriptLineNumber -gt 1

# - check that it's not a script module as expectation is that users don't want to see the line of error within a module
if ((($err.CategoryInfo.Category -eq 'ParserError' -and $err.Exception -is 'System.Management.Automation.ParentContainsErrorRecordException') -or $myinv.ScriptName -or $myinv.ScriptLineNumber -gt 1) -and $myinv.ScriptName -notmatch '\.psm1$') {
$useTargetObject = $false
$shouldShowLineDetail = ($isParseError -or $isMultiLineOrExternal) -and
$myinv.ScriptName -notmatch '\.psm1$'

if ($useTargetObject -or $shouldShowLineDetail) {

# Handle case where there is a TargetObject and we can show the error at the target rather than the script source
if ($_.TargetObject.Line -and $_.TargetObject.LineText) {
$posmsg = ""${resetcolor}$($_.TargetObject.File)${newline}""
$useTargetObject = $true
if ($useTargetObject) {
$posmsg = "${resetcolor}$($err.TargetObject.File)${newline}"
}
elseif ($myinv.ScriptName) {
if ($env:TERM_PROGRAM -eq 'vscode') {
# If we are running in vscode, we know the file:line:col links are clickable so we use this format
$posmsg = ""${resetcolor}$($myinv.ScriptName):$($myinv.ScriptLineNumber):$($myinv.OffsetInLine)${newline}""
$posmsg = "${resetcolor}$($myinv.ScriptName):$($myinv.ScriptLineNumber):$($myinv.OffsetInLine)${newline}"
}
else {
$posmsg = ""${resetcolor}$($myinv.ScriptName):$($myinv.ScriptLineNumber)${newline}""
$posmsg = "${resetcolor}$($myinv.ScriptName):$($myinv.ScriptLineNumber)${newline}"
}
}
else {
$posmsg = ""${newline}""
$posmsg = "${newline}"
}

if ($useTargetObject) {
$scriptLineNumber = $_.TargetObject.Line
$scriptLineNumberLength = $_.TargetObject.Line.ToString().Length
$scriptLineNumber = $err.TargetObject.Line
$scriptLineNumberLength = $err.TargetObject.Line.ToString().Length
}
else {
$scriptLineNumber = $myinv.ScriptLineNumber
Expand All @@ -1155,7 +1170,7 @@ function Get-ConciseViewPositionMessage {
}

$verticalBar = '|'
$posmsg += ""${accentColor}${headerWhitespace}Line ${verticalBar}${newline}""
$posmsg += "${accentColor}${headerWhitespace}Line ${verticalBar}${newline}"

$highlightLine = ''
if ($useTargetObject) {
Expand All @@ -1180,19 +1195,19 @@ function Get-ConciseViewPositionMessage {
$line = $line.Insert($offsetInLine + $offsetLength, $resetColor).Insert($offsetInLine, $accentColor)
}

$posmsg += ""${accentColor}${lineWhitespace}${ScriptLineNumber} ${verticalBar} ${resetcolor}${line}""
$posmsg += "${accentColor}${lineWhitespace}${ScriptLineNumber} ${verticalBar} ${resetcolor}${line}"
$offsetWhitespace = ' ' * $offsetInLine
$prefix = ""${accentColor}${headerWhitespace} ${verticalBar} ${errorColor}""
$prefix = "${accentColor}${headerWhitespace} ${verticalBar} ${errorColor}"
if ($highlightLine -ne '') {
$posMsg += ""${prefix}${highlightLine}${newline}""
$posMsg += "${prefix}${highlightLine}${newline}"
}
$message = ""${prefix}""
$message = "${prefix}"
}

if (! $err.ErrorDetails -or ! $err.ErrorDetails.Message) {
if ($err.CategoryInfo.Category -eq 'ParserError' -and $err.Exception.Message.Contains(""~$newline"")) {
if ($err.CategoryInfo.Category -eq 'ParserError' -and $err.Exception.Message.Contains("~$newline")) {
# need to parse out the relevant part of the pre-rendered positionmessage
$message += $err.Exception.Message.split(""~$newline"")[1].split(""${newline}${newline}"")[0]
$message += $err.Exception.Message.split("~$newline")[1].split("${newline}${newline}")[0]
}
elseif ($err.Exception) {
$message += $err.Exception.Message
Expand All @@ -1214,7 +1229,7 @@ function Get-ConciseViewPositionMessage {
$prefixVtLength = $prefix.Length - $prefixLength

# replace newlines in message so it lines up correct
$message = $message.Replace($newline, ' ').Replace(""`n"", ' ').Replace(""`t"", ' ')
$message = $message.Replace($newline, ' ').Replace("`n", ' ').Replace("`t", ' ')

$windowWidth = 120
if ($Host.UI.RawUI -ne $null) {
Expand Down Expand Up @@ -1249,7 +1264,7 @@ function Get-ConciseViewPositionMessage {
$message += $newline
}

$posmsg += ""${errorColor}"" + $message
$posmsg += "${errorColor}" + $message

$reason = 'Error'
if ($err.Exception -and $err.Exception.WasThrownFromThrowStatement) {
Expand All @@ -1261,8 +1276,8 @@ function Get-ConciseViewPositionMessage {
$reason = $myinv.MyCommand
}
# If it's a scriptblock, better to show the command in the scriptblock that had the error
elseif ($_.CategoryInfo.Activity) {
$reason = $_.CategoryInfo.Activity
elseif ($err.CategoryInfo.Activity) {
$reason = $err.CategoryInfo.Activity
}
elseif ($myinv.MyCommand) {
$reason = $myinv.MyCommand
Expand All @@ -1279,7 +1294,7 @@ function Get-ConciseViewPositionMessage {

$errorMsg = 'Error'

""${errorColor}${reason}: ${posmsg}${resetcolor}""
"${errorColor}${reason}: ${posmsg}${resetcolor}"
}

$myinv = $_.InvocationInfo
Expand All @@ -1290,17 +1305,17 @@ function Get-ConciseViewPositionMessage {
}

if ($err.FullyQualifiedErrorId -eq 'NativeCommandErrorMessage' -or $err.FullyQualifiedErrorId -eq 'NativeCommandError') {
return ""${errorColor}$($err.Exception.Message)${resetcolor}""
return "${errorColor}$($err.Exception.Message)${resetcolor}"
}

if ($ErrorView -eq 'DetailedView') {
$message = Get-Error | Out-String
return ""${errorColor}${message}${resetcolor}""
return "${errorColor}${message}${resetcolor}"
}

if ($ErrorView -eq 'CategoryView') {
$message = $err.CategoryInfo.GetMessage()
return ""${errorColor}${message}${resetcolor}""
return "${errorColor}${message}${resetcolor}"
}

$posmsg = ''
Expand All @@ -1320,7 +1335,7 @@ function Get-ConciseViewPositionMessage {

if ($ErrorView -eq 'ConciseView') {
if ($err.PSMessageDetails) {
$posmsg = ""${errorColor}${posmsg}""
$posmsg = "${errorColor}${posmsg}"
}
return $posmsg
}
Expand All @@ -1340,14 +1355,14 @@ function Get-ConciseViewPositionMessage {

$posmsg += $newline + $indentString

$indentString = ""+ FullyQualifiedErrorId : "" + $err.FullyQualifiedErrorId
$indentString = "+ FullyQualifiedErrorId : " + $err.FullyQualifiedErrorId
$posmsg += $newline + $indentString

$originInfo = $err.OriginInfo

if (($null -ne $originInfo) -and ($null -ne $originInfo.PSComputerName))
{
$indentString = ""+ PSComputerName : "" + $originInfo.PSComputerName
$indentString = "+ PSComputerName : " + $originInfo.PSComputerName
$posmsg += $newline + $indentString
}

Expand All @@ -1357,8 +1372,8 @@ function Get-ConciseViewPositionMessage {
$err.Exception.Message + $posmsg
}

""${errorColor}${finalMsg}${resetcolor}""
")
"${errorColor}${finalMsg}${resetcolor}"
""")
.EndEntry()
.EndControl());
}
Expand Down
9 changes: 9 additions & 0 deletions test/powershell/engine/Formatting/ErrorView.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ Describe 'Tests for $ErrorView' -Tag CI {
start-job { [cmdletbinding()]param() $e = [System.Management.Automation.ErrorRecord]::new([System.Exception]::new('hello'), 1, 'ParserError', $null); $pscmdlet.ThrowTerminatingError($e) } | Wait-Job | Receive-Job -ErrorVariable e -ErrorAction SilentlyContinue
$e | Out-String | Should -BeLike '*ParserError*'
}

It 'Exception thrown from Enumerator.MoveNext in a pipeline shows information' {
$e = {
$l = [System.Collections.Generic.List[string]] @('one', 'two')
$l | ForEach-Object { $null = $l.Remove($_) }
} | Should -Throw -ErrorId 'BadEnumeration' -PassThru | Out-String

$e | Should -BeLike 'InvalidOperation:*'
}
}

Context 'NormalView tests' {
Expand Down