You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "Morten Andersen-Gott (JIRA)" <ji...@apache.org> on 2010/07/07 14:31:51 UTC

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

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
            Priority: Critical


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.


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

Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=60503#action_60503 ] 

Rob Davies commented on AMQ-2812:
---------------------------------

Unfortunately the JMS API says this: Invoking any other Session method on a closed session must throw a JMSException.IllegalStateException. Closing a closed session must not throw an exception - see http://download.oracle.com/docs/cd/E17477_01/javaee/1.4/api/javax/jms/Session.html#close()
 Looks like this is a bug in Spring ...

> 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.


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

Posted by "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 commented on AMQ-2812:
-------------------------------------------

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.


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

Posted by "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=60591#action_60591 ] 

Morten Andersen-Gott commented on AMQ-2812:
-------------------------------------------

sounds good. the issue that lead to this jira being opened has been fixed in spring.

> 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.


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

Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rob Davies reopened AMQ-2812:
-----------------------------

    Regression:   (was: [Regression])

oh, alright then - will fix in 5.4 ;)

> 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.


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

Posted by "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=60505#action_60505 ] 

Morten Andersen-Gott commented on AMQ-2812:
-------------------------------------------

Raised issue with Spring http://jira.springframework.org/browse/SPR-7360

> 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.


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

Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rob Davies resolved AMQ-2812.
-----------------------------

    Fix Version/s: 5.4.0
       Resolution: Won't Fix

The ActiveMQSession actually throws a javax.jms.IllegalStateException if the session is closed when a commit() is called - which is correct. As javax.jms.IllegalStateException is derived from javax.jms.JMSException  -I'm not sure how changing it to a vanilla JMSException will fix your problem 

> 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.


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

Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rob Davies resolved AMQ-2812.
-----------------------------

    Resolution: Won't Fix

Going to resolve this issue - unless there is further feedback on the correct exception to be thrown when accessing a method on a Session when the Session is closed

> 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.


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

Posted by "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=60502#action_60502 ] 

Morten Andersen-Gott commented on AMQ-2812:
-------------------------------------------

Fantastic! Thanks for listening! :-)

> 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.


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

Posted by "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=60504#action_60504 ] 

Morten Andersen-Gott commented on AMQ-2812:
-------------------------------------------

hmm...close() is a strange place to document IllegalStateException behavior for other methods. but it sure seems like you are right. I'll raise it with Spring and add the spring issue number here.

> 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.


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

Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rob Davies reassigned AMQ-2812:
-------------------------------

    Assignee: Rob Davies

> 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
>
> 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.


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

Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/AMQ-2812?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=60506#action_60506 ] 

Rob Davies commented on AMQ-2812:
---------------------------------

thanks Morten!!

> 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.


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

Posted by "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.