Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🩹 [Patch]: Remove unnecessary try..catch blocks #289

Closed
vercellone opened this issue Jan 31, 2025 · 2 comments · Fixed by #281
Closed

🩹 [Patch]: Remove unnecessary try..catch blocks #289

vercellone opened this issue Jan 31, 2025 · 2 comments · Fixed by #281
Assignees

Comments

@vercellone
Copy link

Describe the change

I'm seeing this anti-pattern a lot in your code.

try{
    # Do something
} catch {
    throw $_
}

If you aren't going to do anything with the caught exception there is no benefit to catching it. It adds unnecessary runtime overhead, and adversely affects readability.

Also, if you are going to re-throw the original exception it is better to just throw. throw $_ adds unnecessary overhead.

@MariusStorhaug
Copy link
Member

Feel free to suggest a good way to implement error reporting on the top level functions!
They are "placeholders" until I have gone through some reading on how to handle it using error records.

@vercellone
Copy link
Author

This is a heavily debated topic.

My 2 cents: Less is more. I have tried a few approaches to capturing and repackaging errors and none have proven as resilient and pain free as avoiding try..catch blocks in most public functions.

When I use a try..catch block, it is usually in a common function (like Invoke-GitHubAPI) and generally looks like this:

try {
    # Do something
}
catch {
     # Optionally log the Exception.  DO NOT alter it but DO add more details if useful.
     if ([something I can ignore or work around]) {
         # Do that
     }
     else {
         throw # re-throw the original exception
     }
}

I also often use try..finally (with no catch).

I fully advocate the notion of leading people to the pit of success.
But when it comes to exceptions (especially those involving a PowerShell module that wraps HTTP requests) it is frustrating when the error is in any way altered or obfuscated from that thrown by the underlying API. Depending on your implementation it can also hinder end users' ability to use -ErrorAction/$ErrorActionPreference effectively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants