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());
}
