ruby-amqp / hutch

A system for processing messages from RabbitMQ.
https://gocardless.com/blog/hutch-inter-service-communication-with-rabbitmq/
MIT License
855 stars 137 forks source link

Add delivery_info to properties for error handler #395

Closed tlloydthwaites closed 4 months ago

tlloydthwaites commented 1 year ago

When handling an error, it would be useful to know the delivery_info such as the routing key. This adds delivery_info to message properties. This seemed the only way to supply the information without changing the error handler contract.

michaelklishin commented 1 year ago

Message properties and delivery_info are not the same thing (semantically in the protocol and in Bunny). For that reason, I don't like merging them together.

I need to take a look at what other options we may have, although very likely that'd mean modifying the error reporting interface :(

tlloydthwaites commented 1 year ago

Yep, it's a hack... I don't see any other option other than updating the interface.

One solution might be to check arity on the handler method to see if it allows the extra param.

Something like:

handle_error(properties, payload, consumer, ex, delivery_info)

then:

backend.handle *args.first(backend.method(:handle).arity)

This relies on delivery_info being the last parameter in the handler. We could also check using something like:

if backend.method(:handle).parameters.map { |e| e[1] }.include?(:delivery_info)

But the arity method would allow the extra param on the end without disrupting existing handlers.

tlloydthwaites commented 1 year ago

Updated PR with arity check, working nicely here :)

michaelklishin commented 4 months ago

I still don't find the merge of the two properties to be a good idea but for error reporting and a set of existing error handlers, I guess, this may be the most pragmatic option 😭