-
Notifications
You must be signed in to change notification settings - Fork 139
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
Customizable error response handling (custom nacks) #177
Conversation
@@ -129,11 +130,23 @@ def handle_error(message_id, payload, consumer, ex) | |||
end | |||
end | |||
|
|||
def acknowledge_error(delivery_info, properties, broker, ex) | |||
acks = error_acknowledgements + | |||
[Hutch::Acknowledgements::NackOnAllFailures.new] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simply have a non-empty default for error_acknowledgements
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answering my own question: because :error_acknowldgements
is not a list, it's a chain of responsibility. Why so? Because double-acking would result in a channel exception and make underlying channel useless.
The overall idea is sound. However, I find the way you fall back to the default a bit odd. Once that is addressed, I think we can move forward with this. Thank you! |
class NackOnAllFailures | ||
include Logging | ||
|
||
def handle(delivery_info, properties, broker, ex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the interface here is a single function, it makes more sense to use call
so that functions (blocks, procs, etc) can be used as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch the above. It would lead to inconsistency with error handlers, which use handle
and it is way too late to change that.
@dkastner merged with some minor tweaks. Thank you! |
Thanks for taking a look! |
Code related to #165
By default a nack is always sent on an exception.
Allows configuring custom handlers similar to the way logging is configured: