ruby-amqp / amq-client

THIS PROJECT IS NOW DEPRECATED (part of the amqp gem codebase)
MIT License
36 stars 13 forks source link

AMQ::Client::Async::Channel#on_ack calls non-existent #use_publisher_confirmations! #25

Open eliaslevy opened 11 years ago

eliaslevy commented 11 years ago

If a channel is does not have publisher confirmation on when the #on_ack method is called, #on_ack will attempt to turn confirmations on by calling #use_publisher_confirmations!. As that method does not exist, it will cause a NoMethodError exception.

The problem goes back 2 years. 5653b0719dfdd344ed50c23628b4be8db9d2a6c3 renamed #confirmations to #use_publisher_confirmations!. It also added the line to #on_ack to call #use_publisher_confirmations! if confirmations were not on. The same day you renamed #use_publisher_confirmations! to #confirm_select, but failed to change the reference in #on_ack (a092ae0a8bcfff3c954447fc05cbddccff67cf5f).

Since then the code got moved around, but its remained otherwise the same. You last commit related to it moved the code into AMQ::Client::Async::Channel.

This bug has been around for 2 years and we never triggered it since we always call #confirm_select in the channel before using it. But something in the latest commits must have broken how we were using #confirm_select leaving the channel with confirms off by the time we call #on_ack, thus triggering this old bug.

I am going to dig further to find out why the channel is reaching #on_ack with confirms off in the latest code.

eliaslevy commented 11 years ago

I think I figured out why we are seeing the error now. You committed 0096aaaecd6e96e44f0e3d20b6fb058d7bd83817 2 months ago. It delays calling #confirm_select until the channel is open. In our systems we open, then close, a new channel every time we publish, so as to better encapsulate errors. That means after that change, after we create a channel, and immediately call #confirm_select, the channel will not be in confirm mode. When we next call #on_ack it will notice the channel is not in confirm mode and try to call the non-existent method.

michaelklishin commented 11 years ago

amq-client is deprecated and fully merged into amqp gem master. If you want to investigate what can be done to make it not fail with a non-existent method, look there. I think it should be possible to define on_ack handlers regardless of the current channel state.

eliaslevy commented 11 years ago

I am not using amq-client directly. I am using amp, which as of 1.1.0.pre1 still depended on amp-client, but I see that dependency has been removed in 1.1.0.pre2. I'll give that a whirl. I don't see the but on #on_ack.