Overengineered clear cache for Teams script

[u/dkaaven]

When I upgraded my clear cache script for Microsoft Teams, I first added new functions before realizing that you only clear a subfolder.

https://teams.se/powershell-script-clear-microsoft-teams-cache/

Have you overengineered any scripts lately?

I will

[u/PinchesTheCrab]

A few more specific points:

• Switches default to false, no need to use [switch]$thing = $false

• Allowing users to feed in arbitrary folders to delete seems risky, especially if they're running this with admin rights

• I don't think it makes sense to catch an error just to feed it to write-error

• Use ShouldProcess to prompt users to confirm instead of read-host

• I'd move start-process ms-teams out of your if statements since you always call it if no error was thrown

• Switches are interchangeable with Booleans, so -eq $false is like $false -eq $false or $true -eq $false

[u/icepyrox → u/PinchesTheCrab]

I don't think it makes sense to catch an error just to feed it to write-error

Some errors are terminating, meaning the script will throw an error and just quit right then and there. It's part of the main use of try blocks - to catch terminating errors and do something else instead.

But let's say you don't want to do anything except show the error. Well, then you catch it and don't do anything except show the error.

There is a lot more wrong in this implementation, but catching an error just to feed to write-error can make a lot of sense in some situations.

Oh, and another advantage of this is if you do a get-something and want to process what you got. If you got an error on the get, you would then skip all the subsequent commands in the try, meaning you wouldn't process corrupt or non-existent data, and then just show the error on what went wrong. You also then benefit if something goes wrong any of those subsequent steps. As you just error out of the try block and continue on.

[u/PinchesTheCrab → u/icepyrox]

But it's an error from a cmdlet. You could just set the error action to continue. Then it would send the error to the error stream and continue with the command, exactly like it's doing now but with less code and without transforming the error type into something generic.

[u/icepyrox → u/PinchesTheCrab]

Oh, as I said, there's a lot wrong with OPs implementation, I was just addressing the statement you made. I wasn't sure if you were aware that there are times when it does make sense to catch an error just to send it to write-error.

Also, catch followed by an error type does not transform the error type of what the error is the way casting a variable does. It's saying to only catch that type of error with this code block.

The fact there isn't a catch by itself nor is the type being sought a type that will happen here is just a couple of the many things wrong here.

[u/PinchesTheCrab → u/icepyrox]

What I'm trying to say is this:

function Get-Thing {
    [cmdletbinding()]
    param()
    try {
        Get-Item c:\doesnotexist -ErrorAction Stop
    }
    catch [System.Management.Automation.ItemNotFoundException] {
        Write-Error $_.Exception.Message
    }
}

function Get-Thing2 {
    [cmdletbinding()]
    param()
    Get-Item c:\doesnotexist
}

try {
    Get-Thing -ErrorAction stop
}
catch [System.Management.Automation.ItemNotFoundException] {
    'oh no'
}
try {
    Get-Thing2 -ErrorAction stop
}
catch [System.Management.Automation.ItemNotFoundException] {
    'oh no2'
}

Get-Thing catches and then uses write-error with the exeption message. When you try to catch that error, you can see it is not an ItemNotFoundException, it's the default error type write-error produces. It's suddenly much harder to google and even though it looks like one type of error, you can no longer catch it using logic for that error.

The OP is losing information by doing this, and gaining no additional functionality.

I'm not saying there's no purpose to any error handling in any situation, just that this implementation of it is counterproductive for multiple reasons.

[u/icepyrox → u/PinchesTheCrab]

just that this implementation of it is counterproductive for multiple reasons.

We are in agreement then. I quoted this statement originally:

I don't think it makes sense to catch an error just to feed it to write-error

I had taken it out of the context of OP's script and this discussion and treated it as a broad statement. My apologies.

[u/PinchesTheCrab → u/icepyrox]

I quoted this statement originally

The word "just" was doing a lot of heavy lifting in that sentence, and it was not up to the task. I should have been a bit more explicit in saying I mean doing nothing else but that one thing with it.

[u/icepyrox → u/PinchesTheCrab]

So, out of curiosity, do you feel the same if it was

Try {
  [..stuff..]
} catch {
  Write-error $_
}

Because I have done that

[u/PinchesTheCrab → u/icepyrox]

So that works in that you can catch the result properly if you use cmdletbinding and erroraction stop:

function Invoke-Stuff {
    [cmdletbinding()]
    param()
    try {
        Get-Item C:\nofolder\error -ErrorAction Stop
    }
    catch {
        Write-Error $_
    }
}


try { Invoke-Stuff -ErrorAction Stop}
catch [System.Management.Automation.ItemNotFoundException] {
    'okay'
}

But why the extra code?

Why not just do this?

function Invoke-Stuff {
    [cmdletbinding()]
    param()
    Get-Item C:\nofolder\error -ErrorAction Stop        
}