[jira] Issue Comment Edited: (AMQ-2812) ActiveMqSession should throw JMSException instead of IllegalStateException if connection is closed

["Morten Andersen-Gott (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=60499#action_60499 ] 

Morten Andersen-Gott edited comment on AMQ-2812 at 7/8/10 2:52 AM:
-------------------------------------------------------------------

I'm arguing that throwing a JMSException would be a better option, according to the spec. Also, it breaks the JMS integration in Spring. This is the code from Spring's JmsResourceHolder:

	public void commitAll() throws JMSException {
		for (Session session : this.sessions) {
			try {
				session.commit();
			}
			catch (TransactionInProgressException ex) {
				// Ignore -> can only happen in case of a JTA transaction.
			}
			catch (javax.jms.IllegalStateException ex) {
				// Ignore -> can only happen in case of a JTA transaction.
			}
		}
	}

As you can see, IllegalStateException is ignored, while the vanilla is thrown to the caller. Reading the javax.jms.Session javadoc as well as the JMS specification I'd say that Spring's ignoring of IllegalStateException is perfectly reasonable. 

      was (Author: magott):
    I'm arguing that throwing a JMSException would be a better option, according to the spec. Also, it breaks the JMS integration in Spring. This is the code from Spring's JmsResourceHolder:

	public void commitAll() throws JMSException {
		for (Session session : this.sessions) {
			try {
				session.commit();
			}
			catch (TransactionInProgressException ex) {
				// Ignore -> can only happen in case of a JTA transaction.
			}
			catch (javax.jms.IllegalStateException ex) {
				// Ignore -> can only happen in case of a JTA transaction.
			}
		}
	}

As you can see, IllegalStateException is ignored, while the vanilla is thrown to the caller.
  
> ActiveMqSession should throw JMSException instead of IllegalStateException if connection is closed
> --------------------------------------------------------------------------------------------------
>
>                 Key: AMQ-2812
>                 URL: https://issues.apache.org/activemq/browse/AMQ-2812
>             Project: ActiveMQ
>          Issue Type: Bug
>    Affects Versions: 5.3.2
>         Environment: Spring 2.5.6
>            Reporter: Morten Andersen-Gott
>            Assignee: Rob Davies
>            Priority: Critical
>             Fix For: 5.4.0
>
>
> ActiveMqSession throws IllegalStateException if connection is closed. The ActiveMqSession.checkClosed() will throw the exception if the connection is lost. This is really an internal error which according to the javax.jms.Session.commit javadoc should result in a JMSException.
> The distinction between the two exceptions becomes important when using Spring and ActiveMq. When doing JMS inside a codeblock that is managed by a HibernateTransactionManager (or any other non-JmsTransactionManager/non-JtaTransactionManager) TransactionSynchronizationUtils will try to commit the jms session after the HibernateTransactionManager has committed. This is done by calling the JmsResourceHolder.commitAll() from TransactionSynchronizationManager.invokeAfterCommit(). JmsResourceHolder.commitAll() will silently ignore IllegalStateExceptions as these are only supposed to happen in the case of a JTA transaction.
> If ActiveMqSession.commit -> ActiveMqSession.checkClosed() were to throw a JMSException it would propagate out of the TransactionSynchronizationUtils and at least the application would be made aware that the JMSSession was not committed.
> Below is the description of IllegalStateException from the JMS Specification:
> IllegalStateException: This exception is thrown when a method is invoked a
> an illegal or inappropriate time or if the provider is not in an appropriate 
> state for the requested operation. For example, this exception must be 
> thrown if Session.commit() is called on a non-transacted session. This 
> exception is also must be called when domain inappropriate method is 
> called, such as calling TopicSession.CreateQueueBrowser().
> While it does not specifically state that a closed connection should _NOT_ throw an IllegalStateException, it would seem that throwing a JMSException is a better choice. Especially when IllegalStateException causes problems when using Spring.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.