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