benjamin-hodgson / asynqp

An AMQP library for asyncio
MIT License
84 stars 29 forks source link

Correctly handle zero-length messages #76

Closed anton-ryzhov closed 8 years ago

anton-ryzhov commented 8 years ago

When RabbitMQ sends message without payload — it sends only Content Header frame, but no Contend Body. asynqp waits for Contend Body even if body_length == 0. Message becomes never fully received and never acked. Breaks entire consumer if it limits prefetch count.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.1%) to 95.722% when pulling c0b1ea1f076509edc4f59800100b1d55542801b3 on anton-ryzhov:empty_messages into 8f55f2f03cba8aea7c8f1532fa283df3bc50b02e on benjamin-hodgson:master.

tvoinarovskyi commented 8 years ago

Pls add a test for it. Something like:

class WhenISendZeroMessage(BoundQueueContext):
    def given_an_empty_message(self):
        self.message = asynqp.Message('')
        self.exchange.publish(self.message, 'routingkey')

    def when_I_start_a_consumer(self):
        self.message_received = asyncio.Future()
        self.loop.run_until_complete(asyncio.wait_for(self.start_consumer(), 0.2))

    def it_should_deliver_the_message_to_the_consumer(self):
        assert self.message_received.result() == self.message

    @asyncio.coroutine
    def start_consumer(self):
        yield from self.queue.consume(self.message_received.set_result)
        yield from self.message_received

in integration tests should be ok. The patch looks good.

anton-ryzhov commented 8 years ago

Thank you for help, I understood nothing about your test system.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.01%) to 95.863% when pulling b77ad904153198db4fbc297156f681da3277a9ab on anton-ryzhov:empty_messages into 8f55f2f03cba8aea7c8f1532fa283df3bc50b02e on benjamin-hodgson:master.

TarasLevelUp commented 8 years ago

Ye, it's the first thing we agreed on changing in v0.6

anton-ryzhov commented 8 years ago

What about this PR, will you merge?

tvoinarovskyi commented 8 years ago

I am not a contributor =) @benjamin-hodgson Has to view all of it.

benjamin-hodgson commented 8 years ago

Thanks for the contribution. Looks good!