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

Allows to pass all message properties to error handlers. #238

Merged
merged 2 commits into from
Aug 1, 2016

Conversation

gottfrois
Copy link
Contributor

Message properties can include valuable message informations such as
correlation id, message type, app specific headers and many more.
When handling consumer process errors, we should be able to make decisions
based on any message properties.

Note: This is a breaking change in case people created their own error handlers.

@@ -6,7 +6,8 @@ module ErrorHandlers
class Airbrake
include Logging

def handle(message_id, payload, consumer, ex)
def handle(properties:, payload:, consumer:, ex:)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this syntax.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so these are required keyword arguments that are available in Ruby 2.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, I looked at your travis definition and seems we only care about ruby >= 2.0

  - "2.3.0"
  - "2.2"
  - "2.1"
  - "2.0"
  - "jruby-9.0.0.0"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bunny only supports 2.0+ in recent versions, so that's fine.

@gottfrois
Copy link
Contributor Author

The spec fails because of latest activesupport release:

Gem::InstallError: activesupport requires Ruby version >= 2.2.2.

A choice needs to be made here, whether we want to support older ruby versions or use latest activesupport.

@michaelklishin
Copy link
Member

@gottfrois let's pin to a 4.x version for now.

@michaelklishin
Copy link
Member

@gottfrois perhaps we want to depend on ~ 4.2 does that version also not support Ruby 2.0?

@gottfrois
Copy link
Contributor Author

gottfrois commented Jul 8, 2016

Ruby 2.0 doesn’t have built-in support for required keyword arguments.

My bad, ruby 2.0 doesn't support (foo:, bar:). Will push a fix.

Message properties can include valuable message informations such as
correlation id, message type, app specific headers and many more.
When handling consumer process errors, we should be able to make decisions
based on any message properties.
activesupport 5 requires Ruby version >= 2.2.2.
@michaelklishin michaelklishin self-assigned this Aug 1, 2016
@michaelklishin
Copy link
Member

OK, even though this is a breaking change in the error handler API, it makes sense. I'll update change log to highlight this.

@michaelklishin michaelklishin merged commit 459b837 into ruby-amqp:master Aug 1, 2016
@gottfrois gottfrois deleted the error-handler-parameters branch August 2, 2016 08:27
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.

2 participants