Is there actually a best way to handle errors?

[Jaykul]

https://github.com/PoshCode/PowerShellPracticeAndStyle/blob/master/Best%20Practices/err-05-avoid-testing-for-a-null-variable-as-an-error-condition.md

First of all, let's not have a misleading discussion. The original guideline uses a bad example. Get-ADUser throws an exception when you use the -Identity parameter and it can't find anything. There's obviously no value in writing a warning if you didn't suppress the error, and it's equally silly to let an exception spew and then still test for null -- you need to either suppress the error or use it instead of testing for null.

Let's try a different example. I have a module which requires the user to configure multiple connection strings and a license before it can continue. Imagine that it was this simple to verify that you had, in fact, configured it:

$Config = Get-Content $ConfigurationFile

This will cause an error and an empty variable if the ConfigurationFile doesn't exist, but does not throw (it's not terminating) by default. There are a few ways I could handle that in PowerShell.

For what it's worth: the pythonic way is always to just charge ahead, never test, and just deal with the exceptions (or let them output, if they're clear enough on their own). The old C/C++ way is to always check the hResult, there are no exceptions. The Java and C# way usually involve handling exceptions, but not quite as aggressively as Python, you would rarely force an exception to happen if you could avoid it by a quick test.

So which of these should be recommended? Incidentally, for the sake of this discussion, please imagine my throw statement to be your preferred way of exiting a function with extreme prejudice.

Avoid the error:

if(Test-Path $ConfigurationFile) {
    $Config = Get-Content $ConfigurationFile
} else {
    # We could write a warning and return, but for consistency:
    throw "You must configure MyModule first, please call Initialize-MyModule"
}
<# Do the work #>

Of course, if it's best practice to avoid errors when possible, you still have to have a best practice for dealing with them, because it's not always as easy as Test-Path to avoid them.

Suppress the error and check the output:

if($Config = Get-Content $ConfigurationFile -ErrorAction SilentlyContinue) {
    <# Do the work #>
}
# We could have just done ... nothing, but for consistency:
throw "You have to configure MyModule first, please call Initialize-MyModule"

Or I could write that backwards:

if(!($Config = Get-Content $ConfigurationFile -ErrorAction SilentlyContinue)) {
    throw "You have to configure MyModule first, please call Initialize-MyModule"
}
<# Do the work #>

Force an exception and catch it

try {
    $Config = Get-Content $ConfigurationFile -ErrorAction Stop
} catch { 
    # Normally you'd be using some information from the exception, but for consistency
    throw "You have to configure MyModule first, please call Initialize-MyModule"
}
<# Do the work #>

Deal with the error itself

$Config = Get-Content $ConfigurationFile -ErrorAction SilentlyContinue -ErrorVariable NoConfig

if($NoConfig) {
    # Normally you'd be using some information from the error, but for consistency
    throw "You should configure MyModule first, please call Initialize-MyModule"
}
<# Do the work #>

[KirkMunro]

You picked a good example Joel, because the errors that may be thrown from this example differ if $configurationFile is $null than if $configurationFile is a file that does not exist. If the file does not exist, -ErrorAction can be used to control how the cmdlet throws errors, however if $configurationFile is $null, -ErrorAction has no effect.

The only best practice I can recommend when it comes to handle errors in PowerShell scripts, functions, script modules, etc. in a consistent way is as follows:

Try/catch everything (or alternatively use the trap statement)

Reasoning: if you create a script or function or script module that does not wrap the contents in try-catch (or that does not use trap), then how that script/function/script module works is dependent on how it is called. If it is called from inside of a try/catch block, then terminating errors are actually treated as terminating errors. Otherwise, the script/function/script module will continue to execute after an error that would otherwise be terminating is raised.

Details: In general, I use trap in script modules (Joel's suggestion IIRC) to avoid unnecessary indentation where there is none, and try/catch everywhere else. This includes each begin, process, and end block.

In advanced functions, inside of the catch block, invoke $PSCmdlet.ThrowTerminatingError($_)

Like this:

function Test-Something {
    [CmdletBinding()]
    param()
    try {
        # ...
    } catch {
        $PSCmdlet.ThrowTerminatingError($_)
    }
}

Reasoning: This format allows PowerShell to display the error details along with the command name just as it would if the error came from a native cmdlet. The format is much easier for function consumers, it is more consistent, and it doesn't get them confused with the details about the innards of the function. If you were to simply invoke throw instead, then the line of script that would be displayed to the caller as the source of the error would be the throw statement itself rather than the line of script that was used to invoke the function when the error occurred. These details shouldn't be exposed to the caller when an error occurs as it will only contribute to confusing them.

In Script Modules, include a trap statement at the top of the script module that simply rethrows any errors that occur.

For example:

trap {throw}

Reasoning: This allows a script module to properly fail to load if a terminating error is raised. Without this, script modules may "load" in some unknown/unsupported state even if an error occurs during the loading process.

With all of that in mind, I vote strongly for the Force an exception and catch it approach as the best practice, but with the additional details above that describe how to handle the exception properly (using trap for script modules, try/catch with $PSCmdlet.ThrowTerminatingError($_) for advanced functions, and try/catch{throw} for anything else.

Also, with respect to throw, there are ways to throw ErrorRecord instances that give the caller much better details than they will get from a throw statement that simply throws a string. I have plenty of examples of those, and can dig some up it you want to see it (maybe next week after I'm back home).

[indented-automation]

I much prefer Force an exception and catch it using $pscmdlet.ThrowTerminatingError, but it's perhaps worth noting that throw is not limited to a string.

$errorRecord = New-Object System.Management.Automation.ErrorRecord(
  (New-Object Exception "A generic exception to throw"),
  'My.ID',
  [System.Management.Automation.ErrorCategory]::OperationStopped,
  $Object
)
throw $errorRecord

Chris

[KevinMarquette]

As an alternate to manually creating an error record, we can just use Write-Error to create (and throw it)

Write-Error -Exception (New-Object Exception "A generic exception to throw") -ErrorAction Stop

Instead of using $pscmdlet.ThrowTerminatingError($_) to re-throw, we can also use Write-Error

catch
{
    Write-Error -Exception $PSItem -ErrorAction Stop
}

[christianacca]

How about this:

function Set-Something {
    [CmdletBinding()]
    param()

    try {
        # trigger error
        1 / (1 - 1)
                    
        # this line will NEVER run regardless of caller's preferences or try..catch
        Write-Host 'still running inside Set-Something!'
    }
    catch {
        $errPref = 'Stop'
        if ( $PSBoundParameters.ContainsKey('ErrorAction')) {
            $errPref = $PSBoundParameters['ErrorAction']
        }
        # raise error according to callers preference but hide internal details of error
        Write-Error -ErrorRecord $_ -EA $errPref
    }
}

[KevinMarquette]

I have settled on using this for all public interfaces or functions:

function set-something
{
    [cmdletbinding()]
    param()
    begin
    {
        try
        {
            #...
        }
        catch
        {
            $PSCmdlet.ThrowTerminatingError($PSItem)
        }
    }
    process
    {
        try
        {
            #...
        }
        catch
        {
            $PSCmdlet.ThrowTerminatingError($PSItem)
        }
    }
    end
    {
        try
        {
            #...
        }
        catch
        {
            $PSCmdlet.ThrowTerminatingError($PSItem)
        }
    }
}

I do this because some code executes differently when it is inside a try/catch than it does when outside a try/catch. There is a small set of errors that are terminating in one case and non-terminating in the other. Ensuring all your code executes inside a try/catch solves that and makes everything consistent.

By using the template above for your public functions, it will give the users of your script the correct error message and the choice to either handle it as a terminating error or not. This also lets you throw errors or use Write-Error -ErrorAction Stop internally if that is what you would prefer.

After really digging into this, the PowerShell way is to not allow your function to terminate the calling script. It should give the error and the caller should move onto the next command. Forcing a stop or a throw from within your module to the caller's script is bad form and should be avoided. The caller of your function should decide based on the -ErrorAction if it should terminate their script or not.

This is exactly what $PSCmdlet.ThrowTerminatingError($PSItem) allows. It terminates your function and gracefully hands the decision over to the caller of your function on how to handle it.

Edit: This is counter to the recommendation that I mentioned in a previous comment because I have spent more time working with this and I have adjusted my opinion on the matter.

[christianacca]

Hi Kevin,

What I don't like about using $PSCmdlet.ThrowTerminatingError($PSItem) as you suggest - it seems to make -ErrorAction 'Stop' not behave as I would expect.

Take the following function:

function Set-Something {
    [CmdletBinding()]
    param()
    process {
        try {
            Get-LocalUser 'nope' -EA Stop                        
            Write-Host 'still running inside Set-Something!'
        }
        catch {
            $PSCmdlet.ThrowTerminatingError($_)
        }
    }
}

And the calling code:

Set-Something -EA 'Stop'; Write-Host 'mmm... should not reach here!'

Running the code above I see on the console:

Set-Something : User nope was not found.
At line:1 char:1
+ Set-Something -EA 'Stop'; Write-Host 'mmm... should not reach here!'
+ ~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (nope:String) [Set-Something], UserNotFoundException
    + FullyQualifiedErrorId : UserNotFound,Set-Something

mmm... should not reach here!

In other words, specifying -EA 'Stop' does not result in the calling code terminating. Instead powershell continues to execute the remaining script thus writing mmm... should not reach here! to the console

Contrast this with calling code that executes Get-LocalUser directly:

Get-LocalUser 'nope' -EA Stop; Write-Host 'mmm... should not reach here!'

Powershell immediately halts and does not run Write-Host 'mmm... should not reach here!'

[christianacca]

I think my original template can be improved and simplified:

function Set-Something {
    [CmdletBinding()]
    param()

    process {
        $callerEA = $ErrorActionPreference
        try {
            
            # functions work goes here
            
        }
        catch {
            Write-Error -ErrorRecord $_ -EA $callerEA
        }
    }
}

[cspotcode]

@christianacca Can I tweak your template to use a helper function and reduce duplication? I think this also allows specifying a module-local $ErrorActionPreference that applies to everything that happens within my module's advanced functions without affecting the errors returned by write-error.

function fnwrap($block) {
    $private:callerEA = $ErrorActionPreference
    $ErrorActionPreference = 'Stop'
    try {
        & $block
    }
    catch {
        $MyInvocation = (Get-Variable -Scope 1 MyInvocation).Value
        Write-Error -ErrorRecord $_ -EA $callerEA
    }
}

function Set-Something {
    [CmdletBinding()]
    param()

    process {
        fnwrap {
            # functions work goes here
            # ErrorAction default is 'Stop' thanks to fnwrap
        }
    }
}

EDIT: I updated the code to fix a problem where errors all reported they came from the fnwrap helper. The solution is to pass the parent scope's $MyInvocation to Write-Error.

[Jaykul]

That won't work the same at all, @cspotcode -- your ErrorRecord will be confusing, and you're turning a process block (which runs many times) into invocation of a scriptblock, which means you'll loose variable scope, persistence, etc.

[christianacca]

As there's a bit more interesting conversation on this one, I thought I'd throw out what I've been using in a bunch of modules I've written lately.

function Set-Something {
    [CmdletBinding()]
    param ()
    
    begin {
        Set-StrictMode -Version 'Latest'
        Get-CallerPreference -Cmdlet $PSCmdlet -SessionState $ExecutionContext.SessionState
        $callerEA = $ErrorActionPreference
        $ErrorActionPreference = 'Stop'
    }
    
    process {
        try {
            # body of function goes here
            
        }
        catch {
            Write-Error -ErrorRecord $_ -EA $callerEA
        }
    }
}

Additions to what I posted before:

• turn on strict mode
• default error behaviour is to stop on all errors (edit: within the function body)
• ensure that default caller's preferences are honored (edit: when rethrowing the exception)

That last point about caller's pref's bit me. I ended up using PreferenceVariables module to solve the problem... see this blog post for more details

I'm not especially happy with the amount of boiler plate code, but I find I'm anesthetized to it now!

[Jaykul]

It almost feels like we need a "StrictMode" version of error handling -- but what StrictMode actually does does not seem useful at run time (we've got another thread about that, so I won't go further off topic here) 😉

@christianacca two questions:

1. Why do you feel it's appropriate to read the ErrorActionPreference from your caller (or rather, from your SessionState), rather than behaving the normal way?
2. Do you never encounter situations where you catch an actual exception, instead of an ErrorRecord?

[christianacca]

It might help the conversation by giving an example of the code I would write in the body of the function:

function Set-Something {
    [CmdletBinding()]
    param (
        [string] $Value
    )
    
    begin {
        Set-StrictMode -Version 'Latest'
        Get-CallerPreference -Cmdlet $PSCmdlet -SessionState $ExecutionContext.SessionState
        $callerEA = $ErrorActionPreference
        $ErrorActionPreference = 'Stop'
    }
    
    process {
        try {

            # Start: function body

            if ([string]::IsNullOrWhiteSpace($Value)) {
                throw 'Bad input'
            }
            
            $finalValue = try {
                Get-Something $Value
            }
            catch {
                Get-SomthingElse $Value
            }

            Set-Stuff $finalValue
            Set-LogStuff $finalValue -EA SilentlyContinue


            # End: function body
            
        }
        catch {
            Write-Error -ErrorRecord $_ -EA $callerEA
        }
    }
}

Let me explain my thinking before I get to (hopefully!) answer your question...

As author of Set-Something, here's the semantic I want to achieve:

• When it comes to error behavior, the caller should NEVER influence code within the function body:
  • EG: I NEVER want the caller's preference to allow the function body to continue executing if Set-Stuff throws
• By default I ALWAYS want errors to be treated as terminal within my function body - that way I always get a single consistent method for error handling ie structured error handling via try/catch blocks
  • note: this semantic must also extend to calls out to other functions eg the call to Get-Something
• The caller of Set-Something get's to decide whether an error that is about to be "rethrow" (via Write-Error) should allow the caller's code to continue
• The caller doesn't have to be concerned with terminal vs non-terminal errors:
  • the caller always receives a terminal error (see https://github.com/PoshCode/PowerShellPracticeAndStyle/issues/37#issuecomment-338171370)
  • The calling code can be written either using a try/catch or to set an ErrorPreference/ErrorAction that allows execution to continue

So hopefully I can wind back to answering your questions

Why do you feel it's appropriate to read the ErrorActionPreference from your caller, rather than behaving the normal way

All I want from the caller is his preference for whether the error about to be rethrown via Write-Error should cause the caller's code to stop or continue. I achieve that by:

$callerEA = $ErrorActionPreference
# ... snip
Write-Error -ErrorRecord $_ -EA $callerEA

My function actually disregard the caller's preference within the function body by immediately assigning $ErrorActionPreference = 'Stop' before any other code in the function runs

Do you never encounter situations where you catch an actual exception, instead of an ErrorRecord

On testing, I still get to catch exceptions. Consider the following:

try {
    Get-Stuff '?' -EA Stop
}
catch [MyException] {
    Write-Host 'Logic to handle MyException'
}

Get-Stuff is written using using my standard template. It throws a custom exception... caller get's to catch it (FYI, you can see the code in action here)

[KirkMunro]

It almost feels like we need a "StrictMode" version of error handling.

@Jaykul I was thinking the same thing, and have been thinking about adding an attribute property to PowerShell Core that can be enabled inside of CmdletBinding to handle all of this automatically.

[christianacca]

@KirkMunro, while you're at it can you add the equivalent of npm shrinkwrap/package-locks to PowerShell Core ;-)

PsDepend get's someway toward package-locks but as per this issue doesn't handle transitive dependencies

PS Sorry for off-topic post :-0

[kborowinski]

@christianacca @Jaykul Is there a way to silently exit Begin block without running Process or End block when user calls following function with -ErrorAction -SilentlyContinue? Let say that I have module that setups connection to remote server in Begin block but when the connection setup fails I do not want Process or End blocks to execute but exit silently and continue execution of caller script.

Function:

function Get-Something {
    [CmdletBinding()]
    param()

    begin {
        try {
            Get-LocalUser 'none' -ErrorAction Stop
        } catch {
            Write-Error -ErrorRecord $_ -ErrorAction $ErrorActionPreference
        }
    }
    process {
        'Should not run'
    }
    end {
        'Should not run as well'
    }
}

Calling code:

Get-Something -ErrorAction SilentlyContinue
"Should run and any errors from Get-Something should be suppressed"

[Jaykul]

@kborowinski You have to set -ErrorAction Stop on your write-error line if you want to stop.

If you refuse to handle your process block, you're stopping the whole pipeline -- there's no way a caller's script could reasonably continue unless they don't need your output (which means the right thing to do is throw a terminating exception -- you're terminating the pipeline you are in)

[kborowinski]

@Jaykul Exactly, in this case I don't care about Get-Something output (it just best-effort function). The reason I want to fail early (exit in begin block) is that I don't want to waste time on processing input from pipeline (as it can be hundreds of objects) just to find in the end block that connection was unsuccessful. So I'll set -ErrorAction Stop on Write-Error and then suppress any errors in caller script with try-catch. Thanks!

[copdips]

That won't work the same at all, @cspotcode -- your ErrorRecord will be confusing, and you're turning a process block (which runs many times) into invocation of a scriptblock, which means you'll loose variable scope, persistence, etc.

@Jaykul Maybe you're right for @cspotcode's given example, but his idea of using a helper function to wrap all of this is really interesting at least to me. We should perhaps work more in this helper function to keep what you want (variable scope, persistence).

[cspotcode]

I have an updated version that uses some careful variable manipulation and the dot operator to "do the right thing" and avoid the pitfalls described above. I'll share it when I'm at my computer.

On Sat, Sep 22, 2018, 6:40 PM Xiang ZHU [email protected] wrote:

That won't work the same at all, @cspotcode https://github.com/cspotcode -- your ErrorRecord will be confusing, and you're turning a process block (which runs many times) into invocation of a scriptblock, which means you'll loose variable scope, persistence, etc.

@Jaykul https://github.com/Jaykul Maybe you're right for @cspotcode https://github.com/cspotcode's given example, but his idea of using a helper function to wrap all of this is really interesting at least to me. We should perhaps work more in this helper function to keep what you want (variable scope, persistence).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PoshCode/PowerShellPracticeAndStyle/issues/37#issuecomment-423778435, or mute the thread https://github.com/notifications/unsubscribe-auth/AAW-uCiwW7FpdrTfctdZfymiTTvt-Gbvks5udrxrgaJpZM4FBLpa .

[Jaykul]

@copdips the resulting code would be extremely convoluted: hard to read and hard to debug. You would need to dot-source the wrapper, and then the wrapper would need to dot-source the scriptblock. Then, because you're forcing all errors to be terminating, you have to unwrap every error, and then wrap them in a new error, in order to make the error LOOK like it actually came from the function that the user called, instead of your private wrapper function.

It's far simpler to just use a snippet to generate your function with all that ceremony, like what @KevinMarquette wrote, assuming you play with that and feel like it behaves the way you expect it to (I'm not totally sold yet).

Or maybe we could get it implemented as an overload for CmdletBinding within PowerShell itself, as @KirkMunro suggested ;-)

[erikgraa]

I've adopted @christianacca's solution to most of my code. Thanks. :)

Any thoughts on how to best throw inside a ValidateScript()-attribute of a parameter, e.g. if I were to check whether a string is a valid domain?

[Parameter(Mandatory=$false)]
[ValidateScript({
	try {
		Get-ADDomain -Identity $_
		$true
	}
	catch {
		Write-Error -ErrorRecord $_ -ErrorAction Stop ?
		throw $_ ?
	}
})]
[String]$Domain

Perhaps one should do this instead:

[Parameter(Mandatory=$false)]
[ValidateScript({
	try {
		if (-not(Get-ADDomain -Identity $_ -ErrorAction SilentlyContinue)) {
			throw "$_ is not a valid domain"
		}
	
		$true
	}
	catch {
		Write-Error -ErrorRecord $_ -ErrorAction Stop
	}
})]
[String]$Domain

[ChrisLGardner]

I break out the validation into a dedicated function and then just call that from the ValidateScript so it's more readable. The function then Throws a readable error message if it fails the validation.

[erikgraa]

Can you provide an example @ChrisLGardner? Where do you keep the dedicated function(s)? Can one do the argument validation in the begin block instead?

[ChrisLGardner]

@erikgraa I use private functions as the user never needs to call them. You can do the validation in the begin block but that's got trade offs to it (the other begin blocks in the pipeline will also trigger if this isn't the first command in the pipeline).

An example of one of my functions is:

Function Test-VmcPathValidation {
    [cmdletbinding()]
    [OutputType([System.Boolean])]
    param (
        [Parameter(Mandatory)]
        [string]$Path,

        [string]$ParameterName
    )

    if (Test-Path -Path $Path) {
        $true
    }
    else {
        Throw "Path provided for '$ParameterName' does not exist at '$Path'"
    }
}

[erikgraa]

Thanks! So you invoke Test-VmcPathValidation like this?

[Para...]
[ValidateScript({Test-VmcPathValidation})]
[..]$Path

Is it preferred to use $PSCmdlet.ThrowTerminatingError($_) / throw in the ValidateScript-attribute instead of a Write-Error?

[ChrisLGardner]

Either should work fine but you'll need -EA Stop. I'm just using Throw since the code is a little old and I haven't converted to using $PSCmdlet.ThrowTerminatingError() yet.

[Jaykul]

It might be worth pointing out there's a conversation about implementing some of this as a new keyword in PowerShell Core https://github.com/PowerShell/PowerShell/issues/8819

[JustinGrote]

I made a simple private function called ThrowUser that works just like throw but in a powershell module shows the "outer" stacktrace, so it's more user friendly for a user to see where he screwed up in his code.