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

Raising rescued exceptions in Development mode #72

Closed
pomartel opened this issue May 1, 2014 · 3 comments
Closed

Raising rescued exceptions in Development mode #72

pomartel opened this issue May 1, 2014 · 3 comments

Comments

@pomartel
Copy link
Contributor

pomartel commented May 1, 2014

When running in local, honeybadger will not send exceptions which is fine. But it will also swallow manually rescued exceptions without any notices like in the example mentioned here : http://docs.honeybadger.io/article/32-log-ruby-exceptions-already-rescued

Here is the behaviour I would like to see implemented directly in the notify_honeybadger method :

rescue => ex
  if Rails.env.development?
    raise ex  # or logger.error(ex) 
  else
    notify_honeybadger(ex)
  end
  flash[:failure] = 'Encryptions could not be rerouted, try again.'
end

I could of course do this all over my code but it would bloat it and I really think it would be helpful for most developers to see exceptions raised locally. What do you think ?

@joshuap
Copy link
Member

joshuap commented May 5, 2014

Thanks for the suggestion! Unfortunately I see a few problems with this. Many people have logic in their code which comes after calling notify_honeybadger, so causing this to re-raise would seriously break some projects in development. Also, it's possible to call this method outside of a rescue block, and re-raising there would cause unexpected exceptions to be raised.

If you want to use this everywhere, I'd either override the notify_honeybadger, or build your own method which has the modified behavior:

def report_exception(ex)
  if Rails.env.development?
    raise ex  # or logger.error(ex) 
  else
    notify_honeybadger(ex)
  end
end

@joshuap joshuap closed this as completed May 5, 2014
@pomartel
Copy link
Contributor Author

pomartel commented May 5, 2014

You are right that re-raising the exception might not be such a good idea since the code after that would not be executed in development. But then don't you think it would be at least a good idea to log the error in the console so we know something is happening? Or maybe have a global config to determine what should be the behaviour? Maybe a block of code like you have for the async config. Right now the only choices are to report or ignore the exception in development.

config.development_notify do |notice|  
  Rails.logger.error(ex) # The actual behaviour is set by the user in Honeybadger.configure
end

I could also implement a report_exception as you mention but that would only work in controllers and not my lib and model classes. I guess I would then need another report_exception method for those and suddenly it gets a bit messy for something that should probably handled directly in the gem!

@joshuap
Copy link
Member

joshuap commented May 13, 2014

Hey Pierre, sorry for not replying sooner! I get what you're saying, but I'm still not convinced this is something which belongs in the Honeybadger gem. Honeybadger is providing notification of errors, but it's still up to the user to handle the errors themselves. In your case I recommend creating your own error handling abstraction which will both notify Honeybadger in production and log in development (kind of like the report_exception method I mentioned earlier).

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

No branches or pull requests

2 participants