Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dnwe opened this issue Apr 21, 2017 · 1 comment
Open

Comments

@dnwe
Copy link
Contributor

dnwe commented Apr 21, 2017

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
Copy link
Collaborator

@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.

dnwe added a commit to dnwe/node-amqp10 that referenced this issue May 8, 2017
- default to rcv-settle-mode "first" everywhere, we don't currently handle
  "second" correctly anyway
- add link policy attribute "settlement" to distinguish between auto,
  where the client will immediately accept messages on receipt, and manual
  where the user is expected to call accept when they have finished
  handling a delivery.

Fixes noodlefrenzy#316
dnwe added a commit to dnwe/node-amqp10 that referenced this issue May 8, 2017
- default to rcv-settle-mode "first" everywhere, we don't currently handle
  "second" correctly anyway
- add link policy attribute "settlement" to distinguish between auto,
  where the client will immediately accept messages on receipt, and manual
  where the user is expected to call accept when they have finished
  handling a delivery.

Fixes noodlefrenzy#316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants