PowerShellPracticeAndStyle
PowerShellPracticeAndStyle copied to clipboard
PoshCode
When should you write an error if you have no output?
There's been some discussion recently about when it's appropriate to write errors ...
On behalf of the PowerShell Committee, @SteveL-MSFT wrote this:
@PowerShell/powershell-committee discussed this and we also discussed it in this morning's Community Call. We agreed that the intent is that if a literal is passed and is not found, it should return a non-terminating error. Cmdlets that don't have this behavior have bugs.
In cases where it's explicit or implicit filtering, it should return nothing.
The real issue is that we are missing Test-* cmdlets so users had to rely on current behavior with Get-* cmdlets to see if something exists and we should add those cmdlets in the future.
And then later clarified:
- the intent is for a literal to return a non-terminating error, but wildcards/filtering to return nothing
- we do not plan to accept changes to existing behavior to make them consistent, but any new cmdlets should adhere to this
- even without changing the existing cmdlets, we still want to introduce Test-* cmdlets and make that the norm and best practice
- if not documented, we need to document existing cmdlet behavior where it deviates from the intent
So I ask this community: is that right?
Should a Get command that takes a literal name always write an error when it does not return anything?
Pretend that PowerShell was green-field, and that whatever you decide here will not break existing code.
Would you want errors from commands like:
- Get-Module ModuleThatIsNotLoaded
- Get-Module ModuleThatIsNotInstalled -ListAvailable
- Get-ChildItem FolderThatDoesNotExist
- Get-ChildItem EmptyFolder
- Get-TypeData System.Boolean # there's no TypeData for this by default
What about listing commands where we expect to always have data, but there are edge cases:
- Get-Command -Verb Build # there aren't any commands that use this verb yet
- Get-History # As the first command in a session
- Get-Clipboard # If the clipboard is empty
Spoiler: I think this is wrong.
Here is the clearest, most obvious counter-example I can think of right now: Get-AzureKeyVaultSecret
I use this command because it's one that has a few easy to understand use cases and specific issues that lead to a specific design (in my opinion). It has a few parameter sets:
- If you don't pass a name, it will output ALL the secrets
- If you pass a name, it will output only that one secret (the latest version of it, or a specified version of it), except ...
- If you specify -IncludeVersions (instead of a specific version) it will output the entire history of the specific thing
- You can extend your search to deleted items
In each case, if it doesn't find a matching value, it quietly returns nothing. I consider that to be the best possible design, because:
- As a web command, it's likely to encounter connectivity problems, resulting in terminating errors
- As a command that accesses data that is secure and has access controls, it's likely to encounter permission errors even when the data exists.
However, any time you're looking up an item by name, it's very likely that the value doesn't exist (yet). If the command produced an error for the "everything worked fine, but there's nothing to return" case, I believe it would be extremely complicated to use correctly, and would generally result in careless or inexperienced users hiding the important errors.
Error handling for these cases is terrible
The first time a user calls the command and it returns a trivial "not found" error, they might choose to suppress that error by adding -ErrorAction Ignore or SilentlyContinue and would then miss out on the errors that are actually important (i.e. connectivity problems, or access errors like "Operation 'list' is not allowed by vault policy").
A design which produces errors for missing values forces careful users to capture the error and check which error it was. In fact ...
if the error is not terminating, it's even worse than if it were terminating like the others.
In order to suppress a non-terminating error, you must use -ErrorAction and either:
- set it to
SilentlyContinue(which also suppresses terminating exceptions) and capture the error, test the result, and throw if it's not the simple error - set it to
Stopand wrap it in try/catch in order to test the error type, ignore the "does not exist" error and rethrow the rest.
However, even if the command simply throws a different unique exception for each use case (or writes an error with a different unique error category), producing an error in this situation results in the user needing to explicitly handle the error or avoid it ...
In order to allow you to avoid the error, the PowerShell Committee's proposal essentially requires every Get command to have a Test command.
For expensive commands, I'm afraid this this is a terrible idea
If Get-AzureKeyVaultSecret wrote errors for missing entries and had a separate Test command, then in order to avoid the very complicated error handling for the Get command, you would need to instead wrap your Get call in a Test call.
The problem is that the Test call would require the same slow and heavy round-trip, authenticated network request, and would still have all the same exception cases for access and networking errors.
Worse yet, you're not just making TWO requests when you could just make one -- it's actually the exact same request each time, because there's no REST method for "test" so you would be calling the GET method for Test and for Get ... returning the full data each time ...
On phone so employing brevity.
I strongly disagree that there should be an error.
I see
Get-ChildItem EmptyFolder
As
Give me all of the contents of the folder.
If there aren't any don't return anything. It's not an error if it is doing what I ask.
I also think it is misleading as "an error" means something is wrong. Noth in ng to return is not "wrong"
I strongly agree with Joel.
Returning nothing when there is nothing to return is the appropriate response and not an error.
I don't think Get-ChildItem EmptyFolder is in question here. What the PS committee says is this:
We agreed that the intent is that if a literal is passed and is not found ...
Well, EmptyFolder was found but it was empty. So this proposal does NOT suggest that this scenario should generate a non-terminating error. However the following does error:
PS> Get-ChildItem xyzzy
Get-ChildItem : Cannot find path 'C:\Users\hillr\xyzzy' because it does not exist.
...
And I think rightly so. CMD and Bash also error in this case.
And while we're looking at these examples ... Get-Clipboard isn't a command on PowerShell Core and even on Windows PowerShell, it only takes switch params. So I'm not sure how it applies to this proposal. Get-Command -Verb build is a filtering command that doesn't error. Check. Get-Command xyzzy looks for a specific command and does generate a non-terminating error. Check (and good). Get-History does not error when you run it right after PS starts - when there is no history yet. Check. However it fails if you pass it an id that doesn't exist Get-History -id 8675309. Check (also good).
RE 1, this is a filtering scenario therefore it is behaving as designed - to not return a non-term error. Personally I'm not big fan of these "inherently filtering" commands. If I wanted filtering I would just specify Get-Module ModuleThatIsNotLoaded*. Then I would have more control (filtering or not). Also, there are cases where I want an error when a module is not found. Now, I have to generate that myself rather than rely on -EA stop. Oh well. Too late to make any changes to this command.
RE 2, doesn't error now and wouldn't error if implemented under the proposal IMO. The use of -ListAvailable makes this a filtering scenario.
RE 5, it would have been nice if this generated a non-terminating error IMO. Again would have been easy to avoid with Get-TypeData System.Boolean* or Get-TypeData System.Boolean -ea 0. But "probably" not worth changing at this point.
Back to the original question:
When should you write an error if you have no output?
IMO you error when you don't have the specific item I asked for and your design is to only return a single item i.e. not all things that match the term I specified. But please, require a wildcard char for filtering - that's what they're for. However, there are nuances here, I'll give you that. Like asking for the contents of a specific (empty) folder should not generate an error. Or asking for the "contents" of a specific (empty) file should not generate an error either.
they might choose to suppress that error by adding -ErrorAction Ignore or SilentlyContinue and would then miss out on the errors that are actually important (i.e. connectivity problems ...
Just because a command writes non-terminating errors doesn't mean it can't also throw terminating errors. A connectivity problem should result in a terminating error since there's no way the command can proceed.
I can see that there are strong views for this to continue as originally specified but now I have access to a proper keyboard I will respond again.
This is the conundrum
If
Get-ChildItem EmptyFolder Get-Module ModuleThatIsNotLoaded
should not return an error when there is nothing there
why should
Get-Command NoneExisitngCommand
return an error when there is no command of that name? I have asked for this particular command and I have received an error rather than either
a, Here is precisely what you asked for - nothing b, A more useful message to the user than a full error message
I disagree that this should be an error. I also find that it confuses newcomers. An error and a sea of red text means something is wrong and in this case nothing is wrong, it just doesn't exist.
Personally, I want it to return nothing because there is nothing to be returned. I would be happy with a warning but I see it as a direct question that should be return in a pedantic manner.
"Give me all of the Printers named foo please"
"Here are all of the printers named foo "
Funnily enough 5 minutes afer writing this I find another example that maybe also explains why I see it his way. This is what I get with the virtualmachinemanager module right now too.
Give me all of the Virtual Machines named $Vmname (There aren't any - we are cleaning up a system of old data prior to upgrade)
Which makes my coding easier and training the users easier also ;-)
It makes it easier for users at the command line and it makes it easier for error handling too as Joel mentions above more eloquently than I can.
Thats my opinion
@rkeithhill but you've ignored my argument in favor of picking apart the op examples. let's set all of that aside. Read my first reply, and explain to me how your logic applies to Get-AzureKeyVaultSecret (after that, we can talk about all these other examples from a point of understanding each other, rather than just defending the current behavior).
However, this comment seems like a HUGE misunderstanding, which in my opinion is the root cause of your point of view (and everyone who thinks these should error):
Just because a command writes non-terminating errors doesn't mean it can't also throw terminating errors. A connectivity problem should result in a terminating error since there's no way the command can proceed.
That's simply not reasonable. Take this sample command:
function Test-Error {
[CmdletBinding()]
param(
[switch]$Exception,
[switch]$Error
)
if($Error){
Write-Error "Test Error" -Category SecurityError
}
if($Exception) {
throw ([Exception]::new("Syntax Problem"))
}
}
Are you saying that the user should just learn to ignore and live with the huge block of red from Write-Error, or do you have a magic incantation whereby they can suppress it without suppressing the exception?
do you have a magic incantation whereby he can suppress it without suppressing the other?
Yup:
PS> try { Test-Error -Error -Exception -ea 0 } catch { "oops" }
# << Look Ma, no red (non-terminating) error text
oops
Or if you prefer:
PS> try { Test-Error -Error -Exception } catch {}; Get-Date
Test-Error : Test Error
At line:1 char:7
+ try { Test-Error -Error -Exception } catch {}; Get-Date
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : SecurityError: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Test-Error
Wednesday, August 22, 2018 8:53:45 AM
@SQLDBAWithABeard then you should be disappointed in a lot of PowerShell Get- commands: Get-Variable, Get-Process, Get-PSDrive, Get-PSRepository, Get-PackageSource, Get-PackageProvider, Get-DscResource, etc, etc, etc.
I'm on board with what @Jaykul wrote up top. Tweet-sized:
I much prefer no error. Testing for absence is incredibly easy, can be used in various logic statements, is the same for every command. Looking for a specific error type indicating something isn't found? Ugly af, and IMHO goes against the tenets of PowerShell
The existing commands that follow the error-if-nothing-found convention are painful to deal with.
I don't understand why this is even up for debate. Do folks seriously want to create a common convention forcing everyone who writes PowerShell to handle multiple error types (which differ for each command, generally) for something that should (and often is, currently) as simple as testing for presence of a value? I realize some folks here are very experienced, but this seems like a bad idea that would affect the usability and desirability of the language for a large swath of folks, including myself.
I wonder if it is just the "sea of red" that so many folks have an aversion to that they're willing to not have commands error just to avoid it. I can certainly sympathize. We've tried in the past to argue for more succinct error msgs. I would love to see something like this:
PS> try { Test-Error -Error -Exception } catch {}
Test-Error : Test Error
For more details on this error run: $error[0]
<optional error hook to allow an extensible error message hint capability>
What I don't like about that is that the $error collection is a stack and that error index above of 0 is not stable. I'd like to see something more like this:
PS> try { Test-Error -Error -Exception } catch {}
Test-Error : Test Error
For more details on this error run: Get-Error 8c1c74a
<optional error hook to allow an extensible error message hint capability>
where 8c1c74a uniquely identifies the error record and can be used at any time up until that error record is "aged" out of the $error collection. That would look like:
PS> Get-Error 8c1c74a
Test-Error : Test Error
At line:1 char:7
+ try { Test-Error -Error -Exception } catch {}; Get-Date
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : SecurityError: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Test-Error
Also, with Get-Error, I'd like to see a -Verbose parameter where I could get extended error details - much like the Resolve-Error (in PSCX) command provides:
Get-Error 8c1c74a -Verbose
PSMessageDetails :
Exception : System.Exception: Syntax Problem
TargetObject :
CategoryInfo : OperationStopped: (:) [], Exception
FullyQualifiedErrorId : Syntax Problem
ErrorDetails :
InvocationInfo : System.Management.Automation.InvocationInfo
ScriptStackTrace : at Test-Error, <No file>: line 13
at <ScriptBlock>, <No file>: line 1
PipelineIterationInfo : {}
MyCommand :
BoundParameters : {}
UnboundArguments : {}
ScriptLineNumber : 13
OffsetInLine : 5
HistoryId : -1
ScriptName :
Line : throw ([Exception]::new("Syntax Problem"))
PositionMessage : At line:13 char:5
+ throw ([Exception]::new("Syntax Problem"))
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PSScriptRoot :
PSCommandPath :
InvocationName :
PipelineLength : 0
PipelinePosition : 0
ExpectingInput : False
CommandOrigin : Internal
DisplayScriptPosition :
Exception at nesting level 0 ---------------------------------------------------
Message : Syntax Problem
Data : {}
InnerException :
TargetSite :
StackTrace :
HelpLink :
Source :
HResult : -2146233088
Do folks seriously want to create a common convention forcing everyone who writes PowerShell to handle multiple error types
Yup. Although the "forcing everyone" bit sounds ominous. Did you see that list I posted above? You already have to do this with many Get-* commands. Are you suggesting we should change those and incur breaking changes?
Also folks seem to be only considering the scripting case and not the interactive CLI case. I happen to believe that most folks using a shell expect Get-Item xyzzy to error - just not with a sea of red text. :-) It's been that way on Bash for decades, ditto for CMD.
@rkeithhill or others who are in the it should error when you request a specific item and that item is not found - can you spell out a value proposition here? I'm struggling to see one. bash does this or some-random-command does this is not really a strong value here, IMHO, when this convention adds serious overhead
can you spell out a value proposition here?
Given that the system temp dir is C:\Windows\Temp (and you are running elevated), what do you expect this to return:
PS> gci C:\Windows\Tmp
If this command were to return nothing, I would assume the directory is empty and not that it actually doesn't exist. That's not a good user experience.
@SQLDBAWithABeard then you should be disappointed in a lot of PowerShell Get- commands: Get-Variable, Get-Process, Get-PSDrive, Get-PSRepository, Get-PackageSource, Get-PackageProvider, Get-DscResource, etc, etc, etc.
Yes - when considering this subject.
Although I can work around it when coding or when at the command line I can read it. I have to explain it to many people who are intimidated by the sea of red. I always want PowerShell to be as easy as possible for all people
If there is a chance to influence the choice, my choice is for no error if nothing to return
Given that the system temp dir is C:\Windows\Temp (and you are running elevated), what do you expect this to return:
PS> gci C:\Windows\Tmp If this command were to return nothing, I would assume the directory is empty and not that it actually doesn't exist. That's not a good user experience.
I think this is a poor example @rkeithhill GCI Folderthatdoesntexist is asking for the contents of a folder that doesnt exist so that should give some output
Get-Printer Doesntexist Get-AzureRmVM -Name NotThere -ResourceGroup $RG
Should not error IMHO
This command can be easily made to behave the way you want:
Get-Printer Doesntexist*
BTW I think you guys are tilting at windmills here. On my Windows 10 1803 system in an elevated Windows PowerShell (more commands built-in), I count the following number of get commands:
gcm -verb get | ? {$_.ParameterSets.Parameters.Name -eq 'Name' -and $_.Name -notin "Get-ItemProperty","Get-WebConfigurationProperty","Get-WmiObject"} |
measure
Count : 41
Of those 41, 29 of them error when the specified item is not found:
$error.clear()
gcm -verb get |
? {$_.ParameterSets.Parameters.Name -eq 'Name' -and $_.Name -notin "Get-ItemProperty","Get-WebConfigurationProperty","Get-WmiObject"} |
% {& $_ -Name xyzzy}
$error.Count
That's 71% of the built-in Get commands that do error when the specified item is not found. So even if you don't think the PowerShell Committee is right on this (and yeah, they do have a few years experience), I don't see them changing over to "not finding the specified item should not (non-term) error". That said, I do get @Jaykul perf argument for REST calls. But again, I think for new REST/remote commands like this, you just specify * to tell the command to not write a non-term error if the wildcard specifier results in no output.
Update: the above results are highly dependent on what modules you have loaded. And it also only checks Get commands with a Name parameter. I tried this again with the just following modules loaded:
Microsoft.PowerShell.Management
Microsoft.PowerShell.Security
Microsoft.PowerShell.Utility
Microsoft.WSMan.Management
PackageManagement
PowerShellGet
PrintManagement
PSReadline
PSScheduledJob
PSWorkflow
WebAdministration
And got 41/43 errored which is 95% that error. I'm sure the % will vary depending on the modules you have loaded.
This command can be easily made to behave the way you want:
Get-Printer Doesntexist*
I know that it can. But I am not just speaking for me but for the people who read all the blog posts where this isn't mentioned for all of the commands they use who are confused or have extra work to do when there is an error. Lets make it easier for people
From the original post
So I ask this community: is that right? Should a Get command that takes a literal name always write an error when it does not return anything?
Pretend that PowerShell was green-field, and that whatever you decide here will not break existing code.
If it was green field this would be my view. In the beginning, it was a different view and now the choice has been suggested to have a different view also. I am ok with that, if that is the way the majority want it.
I am not doubting all the years of experience that PowerShell committee have but I would say that there are folk commenting here and elsewhere with plenty of years experience as well and their opinion is welcomed as well
Also - can I mention my Warning suggestion again ? ;-)
something* looks innocuous enough. Using it can be dangerous. How many folks do you think will consider all the implications of a wildcard?
This is the same deal with why we tell people to never, ever use ambiguous parameter names. What if somethingelse is a thing you shouldn't be matching?
You can just is a tempting refrain, but most alternatives I've seen here:
- Hide real errors
- Result in unintended consequences (e.g. above)
- Require obtuse, painful code
What if
somethingelseis a thing you shouldn't be matching?
Fair point. I suppose this would be less than satisfying: Get-Something somethin[g]? :-) I guess there's no getting around some parameters inherently acting as filters.
My expectation and joy of powershell is that Powershell will only error when something is a problem. Powershell has a precedence of great use of errors that help lead scripters to an actionable solution. Not being able to find something does not provide an actionable solution. Null data is not an issue it gives you just as much information about the output as actual data. Embrace null data.
Hi all. I am new PS user, with not much of experience of writing PS code. I've read the thread and I can agree with people saying: I looked for something, that something does not exists, do not return anything.
It seems more natural to me when you go to a store looking for a thing and it's not there (because it's not there, or you have wrong name of the thing). The result would be : not found, rather than error.
Sometimes lack of any message can be confusing as may indicate some other problem.
When we apply analogy to search/not-found there is usually something like: "not found", "not matching criteria", "0 results"
But in many cases it would be Information or Warning, not an Error.
Initially I might want to see the message, rather then nothing, but it might change when I have more experience with PS.
That's from the user perspective. Hope you find it helpful.
Because more cmdlets don't follow the (good, IMO) example of Get-Command, this has become my scripting pattern:
try
{
$result = Get-Something <foo> -EA 0
if( $? )
{
if( $null -ne $result )
{
# yay! I've got data
}
else
{
# the cmdlet says everything is cool, but returned no data
}
else
{
# cmdlet failed
}
}
catch
{
# terminating error
}
Every comment line requires a different set of actions. Don't you think it's a bit ridiculous?
Embrace null data.
I tend to call almost every command with either -ErrorAction Stop or -ErrorAction Ignore. I either care about the error and want to do something when it happens, or I don't care at all because all it really means is that I'm not getting the data that I want and can't do anything about it.
Any time I call a cmdlet that gets something, I have to check to make sure I got a value before continuing anyway. I kind of prefer to not return values over creating errors. I like to see errors as something unexpected. I expect that a get command will either get me something or it will not.
If we are talking green field here, I would require a Test- command for every Get- command that returns an error for no results. I want to be able to avoid errors, not suppress them or be required to handle them. If a Test- command does not make sense in the context of the command, then it does not make sense for the Get- command to generate errors for no results.
I am OK with GCI giving me an error on a missing folder because I can test for it. Only I can decide if a missing folder is an expected or unexpected situation.
I agree with @KevinMarquette on this one.
I also tend to call most commands with -ErrorVariable and -ErrorAction depending on my expected outcome.
But there is the case in point, expected outcome.
Errors belong where unexpected outcomes exist, you can test for the expected and either handle or ignore the unexpected.
GCI throwing an error to me is appropriate. The verb-noun construct is to Get items. If I pass it a nonexistent value I expect an error, and in turn I can handle that how I choose.
Another alternative is If (test-path $path) { gci $path } else { write-host “$path Not Found” }
Or write-host whatever you want here
Random collection of thoughts:
- Please read my reply here. In particular, I don't think we should be talking in terms of "always" or "never". There are good reasons why we have rules of thumb, and sometimes we have good reasons to break those rules of thumb.
- @SQLDBAWithABeard: warnings are hard to handle non-interactively, and we don't want users depending on strings. Error objects allow script authors to handle error behavior deterministically and we're held to not-breaking that behavior over time.
- We don't want to break existing scripts today, hence the revert. While I understand the desire to have a "what if this was greenfield" argument, that ultimately leads us towards an academic discussion that has little bearing on the real-world end result (which, in this case, is basically "leave our inconsistencies so that we're consistent with the past").
- In general, I'm not super concerned about "painting the screen red" as long as we don't halt execution. That being said, I'm open to hearing about more scenarios where non-terminating errors are more painful than I'm understanding.
- Agreed, our error output should be better.
Joey,
Re you're being "open to hearing about more scenarios where non-terminating errors are...painful...."
At my level, much of my code is within try/catch blocks with ErrorActionPreference set to Stop, so non-terminating errors don't exist. Adding complexity to the error handling to handle "expected" errors different from actual errors is painful. It is much easier to distinguish between an error and a null result if one of them is an error and the other is a null result, instead of them being two different types of errors, terminating or not.
And at the level of the junior scripters that make up the bulks of the community, any unnecessary complexity is painful, slows adoption, and steepens the learning curve. It should be possible to add complexity to do complex things, but default behavior should make it simple to do simple things.
@joeyaiello non-terminating errors are one of the most painful things in PowerShell, because literally at any time a caller of my code may change the rules by just calling my function with -ErrorAction Stop and suddenly they all turn into terminating errors ...