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.

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

Write-Error should use positional binding to -ErrorRecord and -Exception #10739

Open
mklement0 opened this issue Oct 8, 2019 · 4 comments

Comments

@mklement0
Copy link
Contributor

commented Oct 8, 2019

When using the parameter sets selected by -ErrorRecord / -Exception of Write-Error, it makes sense to bind (the first) positional argument of types (derived from) System.Management.Automation.ErrorRecord / System.Exception to those parameters.

Currently, that doesn't happen, because these parameters lack the Position=0 attribute field in their `Parameter attributes.

The result is that using something like Write-Error $_ in an apparent effort to pass the System.Management.Automation.ErrorRecord instance in $_ through, you end up with the equivalent of:

Write-Error -Message $_

rather than the more sensible - and probably expected:

Write-Error -ErrorRecord $_

While the two resulting error records ultimately contain the same message (description), the specifics of the input error record are lost.

The same applies analogously to -Exception.

The fix is trivial: Add Position = 0 to the following two locations:

[Parameter(ParameterSetName = "ErrorRecord", Mandatory = true)]
public ErrorRecord ErrorRecord { get; set; } = null;

[Parameter(ParameterSetName = "WithException", Mandatory = true)]
public Exception Exception { get; set; } = null;

Steps to reproduce

Run the following tests:

# ErrorRecord
try { [int]::parse('foo') } catch {}; (Write-Error $Error[0] 2>&1).Exception.GetType().FullName | Should -Be System.Management.Automation.MethodInvocationException

# Exception
try { [int]::parse('foo') } catch {}; (Write-Error $Error[0].Exception.InnerException 2>&1).Exception.GetType().FullName | Should -Be System.FormatException

# Make sure that a string still binds to -Message.
try { [int]::parse('foo') } catch {}; (Write-Error "$($Error[0])" 2>&1).Exception.GetType().FullName | Should -Be Microsoft.PowerShell.Commands.WriteErrorException

Expected behavior

All tests should pass.

Actual behavior

The first two tests fail, because the positional arguments bind to -Message, resulting in a generic Microsoft.PowerShell.Commands.WriteErrorException exception wrapper.

Environment data

PowerShell Core 7.0.0-preview.4
@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Oct 8, 2019

Does this interfere at all with binding Write-Error "message" positionally? I don't think it should, just want to confirm 🙂

@mklement0

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

Good question, @vexx32 - yes, that still works; I've added tests to the OP to confirm that.

@vexx32

This comment has been minimized.

Copy link
Collaborator

commented Oct 8, 2019

Excellent! This seems like a minor enhancement-type change. Marking it for Hacktoberfest. 😁

@tnieto88

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

I'll work on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.