[OS-webwork] ExceptionHandlerIntercepter

[John Patterson]

I just discovered a problem with my ExceptionHandlerInterceptor due to the design of the DefaultActionInvocation class.  I have made some changes to this class which fix the problem and would like to run them past everybody.   The ExceptionHandlerInterceptor catches any exceptions that occur during the invoking of the action (and result) and shows the ERROR result instead.  During previous discussions on this list, no alternative approaches were suggested that would allow for:       1 - Catch exceptions occurring in the Action AND Result     2 - Enable specific error pages to be defined for each package or action     3 - Use action definitions defined in the xwork.xml (eg ERROR)     4 - Retain the offending Action on the stack so it can be examined in an error page.   Strangely enough, it appeared to be working fine before (but for the wrong reasons).   The problem is this: if an exception occurs while processing an Action then the interceptor can return ERROR and the error Result is executed.  However, if an exception occurs while processing the Result and the interceptor returns ERROR, the error Result is NOT executed.  In fact the original result is executed again (which will probably cause the exception again).   This is because once a Result has been instantiated it is retained and reused no matter what resultCode is returned from the interceptor.  executeResult() calls getResult() which returns the Result stored in an instance variable.  It seems that the only reason for this is so that other objects can get the result after it has executed.   Some may say "Exceptions that happen while executing the result should just be rethrown".  But then how are we to implement the error handling requirements above?   Anyway, the changes I made to DefaultActionInvocation are VERY simple and fix the problem by splitting getResult() into two methods (createResult() and getResult()).  executeResult() now calls createResult() instead of recycling the old result.   It all seems to work OK and allows an interceptor to return a result code which will not be ignored unless the result has already executed (executed = true)       Index: DefaultActionInvocation.java =================================================================== RCS file: /cvs/xwork/src/java/com/opensymphony/xwork/DefaultActionInvocation.java,v retrieving revision 1.10 diff -u -r1.10 DefaultActionInvocation.java --- DefaultActionInvocation.java 15 Nov 2003 05:50:15 -0000 1.10 +++ DefaultActionInvocation.java 18 Nov 2003 15:18:34 -0000 @@ -93,49 +93,50 @@  * @throws Exception  */      public Result getResult() throws Exception { -        if (result != null) { -            Result returnResult = result; - -            // If we've chained to other Actions, we need to find the last result -            while (returnResult instanceof ActionChainResult) { -                ActionProxy aProxy = ((ActionChainResult) returnResult).getProxy(); - -                if (aProxy != null) { -                    Result proxyResult = aProxy.getInvocation().getResult(); - -                    if ((proxyResult != null) && (aProxy.getExecuteResult())) { -                        returnResult = proxyResult; -                    } else { -                        break; -                    } -                } else { -                    break; -                } -            } - -            return returnResult; -        } else { -            Map results = proxy.getConfig().getResults(); -            ResultConfig resultConfig = (ResultConfig) results.get(resultCode); - -            if (resultConfig != null) { -                Class resultClass = resultConfig.getClazz(); +     Result returnResult = result; +  +     // If we've chained to other Actions, we need to find the last result +     while (returnResult instanceof ActionChainResult) { +         ActionProxy aProxy = ((ActionChainResult) returnResult).getProxy(); +  +         if (aProxy != null) { +             Result proxyResult = aProxy.getInvocation().getResult(); +  +             if ((proxyResult != null) && (aProxy.getExecuteResult())) { +                 returnResult = proxyResult; +             } else { +                 break; +             } +         } else { +             break; +         } +     } +  +     return returnResult; +    } +    +    public Result createResult() throws Exception { +    Map results = proxy.getConfig().getResults(); +    ResultConfig resultConfig = (ResultConfig) results.get(resultCode); +    Result newResult = null; +    +    if (resultConfig != null) { +      Class resultClass = resultConfig.getClazz(); + +      if (resultClass != null) { +        try { +          newResult = (Result) resultClass.newInstance(); +        } catch (Exception e) { +          LOG.error("There was an exception while instantiating the result of type " + resultClass, e); +          throw e; +        } + +        OgnlUtil.setProperties(resultConfig.getParams(), newResult, ActionContext.getContext().getContextMap()); +      } +    }   -                if (resultClass != null) { -                    try { -                        result = (Result) resultClass.newInstance(); -                    } catch (Exception e) { -                        LOG.error("There was an exception while instantiating the result of type " + resultClass, e); -                        throw e; -                    } - -                    OgnlUtil.setProperties(resultConfig.getParams(), result, ActionContext.getContext().getContextMap()); -                } -            } - -            return result; -        } -    } +    return newResult; +  }        public String getResultCode() {          return resultCode; @@ -260,13 +261,13 @@      }        /** -* Uses getResult to get the final Result and executes it +* Creates a new Result from the resultCode and executes it  */      private void executeResult() throws Exception { -        Result aResult = getResult(); +        result = createResult();   -        if (aResult != null) { -            aResult.execute(this); +        if (result != null) { +            result.execute(this);          } else {              LOG.warn("No result defined for action " + getAction().getClass().getName() + " and result " + getResultCode());          }