You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by khudalla <Ka...@de.bosch.com> on 2008/05/30 10:53:40 UTC
Handling of RuntimeExceptions in ActiveMQSession.run() method
Hi,
I have stumbled across the following code in the ActiveMQSession.run()
method:
try {
messageListener.onMessage(message);
} catch (Throwable e) {
// TODO: figure out proper way to handle error.
LOG.error("error dispatching message: ", e);
connection.onAsyncException(e);
}
As the TODO points out, the way how to handle exceptions properly needs some
discussion. I have come across this while working on the Resource Adapter in
conjunction with GlassFish v2. When I use batching when delivering messages
to an MDB and the MDB marks the current TX as rollback only (e.g. if it
cannot access a database), any subsequent invocation of the MDB's
onMessage() method (actually the invocation of the wrapper around that
method provided by the GlassFish app server) in the same TX will throw a
javax.ejb.TransactionRolledbackLocalException in order to indicate that it
is futile to invoke the bean since the TX will be rolled back anyway. This
RuntimeException will now lead to the catch block being executed, i.e. the
connection's ExceptionListener will be notified eventually which in this
case is the listener that the RA has registered on the connection which in
turn will tear down the connection and reconnect to the broker. However, the
connection never failed in the first place, i.e. reconnecting to the broker
is not necessary at all.
I wonder whether the ActiveMQSession.run() method should better ignore any
RuntimeException thrown by the MessageListener (i.e. do not invoke
connection.onAsyncException()) since these exceptions generally indicate a
problem with the processing of the message, not with receiving the message.
Any thoughts?
Kai
--
View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17554360.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by khudalla <Ka...@de.bosch.com>.
Done!
I actually tried to do that when uploading the patch initially. However, the
checkbox was not available then. It is only available when attaching a file
after the issue has been created ...
Kai
rajdavies wrote:
>
> Kai - can you re-add the patch and click the grant ASF licence option?
> On 2 Jun 2008, at 16:06, khudalla wrote:
>
>
--
View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17616968.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by Rob Davies <ra...@gmail.com>.
Kai - can you re-add the patch and click the grant ASF licence option?
On 2 Jun 2008, at 16:06, khudalla wrote:
>
> Rob,
>
> I have created JIRA issue AMQ-1760 and attached a patch implementing
> the
> functionality we have been talking about.
> Would you please take a look at it?
>
> Regards,
> Kai
>
>
> rajdavies wrote:
>>
>> Sounds good - please send a patch!
>>
>>
>
> --
> View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17603539.html
> Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
>
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by Rob Davies <ra...@gmail.com>.
Great thx Kai!
On 2 Jun 2008, at 16:06, khudalla wrote:
>
> Rob,
>
> I have created JIRA issue AMQ-1760 and attached a patch implementing
> the
> functionality we have been talking about.
> Would you please take a look at it?
>
> Regards,
> Kai
>
>
> rajdavies wrote:
>>
>> Sounds good - please send a patch!
>>
>>
>
> --
> View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17603539.html
> Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
>
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by khudalla <Ka...@de.bosch.com>.
Rob,
I have created JIRA issue AMQ-1760 and attached a patch implementing the
functionality we have been talking about.
Would you please take a look at it?
Regards,
Kai
rajdavies wrote:
>
> Sounds good - please send a patch!
>
>
--
View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17603539.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by Rob Davies <ra...@gmail.com>.
Sounds good - please send a patch!
On 2 Jun 2008, at 07:50, khudalla wrote:
>
> Rob,
>
> I see what you mean. I think that the
> "ClientInternalExceptionListener"
> should not be limited to JMSExceptions though, i.e. it should not
> implement
> javax.jms.ExceptionListener but instead an interface
> that provides an onException(Throwable exception) method, do you
> agree?
>
> If so, I will add this to ActiveMQConnection and send you a patch for
> review, okay?
>
> Kai
>
>
> rajdavies wrote:
>>
>> Hi Kai,
>>
>> I think you missed understood :)
>> I'm proposing to add another on method on Connection - e.g.
>> setClientInternalExceptionListener() - that registers an exception
>> listener of internal exceptions only - like in this
>> ActiveMQSession.run() case.
>>
>> We then only pass exceptions to the ExceptionListener registered by
>> setExceptionListener() that directly affect the connection.
>>
>> cheers,
>>
>> Rob
>>
>
> --
> View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17595013.html
> Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
>
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by khudalla <Ka...@de.bosch.com>.
Rob,
I see what you mean. I think that the "ClientInternalExceptionListener"
should not be limited to JMSExceptions though, i.e. it should not implement
javax.jms.ExceptionListener but instead an interface
that provides an onException(Throwable exception) method, do you agree?
If so, I will add this to ActiveMQConnection and send you a patch for
review, okay?
Kai
rajdavies wrote:
>
> Hi Kai,
>
> I think you missed understood :)
> I'm proposing to add another on method on Connection - e.g.
> setClientInternalExceptionListener() - that registers an exception
> listener of internal exceptions only - like in this
> ActiveMQSession.run() case.
>
> We then only pass exceptions to the ExceptionListener registered by
> setExceptionListener() that directly affect the connection.
>
> cheers,
>
> Rob
>
--
View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17595013.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by Rob Davies <ra...@gmail.com>.
Hi Kai,
I think you missed understood :)
I'm proposing to add another on method on Connection - e.g.
setClientInternalExceptionListener() - that registers an exception
listener of internal exceptions only - like in this
ActiveMQSession.run() case.
We then only pass exceptions to the ExceptionListener registered by
setExceptionListener() that directly affect the connection.
cheers,
Rob
On 30 May 2008, at 12:55, khudalla wrote:
>
> Rob,
>
> I do understand that you don't want to skip the mechanism in general
> in
> order
> to not break any existing code that relies on it.
> However, I am only suggesting to remove the
> connection.onAsyncException()
> call in the ActiveMQSession.run() method. The connection that it is
> called
> upon is the one the RA has created in order to receive inbound
> messages for
> MDBs, i.e. the exception listener is guaranteed to be the one
> registered by
> the RA. Thus, no custom code created by developers can be affected
> by this
> since such code cannot get a reference to this connection in order to
> register its own exception listener on the connection, right?
> Or are you suggesting that the ActiveMQSession.run() method is
> probably
> (mis-)used by developers for purposes other than the RA?
>
> Kai
>
>
> rajdavies wrote:
>>
>> Hi Kai,
>>
>> I agree with what you say - but the original reason we passed
>> exceptions is that some developers required to know if there was a
>> problem by this mechanism. ActiveMQ users in general don't try to
>> reconnect on a Connection.onException() - because they typically use
>> a fault tolerant failover connection (now the default transport).
>>
>> Its to support these users I'm suggesting that we add a supplementary
>> method to call an exception - and use Connection.onException() for
>> its
>> proper purpose :) If you don't register an exception listener on this
>> new method - it won't know anything about it - the exception will
>> just
>> get logged - which handles your case as well
>>
>> cheers,
>>
>> Rob
>>
>>
>
> --
> View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17557168.html
> Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
>
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by khudalla <Ka...@de.bosch.com>.
Rob,
I do understand that you don't want to skip the mechanism in general in
order
to not break any existing code that relies on it.
However, I am only suggesting to remove the connection.onAsyncException()
call in the ActiveMQSession.run() method. The connection that it is called
upon is the one the RA has created in order to receive inbound messages for
MDBs, i.e. the exception listener is guaranteed to be the one registered by
the RA. Thus, no custom code created by developers can be affected by this
since such code cannot get a reference to this connection in order to
register its own exception listener on the connection, right?
Or are you suggesting that the ActiveMQSession.run() method is probably
(mis-)used by developers for purposes other than the RA?
Kai
rajdavies wrote:
>
> Hi Kai,
>
> I agree with what you say - but the original reason we passed
> exceptions is that some developers required to know if there was a
> problem by this mechanism. ActiveMQ users in general don't try to
> reconnect on a Connection.onException() - because they typically use
> a fault tolerant failover connection (now the default transport).
>
> Its to support these users I'm suggesting that we add a supplementary
> method to call an exception - and use Connection.onException() for its
> proper purpose :) If you don't register an exception listener on this
> new method - it won't know anything about it - the exception will just
> get logged - which handles your case as well
>
> cheers,
>
> Rob
>
>
--
View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17557168.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by Rob Davies <ra...@gmail.com>.
Hi Kai,
I agree with what you say - but the original reason we passed
exceptions is that some developers required to know if there was a
problem by this mechanism. ActiveMQ users in general don't try to
reconnect on a Connection.onException() - because they typically use
a fault tolerant failover connection (now the default transport).
Its to support these users I'm suggesting that we add a supplementary
method to call an exception - and use Connection.onException() for its
proper purpose :) If you don't register an exception listener on this
new method - it won't know anything about it - the exception will just
get logged - which handles your case as well
cheers,
Rob
On 30 May 2008, at 12:11, khudalla wrote:
>
> Rob,
>
> thanks for your explanations. However, even if we had such an
> additional
> listener for this kind of exception, what would such a listener be
> supposed
> to do about it? The problem remains the same, i.e. we still do not
> know, how
> to handle the problem properly since it can have arbitrary reasons.
> In this
> particular case however, I don't see how "panicking" (i.e. re-
> connecting to
> the broker) would be of any help. Since the exception listener
> registered on
> the connection is the one registered by the RA (and the
> ActiveMQSession.run() method can be sure of that since the run()
> method is
> used by the RA only), I think we could just log the error and then
> silently
> ignore the exception, don't you think? In other cases (i.e. in the
> rest of
> the client code) the onAsyncException() strategy may be appropriate,
> however.
>
> Kai
>
>
> rajdavies wrote:
>>
>>
>> Hi Kai,
>>
>> Thanks for the feedback and good description of the problem! The
>> problem is that the only mechanism to notify of an internal async
>> exception is through the Connection Exception listener - and and we
>> use this for both internal client exceptions - and actual transport
>> exceptions. However, I agree that this is erroneous - according to
>> the
>> JMS API - onException() should only be called if there is a serious
>> problem with the Connection object itself.
>>
>> We do call this from multiple places in the client side code -
>> normally to notify that there is a runtime exception from the
>> container (as in this case) or the application when consuming a
>> message asynchronously.
>>
>> I think we need to add an additional method on the Connection for
>> registering an Exception listener for general client/application
>> exceptions
>>
>>
>> cheers,
>>
>> Rob
>>
>> http://open.iona.com/products/enterprise-activemq
>> http://rajdavies.blogspot.com/
>>
>>
>>
>>
>>
>>
>
> --
> View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17556456.html
> Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
>
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by khudalla <Ka...@de.bosch.com>.
Rob,
thanks for your explanations. However, even if we had such an additional
listener for this kind of exception, what would such a listener be supposed
to do about it? The problem remains the same, i.e. we still do not know, how
to handle the problem properly since it can have arbitrary reasons. In this
particular case however, I don't see how "panicking" (i.e. re-connecting to
the broker) would be of any help. Since the exception listener registered on
the connection is the one registered by the RA (and the
ActiveMQSession.run() method can be sure of that since the run() method is
used by the RA only), I think we could just log the error and then silently
ignore the exception, don't you think? In other cases (i.e. in the rest of
the client code) the onAsyncException() strategy may be appropriate,
however.
Kai
rajdavies wrote:
>
>
> Hi Kai,
>
> Thanks for the feedback and good description of the problem! The
> problem is that the only mechanism to notify of an internal async
> exception is through the Connection Exception listener - and and we
> use this for both internal client exceptions - and actual transport
> exceptions. However, I agree that this is erroneous - according to the
> JMS API - onException() should only be called if there is a serious
> problem with the Connection object itself.
>
> We do call this from multiple places in the client side code -
> normally to notify that there is a runtime exception from the
> container (as in this case) or the application when consuming a
> message asynchronously.
>
> I think we need to add an additional method on the Connection for
> registering an Exception listener for general client/application
> exceptions
>
>
> cheers,
>
> Rob
>
> http://open.iona.com/products/enterprise-activemq
> http://rajdavies.blogspot.com/
>
>
>
>
>
>
--
View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17556456.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
Re: Handling of RuntimeExceptions in ActiveMQSession.run() method
Posted by Rob Davies <ra...@gmail.com>.
On 30 May 2008, at 09:53, khudalla wrote:
>
> Hi,
>
> I have stumbled across the following code in the ActiveMQSession.run()
> method:
>
> try {
> messageListener.onMessage(message);
> } catch (Throwable e) {
> // TODO: figure out proper way to handle error.
> LOG.error("error dispatching message: ", e);
> connection.onAsyncException(e);
> }
>
> As the TODO points out, the way how to handle exceptions properly
> needs some
> discussion. I have come across this while working on the Resource
> Adapter in
> conjunction with GlassFish v2. When I use batching when delivering
> messages
> to an MDB and the MDB marks the current TX as rollback only (e.g. if
> it
> cannot access a database), any subsequent invocation of the MDB's
> onMessage() method (actually the invocation of the wrapper around that
> method provided by the GlassFish app server) in the same TX will
> throw a
> javax.ejb.TransactionRolledbackLocalException in order to indicate
> that it
> is futile to invoke the bean since the TX will be rolled back
> anyway. This
> RuntimeException will now lead to the catch block being executed,
> i.e. the
> connection's ExceptionListener will be notified eventually which in
> this
> case is the listener that the RA has registered on the connection
> which in
> turn will tear down the connection and reconnect to the broker.
> However, the
> connection never failed in the first place, i.e. reconnecting to the
> broker
> is not necessary at all.
>
> I wonder whether the ActiveMQSession.run() method should better
> ignore any
> RuntimeException thrown by the MessageListener (i.e. do not invoke
> connection.onAsyncException()) since these exceptions generally
> indicate a
> problem with the processing of the message, not with receiving the
> message.
>
> Any thoughts?
>
> Kai
>
> --
> View this message in context: http://www.nabble.com/Handling-of-RuntimeExceptions-in-ActiveMQSession.run%28%29-method-tp17554360s2354p17554360.html
> Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
>
Hi Kai,
Thanks for the feedback and good description of the problem! The
problem is that the only mechanism to notify of an internal async
exception is through the Connection Exception listener - and and we
use this for both internal client exceptions - and actual transport
exceptions. However, I agree that this is erroneous - according to the
JMS API - onException() should only be called if there is a serious
problem with the Connection object itself.
We do call this from multiple places in the client side code -
normally to notify that there is a runtime exception from the
container (as in this case) or the application when consuming a
message asynchronously.
I think we need to add an additional method on the Connection for
registering an Exception listener for general client/application
exceptions
cheers,
Rob
http://open.iona.com/products/enterprise-activemq
http://rajdavies.blogspot.com/