Qpid
  1. Qpid
  2. QPID-3448

JMS Client throws unchecked SessionExceptions to client application

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.10, 0.11, 0.12
    • Fix Version/s: 0.13
    • Component/s: Java Client
    • Labels:
      None

      Description

      On the 0-10 code path, there are instances where the client application receives an unchecked Qpid SessionException rather than a JMSException as mandated by the JMS Specification. The client application should not be forced to catch 'internal' Qpid exceptions.

      For example if a javax.jms.Session#commit takes longer than 60 seconds, the client receives:

      org.apache.qpid.transport.SessionException: timed out waiting for sync: complete = 12, point = 15
              at org.apache.qpid.transport.Session.sync(Session.java:798)
              at org.apache.qpid.transport.Session.sync(Session.java:772)
              at org.apache.qpid.transport.Session.invoke(Session.java:732)
              at org.apache.qpid.transport.Session.invoke(Session.java:561)
              at org.apache.qpid.transport.SessionInvoker.txCommit(SessionInvoker.java:148)
              at org.apache.qpid.client.AMQSession_0_10.sendCommit(AMQSession_0_10.java:423)
              at org.apache.qpid.client.AMQSession_0_10.commit(AMQSession_0_10.java:1008)
      

      For comparison, 0-8..-0-9-1 throws a JMSException with an underlying of AMQTimeoutException.

        Issue Links

          Activity

          Keith Wall created issue -
          Hide
          Alex Rudyy added a comment -

          The patch attached ads try-catch blocks where possible to catch SessionException and convert it into AMQException or JMSException.

          Show
          Alex Rudyy added a comment - The patch attached ads try-catch blocks where possible to catch SessionException and convert it into AMQException or JMSException.
          Alex Rudyy made changes -
          Field Original Value New Value
          Attachment QPID-3448-Prevent-throwing-of-SessionExceptions.patch [ 12492267 ]
          Hide
          Alex Rudyy added a comment -

          Robbie, could you please review an attached patch?

          Show
          Alex Rudyy added a comment - Robbie, could you please review an attached patch?
          Alex Rudyy made changes -
          Assignee Robbie Gemmell [ gemmellr ]
          Alex Rudyy made changes -
          Attachment QPID-3448-Prevent-throwing-of-SessionExceptions.patch [ 12492267 ]
          Hide
          Alex Rudyy added a comment -

          Patch is updated due to recent changes

          Show
          Alex Rudyy added a comment - Patch is updated due to recent changes
          Alex Rudyy made changes -
          Attachment QPID-3448-Prevent-throwing-of-SessionExceptions.patch [ 12492597 ]
          Alex Rudyy made changes -
          Assignee Robbie Gemmell [ gemmellr ] Alex Rudyy [ alex.rufous ]
          Alex Rudyy made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Alex Rudyy made changes -
          Status In Progress [ 3 ] Ready To Review [ 10006 ]
          Hide
          Alex Rudyy added a comment -

          Robbie, could you please review patch attached to this JIRA?

          Show
          Alex Rudyy added a comment - Robbie, could you please review patch attached to this JIRA?
          Alex Rudyy made changes -
          Assignee Alex Rudyy [ alex.rufous ] Robbie Gemmell [ gemmellr ]
          Hide
          Robbie Gemmell added a comment -

          I think the changes should really be made at the entry points to AMQSession mandated by the JMS interfaces, i.e the places we should already be declaring we throw JMSException. Whilst some of the instnaces may be necessary, there seems to be far too much additional try-catching being added within the general implementation methods in AMQSession_0_10 etc which are used to support the JMS methods. The conversion of SessionExceptions to AMQExceptions in several places also doesnt seem that desirable given they are then going to be wrapped in JMSExceptions anyway.

          Making the underlying AMQP/transport related exceptions checked exceptions rather than runtime exceptions could be worthwhile, since it would increase their visibility and they are often things you should have to explicitly choose to ignore rather than unexpected exceptions you cant/shouldnt reasonably expect to handle within the mesaging library, making them seem more suited to checked exceptions.

          Show
          Robbie Gemmell added a comment - I think the changes should really be made at the entry points to AMQSession mandated by the JMS interfaces, i.e the places we should already be declaring we throw JMSException. Whilst some of the instnaces may be necessary, there seems to be far too much additional try-catching being added within the general implementation methods in AMQSession_0_10 etc which are used to support the JMS methods. The conversion of SessionExceptions to AMQExceptions in several places also doesnt seem that desirable given they are then going to be wrapped in JMSExceptions anyway. Making the underlying AMQP/transport related exceptions checked exceptions rather than runtime exceptions could be worthwhile, since it would increase their visibility and they are often things you should have to explicitly choose to ignore rather than unexpected exceptions you cant/shouldnt reasonably expect to handle within the mesaging library, making them seem more suited to checked exceptions.
          Robbie Gemmell made changes -
          Status Ready To Review [ 10006 ] Open [ 1 ]
          Assignee Robbie Gemmell [ gemmellr ] Alex Rudyy [ alex.rufous ]
          Alex Rudyy made changes -
          Attachment QPID-3448-Prevent-throwing-of-SessionExceptions.patch [ 12492597 ]
          Alex Rudyy made changes -
          Attachment QPID-3448-Prevent-throwing-of-SessionExceptions.patch [ 12494012 ]
          Alex Rudyy made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Alex Rudyy made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Alex Rudyy made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Alex Rudyy made changes -
          Status In Progress [ 3 ] Ready To Review [ 10006 ]
          Hide
          Alex Rudyy added a comment -

          Robbie,
          Could you please review an updated patch with problem fix?

          Show
          Alex Rudyy added a comment - Robbie, Could you please review an updated patch with problem fix?
          Alex Rudyy made changes -
          Assignee Alex Rudyy [ alex.rufous ] Robbie Gemmell [ gemmellr ]
          Alex Rudyy made changes -
          Attachment QPID-3448-Prevent-throwing-of-SessionExceptions.patch [ 12494012 ]
          Hide
          Alex Rudyy added a comment -

          Uploaded a new patch from Robbie and me

          Show
          Alex Rudyy added a comment - Uploaded a new patch from Robbie and me
          Alex Rudyy made changes -
          Hide
          Robbie Gemmell added a comment -

          patch applied

          Show
          Robbie Gemmell added a comment - patch applied
          Robbie Gemmell made changes -
          Status Ready To Review [ 10006 ] Resolved [ 5 ]
          Fix Version/s 0.13 [ 12316854 ]
          Resolution Fixed [ 1 ]
          Keith Wall made changes -
          Link This issue is related to QPID-3339 [ QPID-3339 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Reviewable Reviewable Open Open
          1d 9h 46m 1 Robbie Gemmell 07/Sep/11 22:03
          In Progress In Progress Open Open
          10s 1 Alex Rudyy 12/Sep/11 10:50
          Open Open In Progress In Progress
          18d 14h 3m 3 Alex Rudyy 12/Sep/11 10:50
          In Progress In Progress Reviewable Reviewable
          5s 2 Alex Rudyy 12/Sep/11 10:50
          Reviewable Reviewable Resolved Resolved
          1d 4h 54m 1 Robbie Gemmell 13/Sep/11 15:44

            People

            • Assignee:
              Robbie Gemmell
              Reporter:
              Keith Wall
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development