postwait / node-amqp

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

basic.return get message which failed #335

Open funston opened 10 years ago

funston commented 10 years ago

reading the amqp doc, and looking at the code, delivering a mandatory:true message on a queue that has no consumer. is it to correct to assume the following (psuedo code):

msg_failed = false; exchange.on('basic.return',function(){ msg_failed = true; });

exchange.on("basic.ack',function(ack){ if (msg_failed === true){ sequence_number = args.deliveryTag.readUInt32BE(4); // this is the message sequence that failed });

success_seq = exchange.publish("happy_queue", {foo:'bar'}, {mandatory:true}).sequence; failed_seq = exchange.publish("sad_queue", {foo:'bar'}, {mandatory:true}).sequence;

i'm seeing this work, in general, but not under load, with millions of messages. the sequence that comes in the next ack after a basic.return is in fact a message that was delivered and consumed by the happy queue. thanks.

s-ol commented 7 years ago

this is an oversight in node-amqp I think, in this ruby lib the payload is available for example:

A returned message handler has access to AMQP method (basic.return) information, message metadata and payload (as a byte array). The metadata and message body are returned without modifications so that the application can store the message for later redelivery.

in exchange.js:179 the message should be retrieved just like in the basicAck branch above and passed on to the callback IMO. I'm probably going to write a PR for this now. EDIT: dropped the meteor package wrapping this lib and switched over to amqp.node which we are using in another codebase already.