Skip to content
This repository has been archived by the owner on Sep 11, 2022. It is now read-only.

Commit

Permalink
Use MQ.error instead of Kernel#raise [#4].
Browse files Browse the repository at this point in the history
  • Loading branch information
Jakub Šťastný aka Botanicus committed Jan 13, 2011
1 parent 7decaed commit 75e5407
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/mq.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ def process_frame(frame)
end

when Protocol::Channel::Close
raise Error, "#{method.reply_text} in #{Protocol.classes[method.class_id].methods[method.method_id]} on #{@channel}"
MQ.error "#{method.reply_text} in #{Protocol.classes[method.class_id].methods[method.method_id]} on #{@channel}"

when Protocol::Channel::CloseOk
@on_close && @on_close.call(self)
Expand Down

6 comments on commit 75e5407

@arvicco
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it leave closed channel in inconsistent state? I mean, broker has closed the channel (for whatever reason), but you still have MQ instance hanging, and you can still try to do things with it, as if it is alive... How do you tell "live" channels from dead ones now?

Sure, you can set up MQ.error error processing callback to specifically check for this message and then somehow terminate your dead MQ instance, but not many people do this right now...

@botanicus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a jolly good point arvicco, I haven't thought about that. Raising an exception is definitely a wrong thing to do in an async environment, so I'd suggest to cache status in a variable, and have something like raise "X" if status.eql?(:closed) in method definitions. Thusly we'll raise an exception in a sync environment, so user will be able to rescue from it. I'll create an issue for this.

@botanicus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue: #8

@arvicco
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I have to agree that exceptions are a big pain in the a** when dealing with current lib... However we should be careful to not leave business objects in inconsistent state... I've also thought about @status=:closed or something similar...

@botanicus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

868f349

@arvicco
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, this should definitely work much better!

Please sign in to comment.