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

channel.publish callback is not called #220

Closed
shayts7 opened this issue Jan 18, 2016 · 12 comments
Closed

channel.publish callback is not called #220

shayts7 opened this issue Jan 18, 2016 · 12 comments

Comments

@shayts7
Copy link

shayts7 commented Jan 18, 2016

I'm using regular channel (not ConfirmChannel), when calling publish with callback - the callback is not being called at all.
Then I used the return value, but even if the RabbitMQ is down the return value is still true, the only thing that stops the publishing is "onError" event being called on the connection.

The Question is: why publish on regular channel does not support callback properly that will return err when the RabbitMQ is down for instance.

@Kurtas
Copy link
Contributor

Kurtas commented Jan 21, 2016

Same problem, publish method always returns Boolean instead Promise. I tried to use both type of channel.

@squaremo
Copy link
Collaborator

#publish on either kind of channel mimics the Stream interface, and returns true to indicate you can write again, or false to indicate you should wait until it emits a 'drain' event.

Only the ConfirmChannel will do anything with the callback argument.

why publish on regular channel does not support callback properly that will return err when the RabbitMQ is down for instance.

If the connection is dropped, you'll find out about it from the Channel and the Connection. In general, #publish is asynchronous -- the message won't always be written to the socket immediately, and even if it were, it doesn't know if the write actually went on the wire or not.

@Kurtas
Copy link
Contributor

Kurtas commented Jan 22, 2016

@squaremo I'm little confused, according to API doc #publish should return Promise or no?

@squaremo
Copy link
Collaborator

I'm little confused, according to API doc #publish should return Promise or no?

No, it does not return a promise. http://www.squaremobius.net/amqp.node/channel_api.html#channel_publish

@hekike
Copy link

hekike commented Jan 23, 2016

@squaremo I also faced with this issue, check out followng use cases:

channel.publish().then(..)
// => .then is not a function
channel.publish(..., cb)
// cb is called

So it works with callback interface but not with Promise.

@Kurtas
Copy link
Contributor

Kurtas commented Jan 24, 2016

@hekike I found solution, bad it's hard to find this in api doc.
I hope that it's a correct way, it seems that it work for me.

  1. you have to call createConfirmChannel() instead createChannel()
  2. send msg via publish or sendToQueuemethod
  3. use waitForConfirms() to return promise

Example

/**
* Save JSON object to result queue
* @param {Object} Rabbit channel object => #createConfirmChannel
* @param {Object} Queue config object
* @param {Object} msg
* @returns {Promise}
*/
export function rabbitPublishMsqQueue(channel, queue, msg, config) {
  if (typeof config !== 'object') config = {persistent: true};
  channel.sendToQueue(
    queue.name,
    new Buffer(JSON.stringify(msg)),
    config
  );
  return channel.waitForConfirms();
}

@hekike
Copy link

hekike commented Jan 24, 2016

Thanks, it's really helpful.
But actually my real issue is not that I don't have feedback about msg sending, I don't really need it currently.

Instead of it, my concern is that the documentation says it's a promise so probably async but it seems to be that not.
And I don't know how should I handle it. And with callback why is it calling the callback function if it's not async...

I belive the issue is the documentation or the publish(..) itself but I can't decide without the help of the author.

@squaremo what do you think? Should it return Promise for the consistent interface or not?

@squaremo
Copy link
Collaborator

what do you think? Should it return Promise for the consistent interface or not?

In AMQP, the basic.publish method is not an RPC; i.e., a reply is not expected. Returning a promise would be artificial and awkward (when would it resolve? what would it resolve to?). So I am happy that it should be different to the RPCs.

my concern is that the documentation says it's a promise so probably async but it seems to be that not.

I'm pretty sure it doesn't say that publish returns a promise. It does say at the top of the API reference that most methods return a promise (or take a callback); but, in the documentation for publish, it says it returns a boolean.

I do understand why folks don't spot that, in the midst of a fair bit of text. Perhaps I can separate out the methods that don't return promises (it's not just publish -- ack and reject too), to make things clearer.

@hekike
Copy link

hekike commented Jan 25, 2016

@squaremo thanks for the clarification.
Am I read the documenation in a wrong way? :S
http://www.squaremobius.net/amqp.node/channel_api.html#channel_publish

Channel#publish
Promises or callbacks

#publish(exchange, routingKey, content, [options])

Some separation or highlighting would be useful I for me.

About the not rpc than why Promise:
I understand that it won't respond in the Promise but I think still could some issue happen at publish like: cannot publish the message, socket timeout etc. Isn't it possible that it cannot publish for some connection issue? How is it handled now? throw error?

@japel
Copy link

japel commented May 26, 2016

agree with @squaremo ... it shouldnt return a promise if there is no response...
but I also find it misleading to write on every api method "Promises or callbacks" even if there is no promise and no callback... had the same issue, .then is not a function, until I saw it was boolean. So yeah, some improvements on api reference could be done...

@HERRKIN
Copy link

HERRKIN commented Jul 4, 2016

I think it should be async meaning it should tell if it got send or not (knowing the state of a connection), instead of me having to check if the connection is up, I am trying to see how to handle errors in case a message wasn't sent, I am confused.

@villesau
Copy link

villesau commented Jan 2, 2020

I'm little confused, according to API doc #publish should return Promise or no?

No, it does not return a promise. http://www.squaremobius.net/amqp.node/channel_api.html#channel_publish

@squaremo the doc also says:

There are two parallel client APIs available. One uses promises, and the other uses callbacks, mutatis mutandis. Since they present much the same set of objects and methods, you are free to choose which you prefer, without missing out on any features. However, they don’t mix – a channel from the promises API has only the promise-based methods, and likewise the callback API – so it is best to pick one and stick with it.

https://www.squaremobius.net/amqp.node/channel_api.html

It's a bit confusing which apis have promise interface and which does not. Seems pretty arbitrary.

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

8 participants