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

          Hide
          Robbie Gemmell added a comment -

          patch applied

          Show
          Robbie Gemmell added a comment - patch applied
          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
          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?
          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.
          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?
          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
          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?
          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.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development