Docs: "Use try/catch/finally blocks around code that can potentially generate an exception" - bad advice

[arnonax]

IMHO the first advice that is mentioned: "Use try/catch/finally blocks around code that can potentially generate an exception" is a bad one. Instead, I would say: "Don't catch exceptions if you don't have something useful to do with them".
One of the most common problems that I see in junior's code is that they wrap everything with try/catch because they're "afraid" that exceptions will be thrown from their code. This often makes them swallow exceptions. Even if they add a log entry in the catch block, but don't re-throw it, then practically it's pretty much the same as swallowing it. And even if they rethrow it, they either don't do it correctly (using ExceptionDispatchInfo, or at least throw;), or it's just redundant and makes the code more cumbersome. After all, if there's really an unexpected exception being thrown, then you would probably want the application to crash and display the entire exception details, which is what happens anyway.

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: b1ea31f6-516f-100d-046e-1a0445d92824
• Version Independent ID: a713e8c9-8210-761d-9ae7-238793825ff3
• Content: Best Practices for Exceptions
• Content Source: docs/standard/exceptions/best-practices-for-exceptions.md
• Product: dotnet
• GitHub Login: @mairaw
• Microsoft Alias: mairaw

[mgregory22]

I would say it is incomplete advice rather than bad. You are supposed to catch exceptions and handle them in a way that cleans up inconsistent state and makes it easy for the developer or end user to troubleshoot the error. E.g. an error message that provides relevant details about what could have happened, printing the value of a bad parameter in addition to the call stack, or maybe hiding the details for the end user and telling them to call support. You could write an epistle on this topic.

[arnonax]

That's better, though typically mostly cleanup should be done in a finally block rather than in the catch block.
All other cases and examples you gave fall into my proposed advise: "only if you have something useful to do with that". What I'm saying is that the way the advice is given in the article implies that you should always catch every exception (e.g. in every method), and IMHO that's a common mistake that novice developers make. As this article is about best practices for using exceptions, I think that it's important to stress that.
FWIW I encountered this article and wrote this comment after a student of mine asked me if I can give him some best practices for using exceptions. I could answer him directly, but instead I thought to direct him to existing articles about this topic. But when I read this article I was disappointed to see this advice and decided not to direct him to this article because of that.

בתאריך יום ו׳, 31 באוג׳ 2018, 19:10, מאת Matt Gregory ‏<[email protected][e-mail]>:

I would say it is incomplete advice rather than bad. You are supposed to catch exceptions and handle them in a way that cleans up inconsistent state and makes it easy for the developer or end user to troubleshoot the error. E.g. an error message that provides relevant details about what could have happened, printing the value of a bad parameter in addition to the call stack, or maybe hiding the details for the end user and telling them to call support. You could write an epistle on this topic.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com/dotnet/docs/issues/7327#issuecomment-417713006, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJgGwv2-W2jieKw8vOcRQrGP_Sl1oq3Cks5uWV_ygaJpZM4WPWNb.

[sam-wheat]

The correct advise is "Don't catch exceptions you don't handle." Logging is NOT handling.

I would say it is incomplete advice rather than bad. You are supposed to catch exceptions and handle them in a way that cleans up inconsistent state

This is incorrect. You just encountered an exception. You don't know what's wrong - you don't know anything about state - that's why its an Exception and not a code path you created. The correct advise is to let the Exception flow up to a global handler where it is logged. Stop processing. Fix the error.

[mgregory22]

This is incorrect. You just encountered an exception. You don't know what's wrong - you don't know anything about state - that's why its an Exception and not a code path you created. The correct advise is to let the Exception flow up to a global handler where it is logged. Stop processing. Fix the error.

Sam, that's ridiculous. Why would catch blocks even exist if exceptions couldn't be handled? The fact is, there are lots of situations where you know what went wrong, you know how to handle it, and the only way to do it is to catch an exception.

[sam-wheat]

Why would catch blocks even exist if exceptions couldn't be handled?

I didn't say exceptions can't be handled.

I said "Don't catch exceptions you don't handle."

Generally this means wrapping just one or two lines of code in a try block and catching a specific type of exception such as IOException. The ex is handled specifically such as displaying an error message to the user.

This also means (generally) you should only catch Exception in your root/global handler. It is not necessary wrap every line of code or every method you write in individual try blocks. Use the .net global exception handlers or wrap your root call in a try block. Thats all you need and it makes debugging so much simpler.

The whole notion of "log and continue" and trying to recover state is pretty much nonsense. It leads to data corruption and bugs that are difficult or literally impossible to detect.

There used to be a fantastic page on exception best practices which appears to have been been replaced by this incomplete one.

[ChaseViking]

There is literally 0 reasons to not try/catch your code. ZERO. The bottom line is if you don't "handle" the exception being thrown, then there is the problem, not the catching of it. Anyone trying to make an argument, of any kind, that catching exceptions is not good practice and a solid foundation to your code...well, I just hope we never wind up on the same team somewhere. It says a lot about your development methodologies.

[sam-wheat]

@ChaseViking When you comment on topics like this you should post some technical justification to support what you write. Just coming out with a personal attack like this leaves you with no credibility.

[ChaseViking]

There is absolutely no need for me to justify a thing. Proper exception handling practice has been extremely well documented for decades. I’ve been in the industry for 3 of those decades and have seen hundreds, if not more, developers thinking they know a better way. And there was no personal attack. Try not to be so sensitive. Try/Catch your emotions. Sorry, my attempt at levity.

[mairaw]

Thanks everyone for your comments. We've added this issue to a future sprint.