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

[Andre Dietisheim (JIRA)]

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

Andre Dietisheim commented on JBIDE-12911:
------------------------------------------

I had a quick look and checked the classes listed above in an exemplary manner. I still se quite some cases where I'm not sure that the code does its best to handle errors. I'd seek for input on these and close this issue if there are valid arguments:

* Is catching plain Exception and swallowing all possible errors really the best way to go?

{code:title=AS7DeploymentScannerUtility#updateDeploymentScannerInterval}
		try {
			executeWithResult(server, request);
		} catch(Exception e) {
			return false;
		}
{code}

{code:title=AS7DeploymentScannerUtility#getDeploymentScannerIntervals}
try {
			response = executeWithResult(server, request);
		} catch(Exception e) {
			return new HashMap<String, Integer>();
		}
{code}

{code:title=AS7DeploymentScannerUtility#getDeploymentScannersFromServer}
		try {
			response = executeWithResult(server, request);
		} catch(Exception e) {
			return new HashMap<String, String>();
		}
{code}

* empty catch block?
{code:title=ModelChangeListenerWithRefresh#postChange}
		try {
			// refresh infinitely in case the output is exploded
			res.refreshLocal(IResource.DEPTH_INFINITE, new NullProgressMonitor());
		} catch( CoreException ce ) {
		}

{code}

{code:title=ModelChangeListenerWithRefresh#postChange}
	try {
		proj.getFile(new Path(IArchiveModel.DEFAULT_PACKAGES_FILE)).refreshLocal(IResource.DEPTH_ONE, new NullProgressMonitor());
	} catch( CoreException ce ) {
	}
{code}

{code:title=JBossSARFacetInstallationDelegate#execute}
	try{
		IFolder srcFolder = ws.getRoot().getFolder(cpe.getPath());
		
		IVirtualResource[] virtualResource = ComponentCore.createResources(srcFolder);
		//create link for source folder only when it is not mapped
		if( virtualResource.length == 0 ){
			jbiRoot.createLink(cpe.getPath().removeFirstSegments(1), 0, null);							
		}
	}
	catch(Exception e){
		// TODO 
	}
{code}

{code:title=RSEDeploymentPreferenceUI}
				try {
					method.getFileService();
					method.ensureConnection(new NullProgressMonitor());
					IHostFile file = method.getFileService().getFile(remoteFolder.removeLastSegments(1).toOSString(), remoteFolder.lastSegment(), new NullProgressMonitor());
					String path = remoteFolder.toString();
					
					IRemoteFile rf = method.getFileServiceSubSystem().getRemoteFileObject(path, null);
					
					SystemShowInTableAction act = new SystemShowInTableAction(Display.getDefault().getActiveShell()); 
					act.setSelectedObject(rf);
					act.run();
				} catch(SystemMessageException e) {
					
				} catch(CoreException ce) {
					
				}
{code}

* printStackTrace?

{code:title=JBossSARFacetInstallationDelegate#createProjectStructure}
		try {
			node.flush();
		} catch (BackingStoreException e) {
			e.printStackTrace();
		}
{code}

{code:title=NewMessageDrivenBeanWizardPage#createType}
      try
      {
         UndoEdit undo = edit.apply(doc);
         String source = doc.get();
         beanUnit.getBuffer().setContents(source);
         beanUnit.getBuffer().save(monitor, true);

      }
      catch (BadLocationException e)
      {
         e.printStackTrace();
      }

{code}

                
> 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: Andre Dietisheim
>             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