SonarJava: CatchOfThrowableOrErrorCheck: There are good use cases for catching Throwable

96 views
Skip to first unread message

edp...@gmail.com

unread,
Jan 23, 2018, 4:13:55 PM1/23/18
to SonarQube
The rule CatchOfThrowableOrErrorCheck should be fixed as there are good use cases for catching Throwable.
Please see some examples:
https://github.com/spring-projects/spring-framework/blob/master/spring-jdbc/src/main/java/org/springframework/jdbc/support/JdbcUtils.java#L74
https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/StandardWrapperValve.java#L243

This rule should be fixed or people will break correct code.

One way to fix this rule is to only warn when catching Error as I do not know a valid use case for catching Error.

Nicolas Peru

unread,
Feb 2, 2018, 4:51:12 AM2/2/18
to edp...@gmail.com, SonarQube
Hi, 

Given the amount of valid cases of catching Throwable compared to cases where it is a real code smell I would keep the rule as is and let SonarQube users mark valid use cases as won't fix rather than silencing the rule.

Cheers, 

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/2fd0f05a-c2fe-463c-86dd-5c4cd644dd99%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
--
Nicolas Peru | SonarSource

edp...@gmail.com

unread,
Feb 4, 2018, 5:41:37 AM2/4/18
to SonarQube
Hi,

I have created a pull request to improve the documentation as it currently suggests that catching Throwable is a mistake in every case.
https://github.com/SonarSource/sonar-java/pull/1897
How do you think this documentation could be improved?

Cheers,

Nicolas Peru

unread,
Feb 5, 2018, 9:14:53 AM2/5/18
to edp...@gmail.com, SonarQube
Hi, 

The documentation seems quite ok to me as it uses the word : "should" which convey the proper meaning that in general this is not a good idea. Then if you do it, you should know what you are doing and turn the warning off for the corner case where this might be a good idea.

Regarding your suggestion, actually using "may" in this case goes against your intent (if i'm correct) : the wording modification you suggest reinforce the fact that it is absolutely forbidden to catch Throwable whereas the word "should" let the door open that there might exists a case where it is correct.

Cheers, 


For more options, visit https://groups.google.com/d/optout.

edp...@gmail.com

unread,
Feb 5, 2018, 4:48:53 PM2/5/18
to SonarQube
Hi,

I just want to add some text to let the reader know that there are perfectly good use cases for catching Throwable and this one may be one of them.

How do you think the current documentation should be updated to reflect this?

Cheers,

Nicolas Peru

unread,
Feb 6, 2018, 3:22:33 AM2/6/18
to edp...@gmail.com, SonarQube
Hi,

You are asking me the same question so I'll give you the same answer : the documentation already reflect this by the usage of the world "should" that implies there are some valid use cases.
So I don't think we need to update doc for this.
Cheers, 


For more options, visit https://groups.google.com/d/optout.
Reply all
Reply to author
Forward
0 new messages