Trying to publish on a closed channel results in broad exception type RuntimeError

[phijor]

Hi,

we are working on an application that, through aio-pika, uses aiormq. While dealing with some issues caused by unreliable network conditions, we noticed that trying to publish on a channel that has been closed results in a rather broad RuntimeError, raised here:

aiormq/aiormq/channel.py

Line 77 in 5e735e0

raise RuntimeError('%r closed' % self)

Below is a typical stack trace for when this happens, starting at a call to aio_pika.Exchange.publish():

Traceback (most recent call last):
  <snip>
  ...
  </snip>
  File "/<our_project>/source.py", line 106, in _send
    await self.data_exchange.publish(msg, routing_key=metric, mandatory=False)
  File "/<venv>/lib/python3.7/site-packages/aio_pika/exchange.py", line 200, in publish
    ), timeout=timeout
  File "/usr/lib64/python3.7/asyncio/tasks.py", line 414, in wait_for
    return await fut
  File "/<venv>/lib/python3.7/site-packages/aiormq/channel.py", line 426, in basic_publish
    async with self.lock:
  File "/<venv>/lib/python3.7/site-packages/aiormq/channel.py", line 77, in lock
    raise RuntimeError('%r closed' % self)
RuntimeError: <Channel: "1"> closed

We are afraid that, when writing error handling code for this condition, catching RuntimeError might accidentally catch other errors, too.

Would it be possible to raise a more descriptive exception, of distinct type? The exception type might subclass RuntimeError as to not break code that catches that.

[decaz]

Also thought about it. Perhaps existing ChannelClosed exception would fit..

[mosquito]

Fixed in #65. and aiormq==3.1.0

[mosquito]

Thank you for this issue and sorry for the delayed reply. I hope I released what are you wanted :-)

Please close this issue if this enough.

[decaz]

I remember about need of backward compatibility, but if inherit new exception from AMQPChannelError then it would be possible to easily catch any exception raised by aiormq by simply handling AMQPError.

[phijor]

I like @decaz's idea of having a single exception to catch, but right now having ChannelInvalidStateError is good enough, as it cover most (all?) of our use cases.

Thank you :)

Now the only thing that is needed is a release of aio-pika that includes this fix. I see that since mosquito/aio-pika@6498575, it already uses aiormq~=3.0.