gmr / rabbitpy

A pure python, thread-safe, minimalistic and pythonic RabbitMQ client library
http://rabbitpy.readthedocs.org
BSD 3-Clause "New" or "Revised" License
243 stars 58 forks source link

Connection context manager does not close connection on exception? #145

Open jamesrusso opened 2 weeks ago

jamesrusso commented 2 weeks ago

Maybe this is intended, but I'm trying to understand why.

https://github.com/gmr/rabbitpy/blob/f5dc2cf0fe41ef3da53b053bd03cda28836eff49/rabbitpy/connection.py#L149

If we get an exception within the "Connection" context manager we are given no opportunity to close the connection.

      try:
                with rabbitpy.Connection(f"{Config.AMPQ_URL}", {"connection_name": "receiver-thread"}) as connection:
                    with connection.channel() as channel:
                        self.queue = rabbitpy.Queue(channel, self.queue_name, auto_delete=self.auto_delete, exclusive=self.exclusive, durable=self.durable)
                        self.queue.declare()
                        for binding_key in self.binding_keys:
                            self.queue.bind(self.exchange, binding_key)
                        self.startup_event.set()
                        for message in self.queue.consume():
                            logger.info("received message %s", message)
                            self.event = Event.from_dict(message.json())
                            if self.callback and self.callback(self.event):
                                message.ack()
                            else:
                                message.nack()
            except rabbitpy.exceptions.AMQPException:
                logger.exception("AMQP exception", exc_info=True)
                self.startup_event.clear()
                self.queue = None
                continue
            except RuntimeError as ex:
                logger.exception("runtime error %s", ex)
                self.startup_event.clear()
                self.queue = None
                continue
            except rabbitpy.exceptions.RabbitpyException as ex: 
                logger.exception("rabbitmq exception %s", ex)
                self.startup_event.clear()
                self.queue = None
                continue
            finally:
                logger.info("waiting to attempt reconnection...")
                time.sleep(1)

Take this code for example, if the exchange isn't declared it'll raise an exception and come back around for another attempt however this will create one connection/channel for each attempt.

If we are raising an exception here shouldn't the connection be closed prior to the raising of the exception?

jamesrusso commented 2 weeks ago

Seems to me, that you should not be re-raising the exception in the exit method of the context handler. It should simply return not True and the exception will be raised upon return from the exit method.

Python Docs Reference