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

Add exception handler #7

Merged
merged 4 commits into from
Sep 30, 2016
Merged

Add exception handler #7

merged 4 commits into from
Sep 30, 2016

Conversation

k0kubun
Copy link
Collaborator

@k0kubun k0kubun commented Sep 30, 2016

Background

  • We want to handle exception with Raven.capture_exception.
    • There may be some people who don't want to use Raven.
  • Exceptions in Barbeque::Worker must be rescued and given to this exception handler.
    • Other exception are properly handled if we configure raven.

Changes

Add exception handler to handle exception with Rails.logger or Raven.

Please review 👓 @cookpad/dev-infra

@k0kubun k0kubun force-pushed the exception_handler branch 5 times, most recently from 35e4a12 to 5ce1aba Compare September 30, 2016 07:13
@k0kubun
Copy link
Collaborator Author

k0kubun commented Sep 30, 2016

To keep barbeque.gem simple, I changed Barbeque::ExceptionHandler to be pluggable. bd828aa

@eisuke
Copy link
Member

eisuke commented Sep 30, 2016

pluggable 👍
LGTM

module Barbeque
module ExceptionHandler
def self.handle_exception(e)
handler = const_get(Barbeque.config.exception_handler, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, const_get takes a cost so you might like to memorize handler like @handler ||= const_get(Barbeque.config.exception_handler, false).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed b4ea76e

@makimoto
Copy link
Contributor

LGTM

@k0kubun k0kubun merged commit 32210c3 into cookpad:master Sep 30, 2016
@k0kubun k0kubun deleted the exception_handler branch September 30, 2016 07:48
k0kubun pushed a commit that referenced this pull request Sep 30, 2016
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.

None yet

3 participants