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

feat: skip middleware, plugins, and exception reporting when exceptions are disabled #646

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

stympy
Copy link
Member

@stympy stympy commented Nov 19, 2024

This PR makes the exceptions.enabled configuration option work a little better by doing the following:

  • Skip loading ErrorNotifier and associated middleware
  • Skip hooking into Sidekiq, etc. to report exceptions
  • Abort sending any notifications when calling Honeybadger.notify

This reverts commit a00c6a0.
@stympy stympy requested review from roelbondoc and joshuap November 19, 2024 21:59
Copy link
Member

@joshuap joshuap left a comment

Choose a reason for hiding this comment

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

I vaguely remember using the exceptions.enabled config option in the past, but we don't list it in the docs, and it looks like the only place it's currently used is in the sinatra integration. Is that right? 😅

CleanShot 2024-11-20 at 16 41 27@2x

I'm OK with this PR. I think using the requirement block syntax I mentioned where possible would make some of the plugins cleaner, but obviously we can't do that for plugins that also do Insights things (unless we separate them).

@stympy are you sure this covers all the exceptions features in the plugins, though?

lib/honeybadger/plugins/resque.rb Show resolved Hide resolved
@joshuap
Copy link
Member

joshuap commented Nov 21, 2024

@roelbondoc did you review any of the other plugins?

@roelbondoc
Copy link
Member

@roelbondoc did you review any of the other plugins?

I went through all the plugins and this covers everything except for the ErrorSubscriber in the rails plugin.

However, there is a early return based on the config in Honeybadger::Agent#notify method that will prevent any calls so this would cover anything we missed including custom invocations in an application.

@stympy stympy merged commit 6c4d7d5 into master Nov 21, 2024
59 checks passed
@stympy stympy deleted the disable-exceptions branch November 21, 2024 19: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.

3 participants