postwait / node-amqp

[UNMAINTAINED] node-amqp is an AMQP client for nodejs
MIT License
1.69k stars 357 forks source link

Publisher confirm not working #225

Open kevinohara80 opened 11 years ago

kevinohara80 commented 11 years ago

I have an API process that I am attempting to do the following:

  1. Accept JSON payload from the API client
  2. Publish the JSON to RabbitMQ and queue it for work
  3. Receive a Publisher Confirm from RabbitMQ
  4. 202 back to the client when the job is queued (not processed by a worker)

Everything works for me except the publisher confirm. The publish succeeds, I see it hit the queue, and a worker picks it up. My API process never gets a publisher confirm despite setting confirm:true.

I've tried setting it up using both the callback method and the EventEmitter method. The callback never fires and I am never able to receive an ack event message from the publish.

I've looked at the examples in the tests and I am setting them up in the same way. Is there a bug here or am I missing something? I'm using amqp version 0.1.7.

Here is my code using the EE method:

app.post('/test/queue', auth, function(req, res) {
  var msg = { body: req.body };
  var opts = { mandatory: true };

  connection.exchange('test-exc-1', { type: 'direct', confirm: true }, function(exchange) {
    console.log('connected to exchange');

    console.log('publishing...');
    var publish = exchange.publish('', msg, opts);

    publish.addListener('ack', function(data) {
      console.log('publish :: ack');
      return res.json(202, { status: 'ok', message: 'your test message was queued' });
    });

    publish.addListener('error', function(err) {
      console.log('publish :: error :: ' + err);
      return res.json(500, { status: 'error', message: 'unable to queue your message' });
    });

  });

});

And here it is using the callback method:

app.post('/test/queue', auth, function(req, res) {
  var msg = { body: req.body };
  var opts = { mandatory: true };

  connection.exchange('test-exc-1', { type: 'direct', confirm: true }, function(exchange) {
    console.log('connected to exchange');

    console.log('publishing...');
    var publish = exchange.publish('', msg, opts, function(err) {
      if(err) res.json(500, { status: 'error', message: 'unable to queue your message' });
      else res.json(202, { status: 'ok', message: 'your test message was queued' });
    });

  });

});
shernshiou commented 11 years ago

+1

cngan commented 11 years ago

Try to amend the 'amqp' dependency in package.json with "https://codeload.github.com/postwait/node-amqp/tar.gz/master".

looksgood commented 11 years ago

+1

glenjamin commented 11 years ago

I've just run into this, looks like there's an issue in the code to do with comparison of 64bit integers.

This seems to be "fixed" in master by casting the value down to a 32 bit for comparison - I guess as long as you don't try to confirm more than MAXINT messages this will work.

Any idea when there might be another release to npm, anything I can do to help?

lky1001 commented 10 years ago

can you add confirm : true option? var opts = { mandatory: true, confirm : ture };

bernardmo commented 10 years ago

Hi, any help with this? I'm using already declared exchange - CB function is never called but data is received in exchange.

var exchangeConn = amqp.createConnection( { url: connString } );
exchangeConn.on( 'ready', function() {

    exchangeConn.exchange( 'MyExchange', { noDeclare: true, confirm: true }, function( exchange ) {

        exchange.publish( '', 'Some data', { mandatory: true }, function( error ) {

            // never in, but data got in exchange
        });
    });
});
testower commented 10 years ago

I can confirm that the confirm: true option does not help. The confirm callback on exchange.publish is never called.

glenjamin commented 10 years ago

Can you confirm what client & server versions you are seeing this behaviour on?

testower commented 10 years ago

In my case

amqp 0.9.1 RabbitMQ 3.0.4 node-amqp 0.2.0

barshow commented 10 years ago

In the rabbit management interface client you can check the the channel you're publishing with is in confirm mode. If its not thats You can also try https://github.com/dropbox/amqp-coffee if you're writing a new app it may be easier, if not the api changes may make it tough.

testower commented 10 years ago

Where would that be indicated in the interface exactly?

barshow commented 10 years ago

localhost:15672/#/channels there should be a "c" int he Mode column. That indicates that publish confirms are enabled on that channel.

testower commented 10 years ago

Thanks for that tip. As for amqp-coffee, that looks interesting. I don't know much coffee, does it require me to be using coffee? And can I use multiple callbacks for parallelll queue bindings? If so, I might just switch :-)

barshow commented 10 years ago

You don't have to be using coffee script, to run amqp-coffee. You can use multiple in parallel, for different publishes. But that should also work for node-amqp.

testower commented 10 years ago

As per #298, queue.bind will overwrite the callback for each call. So any bind operations going on at the same time, only the last callback passed in will be called, rather than each individual callback being called its bind completes.

testower commented 10 years ago

Back on-topic. Using:

            {
                noDeclare: true,
                confirm: true
            },

Does not seem to put channel in confirm mode:

screenshot 2014-04-14 21 57 19

The mode column is the third column

glenjamin commented 10 years ago

The code at https://github.com/postwait/node-amqp/blob/master/lib/exchange.js certainly looks like it should respect the confirm option, however it appears that when used with noDeclare that code path isn't hit.

bernardmo commented 10 years ago

@glenjamin - Versions: node-amqp 0.2.1 (also tried pulling master from git - doesn't work) RabbitMQ 3.2.4

@davidmba - channel doesn't show "c" option in mode column like @testower says

barshow commented 10 years ago

https://github.com/postwait/node-amqp/blob/master/lib/exchange.js#L91 only happens after exchangeDeclareOk which doesn't happen with no-declare. This is for sure a bug. You can just not use no-declare. You should be able to redeclare a exchange with no problem. Ill plug https://github.com/dropbox/amqp-coffee again, with it you don't publish on a exchange. You publish on the connection which solves the whole no-declare thing.

bernardmo commented 10 years ago

So an "ugly" fix like https://github.com/postwait/node-amqp/pull/328 works for me

jensbjork commented 10 years ago

+1

bernardmo commented 10 years ago

@jensbjork pull the latest version from github, npm installation doesn't shift changes from #328

jensbjork commented 10 years ago

I've specified the git repo through npm as well but the problem still remains. What was the conclusion around this issue if not only to update to the latest repo?

All I really want to do is to close the connection when publishing to a queue. Simply calling connection.disconnect() outside a callback does not seem to terminate the connection, it is still visible as a running connection in rabbitmq.

  var raw_text          = req.param('text');
  var from_identifier   = req.param('msisdn');
  var timestamp         = req.param('message-timestamp');

  if (raw_text && from_identifier && timestamp) {

    app.conn = amqp.createConnection({
      host: 'localhost',
      login: '{{ rabbitmq_username }}',
      password: '{{ rabbitmq_password }}',
      connectionTimeout: 30000
    });

    app.conn.on('ready', function() {
      console.log("Connected to RabbitMQ");

      app.e = app.conn.exchange('amq.fanout', { confirm: true });
      app.q = app.conn.queue('events');

      app.e.publish('', { raw_text: raw_text, from_identifier: from_identifier, timestamp: timestamp }, {}, function(status) {
        console.log("Sent message and disconnecting");
        app.conn.disconnect();
      });

    });
  }

Are there any alternative methods that can be used in order to achieve this? All input is highly appriciated!

chevett commented 10 years ago

This appears to work for me.

rabbit: 3.2.4 node-amqp: both 2.0 and master node: v0.10.31

I can run sudo rabbitmqctl list_channels name confirm and I see that my channels are in confirm mode.

soichih commented 9 years ago

+1

Having the same problem on.

RabbitMQ: 3.5.1 node-amqp: 0.2.4 nodejs: v0.10.36

I am not using noDeclare option, so I am not sure how to go around this problem..

conn.exchange('topic-test', {type: 'topic', confirm: true}, function(ex) {...}