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

Move the Sidekiq reporter to after the logging middleware #326

Closed
wants to merge 1 commit into from

Conversation

sb8244
Copy link
Contributor

@sb8244 sb8244 commented Sep 20, 2016

The Server::Logging middleware is the outermost middleware that Sidekiq provides by default.
This is a very safe place to put error reporting middleware, because any custom code implemented
by users of Bugsnag will probably not cause erroneous reports.

Specifically, we use a class that prevents error logging for certain error types Sidekiq::Retry unless it's the final retry. This helps reduce noise greatly. Bugsnag will report these errors even if we catch them ourselves, because it runs before our middleware. However, this middleware must execute after RetryJobs middleware.

We have a fix for this in our code (manually removing / adding Bugsnag back in), but wanted to also provide this insight for the Bugsnag team.

The Server::Logging middleware is the outermost middleware that Sidekiq provides by default.
This is a very safe place to put error reporting middleware, because any custom code implemented
by users of Bugsnag will probably not cause erroneous reports.
@sb8244
Copy link
Contributor Author

sb8244 commented Sep 20, 2016

Looks like travis had a hiccup?

@kattrali kattrali closed this in c7862ea Apr 7, 2017
reubenbrown added a commit to reubenbrown/bugsnag-ruby that referenced this pull request Apr 25, 2017
kattrali pushed a commit that referenced this pull request Apr 27, 2017
kattrali pushed a commit that referenced this pull request Oct 28, 2017
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.

1 participant