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

Mark message as failed when fail to process #545

Closed
wants to merge 1 commit into from

Conversation

jairhenrique
Copy link
Contributor

Mark message as failed when actor raises an Exception.

On self.consumers[message.queue_name].post_process_message(message) the method nack will be called.

This change will enable refactor on dramatiq_sqs to skip message delete and redrive message to dlq. Bogdanp/dramatiq_sqs#10 (comment)

@spumer
Copy link

spumer commented Dec 12, 2023

Without that fix we have incorrect behaviour in Retries middleware, when enqueue to DELAY queue failed we silently ACK message and lose it

@Bogdanp

@Bogdanp
Copy link
Owner

Bogdanp commented Apr 29, 2024

I don't think nacking here is the right solution, since that would cause the same message to be redelivered later by the Redis and RMQ backends.

@Bogdanp Bogdanp closed this Apr 29, 2024
@spumer
Copy link

spumer commented Jun 15, 2024

It's would not be redelivered since it's acked.

Steps:

  • processing in actor failed
  • retry middleware try to delay (in after_process_message hook)
  • delay failed (in after_process_message hook)
  • message acked as processed

As result we have error but no have message in dead letter queue or delay queue.

I mean if exception raised in this line, when Retries middleware try to enqueue message:

self.broker.emit_after("process_message", message, exception=e)

Above fix does not help, it's right. But problem is real. We have workaround where we catch exception in retry middleware and mark message as failed.

We can open new issue for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants