Qpid
  1. Qpid
  2. QPID-2657

Verify/correct error handling on client 0-10 codepath

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.7
    • Fix Version/s: 0.7
    • Component/s: None
    • Labels:
      None

      Description

      Verify and correct existing error handling on 0-10 CLIENT codepath:

      • whats the structure and how does it compare with whats there on the 0-9 codepath ?
      • do errors thrown to the client from the broker have the correct result on an open connection/session/producer/consumer ?
      • is logging correctly implemented ?

        Activity

        Hide
        Andrew Kennedy added a comment -

        Updated with fixes for close race condition.

        Show
        Andrew Kennedy added a comment - Updated with fixes for close race condition.
        Hide
        Rajith Attapattu added a comment -

        First of all, thanks for doing this.
        I have been meaning to do this for a while now.

        I have the following questions about the patch.
        In @@ -139,36 +139,24 @@ public class BasicMessageConsumer_0_10 ... you have added the following code.

        + if (isMessageListenerSet() && capacity == 0)
        +

        { + _0_10session.getQpidSession().messageFlow(getConsumerTagString(), + MessageCreditUnit.MESSAGE, 1, + Option.UNRELIABLE); + }

        + _logger.debug("messageOk, trying to notify");
        + super.notifyMessage(jmsMessage);

        1. What is the reason for adding this?

        2. How is this related to exception handling? (or is this part of some other JIRA that got accidentally added to this patch ?)

        3. Have you run the tests against the 0-10 cpp broker profiles?

        In general If any code is modified in the 0-10 client side, I'd like to kindly request that the 0-10 cpp broker test profiles are run before the patch is committed.

        Show
        Rajith Attapattu added a comment - First of all, thanks for doing this. I have been meaning to do this for a while now. I have the following questions about the patch. In @@ -139,36 +139,24 @@ public class BasicMessageConsumer_0_10 ... you have added the following code. + if (isMessageListenerSet() && capacity == 0) + { + _0_10session.getQpidSession().messageFlow(getConsumerTagString(), + MessageCreditUnit.MESSAGE, 1, + Option.UNRELIABLE); + } + _logger.debug("messageOk, trying to notify"); + super.notifyMessage(jmsMessage); 1. What is the reason for adding this? 2. How is this related to exception handling? (or is this part of some other JIRA that got accidentally added to this patch ?) 3. Have you run the tests against the 0-10 cpp broker profiles? In general If any code is modified in the 0-10 client side, I'd like to kindly request that the 0-10 cpp broker test profiles are run before the patch is committed.
        Hide
        Rajith Attapattu added a comment -

        Ah my bad, there was a local modification in my code when the patch was applied, and due to that I misunderstood that particular code segment.
        I failed to realize that you had merely rearranged the code there.

        Btw question (3) still stands

        Show
        Rajith Attapattu added a comment - Ah my bad, there was a local modification in my code when the patch was applied, and due to that I misunderstood that particular code segment. I failed to realize that you had merely rearranged the code there. Btw question (3) still stands

          People

          • Assignee:
            Robbie Gemmell
            Reporter:
            Andrew Kennedy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development