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

Fix nack method on frame object #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

chepike
Copy link

@chepike chepike commented Apr 11, 2014

For the NACK method you must specify the destination queue instead of
the subscription id.

For the NACK method you must specify the destination queue instead of
the subscription id.
@jmesnil
Copy link
Owner

jmesnil commented Apr 11, 2014

That patch is not correct. ACK and NACK requires only to have the message id in STOMP 1.2.
The subscription header is also sent to be compatible with STOMP 1.1 but there is no need to pass the destination.

Did you encounter an issue related to this with a STOMP broker?

@chepike
Copy link
Author

chepike commented Apr 11, 2014

Yes, I did, I had problems sending the NACK, the server indicated that it was consumed. However, by modifying the parameter using destination rather than subscription, if I receive the message and the server marks it as pending message, and the message remains in the queue.

Server: Apache ActiveMQ 5.7.0
STOMP Version: 1.1

var url = "ws://localhost:61614/";
var client = Stomp.client(url);

var connectCallback = function() {
var subscription = null;
var onMessageCallback = function(frame) {
if (frame.body == "Foo")
frame.ack();
else if (frame.body == "Bar") {
frame.nack();
/*
subscription.unsubscribe();
setTimeout(function () {
subscription = client.subscribe('testClient', function (framePending) {
framePending.ack();
client.disconnect();
}, {ack: 'client'});
}, 1000);
*/
}
};
subscription = client.subscribe('testClient', onMessageCallback, {ack: 'client'});
client.send('testClient', {}, 'Foo');
setTimeout(function () {
client.send('testClient', {}, 'Bar');
}, 1000);
};

client.connect({}, connectCallback);

@chepike
Copy link
Author

chepike commented Apr 11, 2014

For an ACK must be sent subscriber identifier, but for an NACK must be sent the name of the queue. Both in the same 'subscription' parameter of the 'header' object.

@jmesnil
Copy link
Owner

jmesnil commented Apr 11, 2014

ACK and NACK frame format has changed between STOMP 1.1 (that is used by your version of ActiveMQ) and STOMP 1.2 (supported in the latest version).

In STOMP 1.1, NACK must have a message-id header corresponding to the message's message-id and a subscription header set to the subscription's id header.

In STOMP 1.2, NACK only need a id header corresponding to the value of the message's ack header.

stomp.js code is not correct for both STOMP 1.2 and STOMP 1.1.
To have it work for both versions, it should be modified to also look at the message's message-id and subscription and add them (that will make the ACK and NACK frames compatible with both STOMP 1.1 and 1.2).

In any case, the destination header is not required by the ACK/NACK frame in either version. If ActiveMQ requires it, that a bug in their implementation.

@jmesnil
Copy link
Owner

jmesnil commented Apr 11, 2014

Could you try your application with https://github.com/jmesnil/stomp-websocket/blob/stomp_1.1_ack/lib/stomp.js ?

With this commit (8538106) I have verified that ack & nack frames are working with both STOMP 1.1 and 1.2 with ActiveMQ 5.9.0.

@chepike
Copy link
Author

chepike commented Apr 11, 2014

Still not working, and the frame object is to acknowledge is:
{
content-length: "3"
destination: "/queue/testClient"
expires: "0"
message-id: "ID:SocieproJGPC-12182-1397242398606-2:3:-1:1:2"
priority: "4"
subscription: "sub-0"
timestamp: "1397242559834"
}

@timgriffin
Copy link

I'm digging up an old one here but @jmesnil or @chepike did you ever get further with this?

I'm on ActiveMQ 5.9.0 and I am having similar issues to others. I've tried the 8538106 commit and replaced stomp.js from v2.3.3 to no avail.

Below is a snippet of my code - basically it subscribes, successfully gets a message and non-acknowledges all messages, but the message does not stay on the queue.

https://gist.github.com/timgriffin/f541366082e88c52947a121145a8f951

I am connecting over TCP if that makes a difference (var client = Stomp.overTCP(....)).

Any help would be appreciated!

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

Successfully merging this pull request may close these issues.

3 participants