noodlefrenzy / node-amqp10

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

rcvSettleMode is mapping `first` => `autoSettle` and `second` => `manually settle` #316

Open dnwe opened 7 years ago

dnwe commented 7 years ago

Currently the amqp10 module seems to have re-purposed AMQP's rcv-settle-mode on a receiver link to determine whether or not the client library itself should be doing automatic settlement when messages received, or whether to expect the user to call .accept() to settle received messages.

However, I think this is incorrect. The rcv-settle-mode FIRST vs SECOND is specifically governing the contract between sender and receiver link as to who MUST send the settlement disposition first.

From the spec

rcv-settle-mode If first, this indicates that the Receiver MUST settle the delivery once it has arrived without waiting for the Sender to settle first. If second, this indicates that the Receiver MUST NOT settle until sending its disposition to the Sender and receiving a settled disposition from the sender. If not set, this value is defaulted to the value negotiated on link attach. If the negotiated link value is first, then it is illegal to set this field to second. If the message is being sent settled by the Sender, the value of this field is ignored. The (implicit or explicit) value of this field does not form part of the transfer state, and is not retained if a link is suspended and subsequently resumed.

Currently the 'SECOND' behaviour isn't implemented by the module, as per:

ReceiverLink.prototype._dispositionReceived = function(details) {
  debug('not yet processing "receiver" disposition frames: ', details);
};

I think for now the right thing would be to permanently lock rcv-settle-mode to FIRST to have AMQP.Constants.receiverSettleMode.autoSettle and AMQP.Constants.receiverSettleMode.manualSettle to simply distinguish whether or not the user plans to settle messages themselves.

mbroadst commented 7 years ago

@dnwe hm yeah you're right (man those are killer names for the two modes 'first' and 'second' :) ). Though it wouldn't take too much effort to track messages and settle them upon disposition (for the second case), it's not clear whether anyone is actually using that because nobody has complained of the aforementioned debug messages on ReceiverLink.prototype._dispositionReceived.