Return Statement not Working with continue

Question

Update

I was able to resolve this by changing Rename-Item to Move-Item and just changing the name in the process. However, this does not solve the mystery of why the return statement was not executed, or at least if it was, why the function continued to call New-Item.

I am trying to rename an item at logon, but if the item doesn't exist, I want to create it. Here is the function which does this:

function SelfHelpAppData {
    $ErrorActionPreference = "Stop"
    trap {Log-Error $_ $MyInvocation.MyCommand; Continue}

    $files = Get-ChildItem $AppData

    ForEach ($file in $files) {
        If ($file.Name -match 'qxjz') {

            Rename-Item $file.PSPath "qxjz$env:COMPUTERNAME.txt" -Force
            WriteLogonEntryData
            return
        }
    }

    New-Item "$AppData\qxjz$env:COMPUTERNAME.txt"
    WriteLogonEntryData
}

However, when this runs, I am receiving these errors in my log:

Windows PowerShell is in NonInteractive mode. Read and Prompt functionality is not available.

At \DC1\NETLOGON\PSSubs\mainlogon.ps1:841 char:5

New-Item "$AppData\qxjz$env:COMPUTERNAME.txt"

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And then:

Cannot create a file when that file already exists.

At \DC1\NETLOGON\PSSubs\mainlogon.ps1:835 char:13

        Rename-Item $file.PSPath "qxjz$env:COMPUTERNAME.txt" -Force

        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

But this is impossible, as if the first line (rename-item) attempted to run, then the function should have returned before the other line had a chance. I have tried changing the return statement to return $null but with no effect. Does anyone know what is going on here?

Here is the Log-Error code:

function Log-Error {
    param (
        $error,
        [string]$sub
    )
    $ErrorActionPreference = "Stop"
    trap {Log-Error $_ $MyInvocation.MyCommand; Continue}

    $filename = "\\file\administration\Unused\LogonScriptErrors\$env:USERNAME - $env:COMPUTERNAME - $(Get-Date -Format ddMMyyyy-HHmmss) - $($error.InvocationInfo.ScriptLineNumber).log"

    New-Item $filename -ItemType File -Value "$($error.Exception.Message) `r`n `r`n $($error.InvocationInfo.PositionMessage) `r`n `r`n $sub"
}

score 2 · Accepted Answer · answered Apr 04 '16 at 15:27

The reason for the behavior you observed is the continue in your trap statement list. It causes the script to continue with the next instruction after the loop where the error occurred. This is documented behavior.

If you want the script to terminate operation in case of an error remove the continue. If you want it just exit from the function change continue to return.

With that said, I'd recommend to move away from trap and use try/catch instead, which offers better control, even visually.

BTW, the New-Item error occurs, because you omitted the mandatory parameter -Type. Without that parameter the cmdlet tries to prompt you for the required information, but can't because the script is run non-interactively. To get rid of the error add -Type File to the statement.

beat me to it! Hopefully my example fits the situation appropriately. — Chris Kuperstein, Apr 04 '16 at 15:51
@AnsgarWiechers Thanks! I changed the traps inside my functions to `Return`, and kept the ones which were outside in the main body of the script as `Continue`. I find `trap` to look a lot cleaner, but you're right using `try/catch` would give me more control — Bassie, Apr 05 '16 at 11:51

score 1 · Answer 2 · answered Apr 04 '16 at 15:50

The PS error trap is being thrown every time because Error Action handling in powershell is a lot more "loose" than traditional Exception handling. Some revisions I made:

Instead of doing a global blanket trap, use try/catch.
Use -Force on both your new-item and rename-item statements.
You forgot the $env: in $env:AppData
You can get around the console verbage with | Out-Null

For the most part, doing individual try/catch error handling is tedious and ugly, which is probably why you went with trap{}. You can just do general exception handling with match cases instead if you really need that level of error handling with simple rename/create file operations.

function Do-Stuff {

    $EAPREF = 'Stop'
    $FILES = gci $env:APPDATA

    try {
        ForEach ($file in $FILES) {
            If ($file.Name -match 'qxjz') {
            Rename-Item $file.PSPath "qxjz$env:COMPUTERNAME.txt" -Force -ErrorAction $EAPREF | Out-Null
            return
            }
        }
        New-Item "$env:AppData\qxjz$env:COMPUTERNAME.txt" -ItemType 'File'  -Force -ErrorAction $EAPREF | Out-Null
    } 
    catch [System.Exception]{
        #Error handling here using type/parameter qualifier cases.

        if ($_.FullName -eq 'So and So'){
            # Handle the error a certain way for so-and-so
        }

        if ($_.FullName -eq 'Other exception type you expect'){
            # Handle another type of error separately
        }
    }
}

Yes that is exactly why I went with trap - it looks so much better. As this is for a logon script I just need to log every error/exception that I can in a file on the network. I'm not sure if there is a chance that using try/catch as you mentioned means I could potentially miss some (by not specifying all the possible `.fullnames`) but that was my thinking initially! Also, as every error is handle in the exact same way (write to a file) it seemed unnecessary to add loads of checks! — Bassie, Apr 05 '16 at 11:54

Return Statement not Working with continue

2 Answers2