You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by Scott Eade <se...@backstagetech.com.au> on 2005/09/28 14:41:56 UTC
Exceptions thrown in actions
Should an exception thrown in an action result in execution of the
screen configured as "screen.error" in TurbineResources.properties?
I had kind of assumed that it should, but it certainly does not -
VelocityActionEvent simply logs the exception and then carries on as if
all is well. This is not that great because it means the user receives
no feedback that an exception has occurred.
Scott
--
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by sergiu gordea <gs...@ifit.uni-klu.ac.at>.
The turbine 2.2 was throwing these exceptions, but Turbine 2.3 is not
rowing them anymore,
I needed to overwrite a method that is catching all exceptions in
turbine, don't remember exactly which one.
Best,
Sergiu
Scott Eade wrote:
>Should an exception thrown in an action result in execution of the
>screen configured as "screen.error" in TurbineResources.properties?
>
>I had kind of assumed that it should, but it certainly does not -
>VelocityActionEvent simply logs the exception and then carries on as if
>all is well. This is not that great because it means the user receives
>no feedback that an exception has occurred.
>
>Scott
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by Thomas Vandahl <th...@tewisoft.de>.
Henning P. Schmiedehausen wrote:
> Hm. This was put in > two years ago. I do remember that I got this
> from load testing one of our applications and that was the
> solution. We were getting spurious ITEs without any real cause in the
> application but out of the invoke() code. It raised no comment on the
> -dev list back then (according to the archives).
That's interesting. I have a similar case that somehow depends on MS IE
as a client, where the eventSubmit_doSomething call fails somehow and
this causes the doPerform() method to be executed afterwards.
As the method making the call is only catching an ITE, I wonder now if
this may be related to your problem (we're talking 2.3.1). I wrote an
ugly workaround to fix this, so I would rather get rid of it asap.
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by sergiu gordea <gs...@ifit.uni-klu.ac.at>.
Henning P. Schmiedehausen wrote:
>Scott Eade <se...@backstagetech.com.au> writes:
>
>
>
>>Do you have a comment on this Henning?
>>
>>
>
>Hm. This was put in > two years ago. I do remember that I got this
>from load testing one of our applications and that was the
>solution. We were getting spurious ITEs without any real cause in the
>application but out of the invoke() code. It raised no comment on the
>-dev list back then (according to the archives).
>
>Should we address it in 2.3.2? That code is in there ever since 2.3RC1
>and the whole 2.3 cycle. Changing it would be actually changing
>behaviour from previous 2.3 releases to 2.3.2. So I'd say cautiously
>"no, not in the default". We might add a property to control this, but
>this feels like a kludge.
>
>
I, as a turbine user I would vote for throwing the exception, otherwise
turbine doesn't redirect
the page to the error screen, when an exception occurs in Actions.
In other words, it hides probably the most important exceptions, those
that update the information in the database.
Best,
Sergiu
>I will post the result of the vote today. I will hold the actual
>rolling of the release until tomorrow to see if anyone else pipes
>up. If you have a patch ready for review that would be even better.
>
> Best regards
> Henning
>
>
>
>
>
>>IMO this should be addressed for 2.3.2.
>>
>>
>
>
>
>>Scott
>>
>>
>
>
>
>>Scott Eade wrote:
>>
>>
>
>
>
>>>It looks like Henning's change in revision 221603 is the culprit.
>>>
>>>The commit message is:
>>>
>>>
>>>
>>>
>>>
>>>>Catch InvokationTargetExceptions and log them at error level. I was
>>>>able to provoke this by rapidly firing and aborting HTTP requests for
>>>>an action at a Tomcat server. Sooner or later, the container would get
>>>>out of step and suddently the invoked methods would disappear from the
>>>>class loader.
>>>>
>>>>I was also thinking about
>>>>
>>>>Throwable t = ite.getTargetException();
>>>>if (t instanceof Exception)
>>>>{
>>>> throw (Exception) t;
>>>>}
>>>>else
>>>>{
>>>> throw new TurbineException(t);
>>>>}
>>>>
>>>>but decided against it, because this would be a spurious and hard to
>>>>reproduce (and debug) error reported by the users. Discussion wanted.
>>>>
>>>>
>>>>
>>>>
>>>Another way of provoking this exception is to have an action method thus:
>>>
>>> public void doThrowexception(RunData data, Context context)
>>> throws Exception
>>> {
>>> throw new MyException(".doThrowexception() - Goody!");
>>> }
>>>
>>>Scott
>>>
>>>
>>>
>>>
>
>
>
>>Scott Eade wrote:
>>
>>
>
>
>
>>>>Should an exception thrown in an action result in execution of the
>>>>screen configured as "screen.error" in TurbineResources.properties?
>>>>
>>>>I had kind of assumed that it should, but it certainly does not -
>>>>VelocityActionEvent simply logs the exception and then carries on as if
>>>>all is well. This is not that great because it means the user receives
>>>>no feedback that an exception has occurred.
>>>>
>>>>Scott
>>>>
>>>>
>>>>
>
>
>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
>>For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
>>
>>
>
>
>
Re: Exceptions thrown in actions
Posted by Scott Eade <se...@backstagetech.com.au>.
Henning P. Schmiedehausen wrote:
>Scott Eade <se...@backstagetech.com.au> writes:
>
>
>>+ throw (Exception) ite.getTargetException();
>>
>>
>[...]
>
>
>>+ throw (Exception) ite.getTargetException();
>>
>>
>
>And I firmly -1 it in that incarnation. Please read my comments to the
>commit message of r221603.
>
>
Doh!
>If you simply cast a potential Throwable to Exception, then you are in
>for CCE's sooner or later and _that_ will leave users bewildered even
>more. If there really is a need for fixing this, let's fix it
>correctly.
>
>
Agreed, but of course if you are getting something other than an
Exception you have bigger problems :-)
I didn't see any point in wrapping the Throwable in a TurbineException -
the ITE is probably a truer indication of what has gone on. I am happy
to wrap this if you feel strongly about it.
Cheers,
Scott
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Scott Eade <se...@backstagetech.com.au> writes:
>Henning P. Schmiedehausen wrote:
>>Hm. This was put in > two years ago. I do remember that I got this
>>from load testing one of our applications and that was the
>>solution. We were getting spurious ITEs without any real cause in the
>>application but out of the invoke() code. It raised no comment on the
>>-dev list back then (according to the archives).
>>
>>
>I think this falls into the "the consequences of the change were not
>particularly obvious at the time" bucket - after all, if your actions
>don't throw any exceptions you may not have noticed it. For my part I
>know that the pressure has been to correct the problem (the cause of the
>exception being thrown) more than it has been on ensuring that the fact
>that something has gone wrong is reported to the user.
>>Should we address it in 2.3.2? That code is in there ever since 2.3RC1
>>and the whole 2.3 cycle. Changing it would be actually changing
>>behaviour from previous 2.3 releases to 2.3.2. So I'd say cautiously
>>"no, not in the default". We might add a property to control this, but
>>this feels like a kludge.
>>
>>
>I would say that by not fixing it we are not adhering to our own
>specification - see:
>http://jakarta.apache.org/turbine/turbine/development/turbine-2.3/fsd.html#Exception_Handling
>>I will post the result of the vote today. I will hold the actual
>>rolling of the release until tomorrow to see if anyone else pipes
>>up. If you have a patch ready for review that would be even better.
>>
>>
>I am curious to know more about your load testing. Could the "ITEs
>without real cause" have actually had a cause? Was the testing such
The problem were HTTP requests that were sent out by the browser/load
testing tool (ab) but aborted before the application/container could
actually send out a reply. I don't have the details really handy
anymore but we basically used a really simple action class (which was
guaranteed to not throw any exceptions) and still ended up with
Exceptiones being thrown. These were "class not found" errors, which
is pretty scary, considering the fact that the getMethod() call a few
nanos earlier had succeeded.
>that the behaviour of the processing was confirmed (not necessarily
>something that would occur during load testing) - i.e. did it ignore the
>fact that the Action code was encountering an exception that was being
>swallowed with the processing left incomplete? Was the
>Cause/TargetException of the ITEs examined - and if so was there an
>underlying cause that could be swallowed rather than the ITE itself
>(currently this is the equivalent of swallowing Exception rather than
>the specific subclass).
>The fact that any action that encounters an exception reports nothing to
>the user at all, leaving them totally bewildered as to why they are
>stuck on the same page is not good at all.
I understand that.
>A tested patch is included below.
>+ throw (Exception) ite.getTargetException();
[...]
>+ throw (Exception) ite.getTargetException();
And I firmly -1 it in that incarnation. Please read my comments to the
commit message of r221603.
If you simply cast a potential Throwable to Exception, then you are in
for CCE's sooner or later and _that_ will leave users bewildered even
more. If there really is a need for fixing this, let's fix it
correctly.
Besides from this, the patch is IMHO ok.
Best regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
4 - 8 - 15 - 16 - 23 - 42
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by Scott Eade <se...@backstagetech.com.au>.
Henning P. Schmiedehausen wrote:
>Hm. This was put in > two years ago. I do remember that I got this
>from load testing one of our applications and that was the
>solution. We were getting spurious ITEs without any real cause in the
>application but out of the invoke() code. It raised no comment on the
>-dev list back then (according to the archives).
>
>
I think this falls into the "the consequences of the change were not
particularly obvious at the time" bucket - after all, if your actions
don't throw any exceptions you may not have noticed it. For my part I
know that the pressure has been to correct the problem (the cause of the
exception being thrown) more than it has been on ensuring that the fact
that something has gone wrong is reported to the user.
>Should we address it in 2.3.2? That code is in there ever since 2.3RC1
>and the whole 2.3 cycle. Changing it would be actually changing
>behaviour from previous 2.3 releases to 2.3.2. So I'd say cautiously
>"no, not in the default". We might add a property to control this, but
>this feels like a kludge.
>
>
I would say that by not fixing it we are not adhering to our own
specification - see:
http://jakarta.apache.org/turbine/turbine/development/turbine-2.3/fsd.html#Exception_Handling
>I will post the result of the vote today. I will hold the actual
>rolling of the release until tomorrow to see if anyone else pipes
>up. If you have a patch ready for review that would be even better.
>
>
I am curious to know more about your load testing. Could the "ITEs
without real cause" have actually had a cause? Was the testing such
that the behaviour of the processing was confirmed (not necessarily
something that would occur during load testing) - i.e. did it ignore the
fact that the Action code was encountering an exception that was being
swallowed with the processing left incomplete? Was the
Cause/TargetException of the ITEs examined - and if so was there an
underlying cause that could be swallowed rather than the ITE itself
(currently this is the equivalent of swallowing Exception rather than
the specific subclass).
The fact that any action that encounters an exception reports nothing to
the user at all, leaving them totally bewildered as to why they are
stuck on the same page is not good at all.
A tested patch is included below.
Scott
Index: C:/eclipse-workspace/jakarta-turbine-2_3/xdocs/changes.xml
===================================================================
--- C:/eclipse-workspace/jakarta-turbine-2_3/xdocs/changes.xml
(revision 292693)
+++ C:/eclipse-workspace/jakarta-turbine-2_3/xdocs/changes.xml
(working copy)
@@ -24,6 +24,11 @@
</properties>
<body>
+ <release version="2.3.2" date="2005-09-30">
+ <action type="fix" dev="seade">
+ Exceptions that occur in Actions once again redirect to the error
page.
+ </action>
+ </release>
<release version="2.3.2-rc2" date="2005-09-19">
<action type="fix" dev="henning" issue="TRB-5" due-to="Thomas Vandahl">
The skin properties file must be loaded as stream from the filesystem
Index:
C:/eclipse-workspace/jakarta-turbine-2_3/src/java/org/apache/turbine/modules/ActionEvent.java
===================================================================
---
C:/eclipse-workspace/jakarta-turbine-2_3/src/java/org/apache/turbine/modules/ActionEvent.java
(revision 280458)
+++
C:/eclipse-workspace/jakarta-turbine-2_3/src/java/org/apache/turbine/modules/ActionEvent.java
(working copy)
@@ -183,11 +183,9 @@
throw new NoSuchMethodException("ActionEvent: The button
was null");
}
- Method method = null;
-
try
{
- method = getClass().getMethod(theButton, methodParams);
+ Method method = getClass().getMethod(theButton, methodParams);
Object[] methodArgs = new Object[] { data };
if (log.isDebugEnabled())
@@ -199,8 +197,7 @@
}
catch (InvocationTargetException ite)
{
- Throwable t = ite.getTargetException();
- log.error("Invokation of " + method , t);
+ throw (Exception) ite.getTargetException();
}
finally
{
Index:
C:/eclipse-workspace/jakarta-turbine-2_3/src/java/org/apache/turbine/util/velocity/VelocityActionEvent.java
===================================================================
---
C:/eclipse-workspace/jakarta-turbine-2_3/src/java/org/apache/turbine/util/velocity/VelocityActionEvent.java
(revision 279565)
+++
C:/eclipse-workspace/jakarta-turbine-2_3/src/java/org/apache/turbine/util/velocity/VelocityActionEvent.java
(working copy)
@@ -122,10 +122,9 @@
"ActionEvent: The button was null");
}
- Method method = null;
try
{
- method = getClass().getMethod(theButton, methodParams);
+ Method method = getClass().getMethod(theButton, methodParams);
Object[] methodArgs = new Object[] { data, context };
if (log.isDebugEnabled())
@@ -149,8 +148,7 @@
}
catch (InvocationTargetException ite)
{
- Throwable t = ite.getTargetException();
- log.error("Invokation of " + method , t);
+ throw (Exception) ite.getTargetException();
}
finally
{
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Scott Eade <se...@backstagetech.com.au> writes:
>Do you have a comment on this Henning?
Hm. This was put in > two years ago. I do remember that I got this
from load testing one of our applications and that was the
solution. We were getting spurious ITEs without any real cause in the
application but out of the invoke() code. It raised no comment on the
-dev list back then (according to the archives).
Should we address it in 2.3.2? That code is in there ever since 2.3RC1
and the whole 2.3 cycle. Changing it would be actually changing
behaviour from previous 2.3 releases to 2.3.2. So I'd say cautiously
"no, not in the default". We might add a property to control this, but
this feels like a kludge.
I will post the result of the vote today. I will hold the actual
rolling of the release until tomorrow to see if anyone else pipes
up. If you have a patch ready for review that would be even better.
Best regards
Henning
>IMO this should be addressed for 2.3.2.
>Scott
>Scott Eade wrote:
>>It looks like Henning's change in revision 221603 is the culprit.
>>
>>The commit message is:
>>
>>
>>
>>>Catch InvokationTargetExceptions and log them at error level. I was
>>>able to provoke this by rapidly firing and aborting HTTP requests for
>>>an action at a Tomcat server. Sooner or later, the container would get
>>>out of step and suddently the invoked methods would disappear from the
>>>class loader.
>>>
>>>I was also thinking about
>>>
>>> Throwable t = ite.getTargetException();
>>> if (t instanceof Exception)
>>> {
>>> throw (Exception) t;
>>> }
>>> else
>>> {
>>> throw new TurbineException(t);
>>> }
>>>
>>>but decided against it, because this would be a spurious and hard to
>>>reproduce (and debug) error reported by the users. Discussion wanted.
>>>
>>>
>>
>>Another way of provoking this exception is to have an action method thus:
>>
>> public void doThrowexception(RunData data, Context context)
>> throws Exception
>> {
>> throw new MyException(".doThrowexception() - Goody!");
>> }
>>
>>Scott
>>
>>
>Scott Eade wrote:
>>>Should an exception thrown in an action result in execution of the
>>>screen configured as "screen.error" in TurbineResources.properties?
>>>
>>>I had kind of assumed that it should, but it certainly does not -
>>>VelocityActionEvent simply logs the exception and then carries on as if
>>>all is well. This is not that great because it means the user receives
>>>no feedback that an exception has occurred.
>>>
>>>Scott
>>>
>>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
4 - 8 - 15 - 16 - 23 - 42
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by Scott Eade <se...@backstagetech.com.au>.
Do you have a comment on this Henning?
IMO this should be addressed for 2.3.2.
Scott
Scott Eade wrote:
>It looks like Henning's change in revision 221603 is the culprit.
>
>The commit message is:
>
>
>
>>Catch InvokationTargetExceptions and log them at error level. I was
>>able to provoke this by rapidly firing and aborting HTTP requests for
>>an action at a Tomcat server. Sooner or later, the container would get
>>out of step and suddently the invoked methods would disappear from the
>>class loader.
>>
>>I was also thinking about
>>
>> Throwable t = ite.getTargetException();
>> if (t instanceof Exception)
>> {
>> throw (Exception) t;
>> }
>> else
>> {
>> throw new TurbineException(t);
>> }
>>
>>but decided against it, because this would be a spurious and hard to
>>reproduce (and debug) error reported by the users. Discussion wanted.
>>
>>
>
>Another way of provoking this exception is to have an action method thus:
>
> public void doThrowexception(RunData data, Context context)
> throws Exception
> {
> throw new MyException(".doThrowexception() - Goody!");
> }
>
>Scott
>
>
Scott Eade wrote:
>>Should an exception thrown in an action result in execution of the
>>screen configured as "screen.error" in TurbineResources.properties?
>>
>>I had kind of assumed that it should, but it certainly does not -
>>VelocityActionEvent simply logs the exception and then carries on as if
>>all is well. This is not that great because it means the user receives
>>no feedback that an exception has occurred.
>>
>>Scott
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: Exceptions thrown in actions
Posted by Scott Eade <se...@backstagetech.com.au>.
It looks like Henning's change in revision 221603 is the culprit.
The commit message is:
> Catch InvokationTargetExceptions and log them at error level. I was
> able to provoke this by rapidly firing and aborting HTTP requests for
> an action at a Tomcat server. Sooner or later, the container would get
> out of step and suddently the invoked methods would disappear from the
> class loader.
>
> I was also thinking about
>
> Throwable t = ite.getTargetException();
> if (t instanceof Exception)
> {
> throw (Exception) t;
> }
> else
> {
> throw new TurbineException(t);
> }
>
> but decided against it, because this would be a spurious and hard to
> reproduce (and debug) error reported by the users. Discussion wanted.
Another way of provoking this exception is to have an action method thus:
public void doThrowexception(RunData data, Context context)
throws Exception
{
throw new MyException(".doThrowexception() - Goody!");
}
Scott
--
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au
Scott Eade wrote:
>Should an exception thrown in an action result in execution of the
>screen configured as "screen.error" in TurbineResources.properties?
>
>I had kind of assumed that it should, but it certainly does not -
>VelocityActionEvent simply logs the exception and then carries on as if
>all is well. This is not that great because it means the user receives
>no feedback that an exception has occurred.
>
>Scott
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org