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 Is there actually a best way to handle errors? #37 (comment))
  • 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.