fix: raise ChannelInvalidStateError at exchange.publish with closed channel #637
Conversation
| raise aiormq.exceptions.ChannelInvalidStateError( | ||
| "%r closed" % self.channel, | ||
| ) | ||
|
|
||
| channel = await self.channel.get_underlay_channel() |
There was a problem hiding this comment.
Isn't channel closure checked in this method?
There was a problem hiding this comment.
Nope, it is waiting for channel readiness forewer
There was a problem hiding this comment.
Did you mean waiting here?
aio-pika/aio_pika/robust_channel.py
Lines 78 to 80 in 9c1ee8f
There is waiting for connection readiness - not for channel readiness, and in the test you are closing connection, not channel. Maybe in publish method should be checked connection (not channel) and aiormq.exceptions.ConnectionClosed exception raised?
There was a problem hiding this comment.
Not sure, that is correct. I just followed the aiormq exceptions style. Also, connection and channel and quite closely related, so they can be open/closed both only. I am checking channel due it is the most important thing for we functionality. Can @mosquito resolve the dispute?
There was a problem hiding this comment.
I can change the code without any problems and basically agree with your points, but not sure about the person, who makes the final decision about PR merging
There was a problem hiding this comment.
Anyway, we have no access to self.channel._connection object due it is RobustChannel-class attribute only. So, we should check self.channel to be compatible with any AbstractChannel implementation
| raise aiormq.exceptions.ChannelInvalidStateError( | ||
| "%r closed" % self.channel, | ||
| ) | ||
|
|
||
| channel = await self.channel.get_underlay_channel() |
There was a problem hiding this comment.
Did you mean waiting here?
aio-pika/aio_pika/robust_channel.py
Lines 78 to 80 in 9c1ee8f
There is waiting for connection readiness - not for channel readiness, and in the test you are closing connection, not channel. Maybe in publish method should be checked connection (not channel) and aiormq.exceptions.ConnectionClosed exception raised?
| @@ -339,6 +339,34 @@ async def test_simple_publish_and_receive_to_bound_exchange( | |||
|
|
|||
| await queue.unbind(dest_exchange, routing_key) | |||
|
|
|||
| async def test_simple_publish_with_closed_channel( | |||
There was a problem hiding this comment.
Shouldn't it be called "test_simple_publish_with_closed_connection" ?
|
@Lancetnik This was released to day with v9.4.3 |
Close #636, close #556, close airtai/faststream#1579
Exception was copied from https://github.com/mosquito/aiormq/blob/master/aiormq/channel.py#L145