Error Handling and Infrastructure Report

Current Status (11/19/07) - following issues resolved

• search for all e.printStackTrace() and remove and handle appropriately
  • matching to issue 1~3, 10
• search for all swallowed exceptions and emit appropriately
  • matching to issue 1~3, 8
• org.knownspace.fluency.engine.core.exceptions* to be subclasses of FluencyException and move to org.knownspace.fluency.shared.exceptions
  • matching to issue 9

Current Status (11/11/07)

• Abstract class FluencyException extends RuntimeException and is intended to be the base exception for all Exceptions generated by Fluency.
• Two exceptions have been created which extend this base Exception:
  
  • FatalFluencyException
    
  • NonFatalFluencyExeption
    The expectation is that all application exceptions will be subclasses of one of these two types.
• FluencyException is abstract because it is cleaner to force instantiation of one of the above subtypes.
  
• main methods in Fluency.java and FluencyEngine.java are catching exceptions like this:
  sample main method

  public static void main(String[] args) {
    FluencyExceptionUtil.init();
    try {
      FluencyEditorFrame.FLUENCY_WINDOW.start();			
    } catch (FatalFluencyException ffe) {
      FluencyExceptionUtil.handleException(ffe);
    } catch (NonFatalFluencyException nffe) {
      FluencyExceptionUtil.handleException(nffe);
    } catch (RuntimeException re) {			
      FluencyExceptionUtil.handleException(new FatalFluencyException(re));
    } catch (Exception e) {
      FluencyExceptionUtil.handleException(new NonFatalFluencyException(e));
    }
  }

  The order of the catch blocks is significant (particularly, RuntimeException and Exception) because it allows for cleaner chaining and does not clutter the code in the handler with lots of instanceof if blocks.
  

• If you want to temporarily turn off exception handling when you are debugging a problem (and you do not want Fluency to terminate on Fatal exceptions) you can turn off exception handling by setting handle_exceptions=false as an environment variable. The default is always on.
  You can do this easily in Eclipse (open run dialog --> environment tab for your executable class).
• If you are throwing an exception in a catch block that is a subclass of FluencyException then it is not necessary to log the exception. Logging is handled implicitly by FluencyException.

Outstanding Issues (updated 11/11/07 – includes New items)

1. Much of the current code base does not throw Exceptions in a typical Java fashion.

• exceptions are swallowed throughout the codebase
• majority of methods do not use throws clause
• type Exception is caught rather than the most specific exception that applies

2. In many places we do not need to catch exceptions but should just throw them to be handled further up the call stack

3. We cannot use "brute force" approach to change all methods which now swallow exceptions to instead throw all exceptions because sometimes we actually want to ignore the problem and do something else

• see TagXMLHandler.getTags() where we return an Empty Tag object if we could not the load one from file

Update for Items 1 - 3 I have fixed exception swallowing in a few places. Here is what I think we should all be aware of as we are working in the codebase: We should not catch type Exception, FluencyException, RunTimeException, etc. We should catch the most specific checked exception(s) type which applies to your try block (e.g. IOException, FileNotFoundException, etc.) We should attempt to handle exceptions where they occur (i.e. in catch blocks) if appropriate. There are legitimate reasons where we will not need to "throw" the exception further up the call stack. If we do not know how to handle the exception (or believe that the Exception handling framework should handle it for us), we should make a determination as to whether it is a Fatal (will shut fluency down, typically these are the standard java RuntimeExeptions) or NonFatal (which we should report and log but try to recover from) exception. Then we should chain the caught exception and rethrow as a subclass as FluencyException. For example: throw new NonFatalFluencyException("Error doing something in a method", caughtException);

4. We need to integrate the Log4J logging framework into the handling of exceptions

Update for Item 4 Resolved. Handled implictly when exceptions are created which are subclasses of FluencyException. This also applies to chaining exceptions inside a FluencyException.

5. A number of constructors have A LOT of logic in them and many are calling methods which presently swallow exceptions. It is generally discouraged to throw exceptions from a constructor.

• One solution is to refactor to use factory pattern
• In at least a few cases this is true of a singleton class (see FluencyEditorFrame) so need an approach for this case

6. A number of overridden swing interface methods do not allow for a throws clause. For example:

interface ActionListener {
   public void actionPerformed(ActionEvent actionEvent);
 }

7. We need to consider how we will report errors vs exception. For example:

class MyActionListener {
   public void actionPerformed(ActionEvent actionEvent) {
     //  do something which may cause an exception like IOException
     //  but cannot throw an exception from this method.
     // Should we create a light Error Message reporting framework for this?
   }
 }

Update for Items 5 - 7 Since our base exception subclasses RuntimeException, these are no longer concerns.

8. Right now, generally only Unchecked (i.e. traditional Java Runtime exceptions) will trigger the dialog to display a message to the user because most Checked exceptions are swallowed.

Update for Item 8 This is the case because we swallow (obviously!) so many exceptions. See the guidelines in the update for Items 1 - 3 to resolve this.

9. New! Need to change "legacy" exceptions in org.knownspace.fluency.engine.core.exceptions* to be subclasses of FluencyException and move to org.knownspace.fluency.shared.exceptions

10. New! Need to search for all e.printStackTrace() and remove and handle appropriately per the steps listed in update to item 1 - 3.