[jbosstools-issues] [JBoss JIRA] (JBIDE-12911) CLONE - Review error handling code in as plugins

[Rob Stryker (JIRA)]

    [ https://issues.jboss.org/browse/JBIDE-12911?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12736615#comment-12736615 ] 

Rob Stryker commented on JBIDE-12911:
-------------------------------------

For all instances of AS7DeploymentScannerUtility listed, the caller is expecting return values and do not wish to see exceptions. Any caller wishing to be informed of errors (and as of now there are none) would need to request a new API to see such. 

Again, the callers here simply do not wish to see such exceptions.  They just want the answers. 

For ModelChangeListenerWithRefresh, in one instance you have specifically ignored the comment indicating an intentional lack of reporting:  					// Also do not log. 

One instance of ModelChangeListenerWithRefresh has an error on the setPersistentPRoperty (though the code is nested and could be cleaned up a bit). This will only fail if the project or file does not exist, a situation which is not requiring error handling. It may require if-statements before it, but the general idea of this code is to refresh something *IF* its available, or set the property *IF* its available. If it's not, do nothing. Hence, ignore the exception. DO not log it to confuse the users. 

The error in JBossSARFacetInstallationDelegate was most likely there as debugging when I was testing a runtime exception which was an error of my own code. The entire try / catch portion can be removed, as there are no checked exceptions there. The second in that class should simply be logged, as not being able to communicate witht he backing store is critical but also not a common occurance. 

I would imagine the errors in RSEDeploymentPreferenceUI should be logged. 

NewMessageDrivenBeanWizardPage is very old code nad probably needs to be revisted. The intent and the cause of the exception is unclear based on a quick view. 


                
> CLONE - Review error handling code in as plugins
> ------------------------------------------------
>
>                 Key: JBIDE-12911
>                 URL: https://issues.jboss.org/browse/JBIDE-12911
>             Project: Tools (JBoss Tools)
>          Issue Type: Bug
>          Components: JBossAS/Servers
>    Affects Versions: 4.0.0.Beta1
>            Reporter: Andre Dietisheim
>            Assignee: Rob Stryker
>             Fix For: 4.0.0.CR1
>
>
> It is still very easy to find (search Exception in *.java files and review results) wrong exception handling like:
> * hiding exception by returning null or default value in catch block without proper logging (Examples: JBossLaunchAdapter, XPathQuery, AS7DeploymentScannerUtility)
> * use empty catch blocks (examples: ModelChangeListenerWithRefresh, FilesetWizard, RSEJBoss7BehaviourDelegate, JBossSARFacetInstallationDelegate)
> * use generic Exception instead of specific ones (Examples: FilesetWizard, RSEJBoss7BehaviourDelegate, JBossSARFacetInstallationDelegate)
> * use printStackTrace in try/catch blocks which is not always visible unles eclipse is started with console (Example: RSEDeploymentPreferenceUI, DeploymentModuleOptionCompositeAssistant, JBossSARFacetInstallationDelegate, NewMessageDrivenBeanWizardPage)
> Please review the error handling code in JBossAS plugin. I replaced a lot of e.printStackTrace() (in JBIDE-8434) by proper logging. I might have missed some, in some cases more handling is needed, some might not be correct.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira