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