Remove or replace catch blocks that handle Exception #4870
Comments
mairaw
added
doc-enhancement
|
Thanks for opening this issue, @jinek. This is a problem throughout the documentation set, so I've added it to the .NET Community Contributors project.. So if anyone would like to work on this, please let us know.
|
|
I would like to work on it |
|
Thank you so much for your offer to help with addressing exception handling in the doc set, @jinek! I'll respond by tomorrow with a detailed plan for reviewing the examples that will help to get you started. |
|
To address the issue of incorrect exception handling:
|
|
The following are some of the samples that catch Exception. These are all C# examples. In most cases, there is a parallel Visual Basic example in the corresponding snippets\visualbasic directory, and there is sometimes an example in the corresponding snippets\cpp directory.
|
rpetrusha
changed the title
rpetrusha
changed the title
|
Is there a reason why this issue is still open when the changes have already been made and merged? What am I missing? |
dotnet-bot
added
the
won't fix
|
This issue has been closed as part of the issue backlog grooming process outlined in #22351. That automated process may have closed some issues that should be addressed. If you think this is one of them, reopen it with a comment explaining why. Tag the |
There were fixed only few. Thousands of them remain same issue. |
sharwell
removed
the
won't fix
IEvangelist
added
the
mvp-docs-sprint
|
Is this actionable? How would the relevant places be identified? |
Either analysis rule https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1031 can be checked or using regular expression. chatGPT proposed this: It will probably give some false-positive lines tho. |
There are a lot of examples demonstrating the use of "catch(Exception)" or "catch{}" that leads application to unexpected behavior.
Literally such catch block says "whatever happened lets do one thing". In most cases it does not make sense.
For example, on this page there is code
catch (Exception ex) { Console.WriteLine("Exception caught in CreateTestMessage1(): {0}", ex.ToString() );In case of any issues with application itself or environment that finished by Exception (NullReferenceException, OutOfMemoryException etc) we just swallow that exception and put some information in to console. First of all any unhandled exception in console application will be written to console by itself. Second, should the caller of this method be notified about something happened? Again, in most cases yes, because mostly code that follows this method assumes this method success. If the task was just "try to send email" then we should put appropriate exception types (SmtpFailedRecipientException, SmtpException etc), because other exceptions do not mean email was not sent, situation when exception was thrown after email was sent possible too. In common other exceptions not stated in documentation can be not related to email sending at all.
Statement from c# Programming Guide:
Do not catch an exception unless you can handle it and leave the application in a known state. If you catch System.Exception, rethrow it using the throw keyword at the end of the catch block.
More information here: MSDN
I think in most cases try/catch blocks can be just removed from examples. Sometimes they can be replaced/improved by exact exception type catching and sometimes by rethrowing.
In very rare case catch{} makes sense in context of application lifetime considering.
We have fixed such several examples, but there are much more of them (example) (especially in code related to UWP).
I'd like to assign to it, but anyway I want to discuss it as new examples will be added, may be it makes sense to create documentation tests, for example.
@mairaw , asking you, please, kindly review this issue as you already had a deal with it
The text was updated successfully, but these errors were encountered: