Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: raising NackMessage() ignores the retry parameter #1046

Closed
sidjakinsboriss opened this issue Dec 13, 2023 · 1 comment · Fixed by #1048
Closed

Bug: raising NackMessage() ignores the retry parameter #1046

sidjakinsboriss opened this issue Dec 13, 2023 · 1 comment · Fixed by #1048
Labels
bug Something isn't working

Comments

@sidjakinsboriss
Copy link

Describe the bug
If NackMessage is raised in the message handler, the retry parameter is ignored, and the messages keeps getting re-queued

How to reproduce
Include source code:

@rabbit_router.subscriber(queue, exchange, retry=5)
async def handle_message(user: User, message: RabbitMessage):
    raise NackMessage()

Expected behavior
Message will be re-queued a maximum of retry times.

Observed behavior
Messaged is re-queued forever.

Additional context
Happens because of this logic in push_back_watcher.py, so the number of retries is never checked:

if not exc_type:
   await self.__ack()

elif isinstance(exc_val, SkipMessage):
    self.watcher.remove(self.message.message_id)

elif isinstance(exc_val, HandlerException):
    if isinstance(exc_val, AckMessage):
        await self.__ack()
    elif isinstance(exc_val, NackMessage):
        await self.__nack()
    elif isinstance(exc_val, RejectMessage):  # pragma: no branch
        await self.__reject()
    return True

elif self.watcher.is_max(self.message.message_id):
    await self.__reject()

Moreover, is message.nack() is called, exc_type is None and the first branch is taken calling await self.__ack()

@sidjakinsboriss sidjakinsboriss added the bug Something isn't working label Dec 13, 2023
@Lancetnik Lancetnik mentioned this issue Dec 13, 2023
13 tasks
@Lancetnik
Copy link
Member

@sidjakinsboriss thank you for the Issue

I changed this behavior with 0.3.5 and now NackMessages also checks is_max condition

Btw, don't worry about default self.__ack - all messages can't be acked twice, so if you call ack/nack/reject manually, all following calls just do nothing

@Lancetnik Lancetnik moved this to In Progress in FastStream Dec 13, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in FastStream Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants