Fix #465 #466
Fix #465 #466
Conversation
spec/issues/issue465_spec.rb
Outdated
| end | ||
|
|
||
| describe '#start' do | ||
| it 'should be possible to reopen it' do |
There was a problem hiding this comment.
I didn't plan on supporting this but if we do and it's actually a safe thing to do in practice, no objections from me.
There was a problem hiding this comment.
Ok, given that I'll remove the recommendation given in current ConnectionManuallyClosed exception (name will change), I'll also drop these specs in order to maintain things coherent and not add official support for connection reopen. @michaelklishin do you agree?
lib/bunny/exceptions.rb
Outdated
| @@ -97,6 +97,12 @@ def initialize(frame) | |||
| end | |||
| end | |||
|
|
|||
| class ConnectionManuallyClosed < Exception | |||
There was a problem hiding this comment.
Other clients use AlreadyClosedException (Java) and similar names. I'd prefer that.
lib/bunny/exceptions.rb
Outdated
| @@ -97,6 +97,12 @@ def initialize(frame) | |||
| end | |||
| end | |||
|
|
|||
| class ConnectionManuallyClosed < Exception | |||
| def initialize | |||
| super('Connection has been manually closed. Call #start to reopen it') | |||
There was a problem hiding this comment.
I'm not sure how I feel about recommending this. It may or may not be safe and I don't think we should support reopening at all. It is hard and is not meaningfully easier to use than opening a new connection.
So let's drop the recommendation.
…ection reopen after it has been manually closed Change ConnectionManuallyClosed exception name to ConnectionAlreadyClosed Minor improvements
|
Thank you! |
I provided some specs related to #465 and also suggested a fix that I hope correctly addresses the issue.