search

noodlefrenzy / node-amqp10

amqp10 is a promise-based, AMQP 1.0 compliant node.js client
MIT License
134 stars 56 forks source link

Settle mode policy configuration broken #313

Closed princjef closed 7 years ago

princjef commented 7 years ago

Starting in version 3.5.1 (specifically this commit), the receiverSettleMode and senderSettleMode policy configuration options no longer set the receiver and sender settle modes correctly, causing them to fall back to their default values.

It makes sense to support the names from the AMQP spec (rcvSettleMode and sndSettleMode), but we should at least internally translate values under the old names to the new ones until the next major version to maintain backwards compatibility for custom policies

mbroadst commented 7 years ago

@princjef sorry this maybe should have been a major bump. We encountered the issues in #312, and I was trying to correct some sins of the past. I'll commit a fix supporting the old names. What's unclear to me on initial inspection is how this worked in the first place, as the frame definition has a field for sndSettleMode and recvSettleMode but not the other names..

princjef commented 7 years ago

Yeah I'm not sure about rcvSettleMode, since service bus at least seemed to accept the other form. As for sndSettleMode, I'm thinking that it just fell back to the default, since I work with mixed mode, but never overrode the value in the policy, which is settled

princjef commented 7 years ago

Either way, the change in 3.5.2 looks good. Thanks for the quick fix