Add Sidekiq support #63
Comments
|
I'm on it, thanks! |
|
Sidekiq 3.0 is now on master. You'll want to review this: https://github.com/mperham/sidekiq/blob/master/Upgrading.md#error-service-providers |
In response to #63, this commit adds reporting of exceptions which happen in Sidekiq processes. The new error_handler API is used so that all exceptions are reported, not just those which occur during job execution: https://github.com/mperham/sidekiq/blob/master/Upgrading.md#error-service-providers A server middleware is also added in order to clear the Honeybadger context hash after each job executes. Because earlier versions of Sidekiq had native support for reporting exceptions but did not clear the context hash, the middleware and error handler are installed separately; all Sidekiq versions get the middleware, but only >= 3.0.0 get the error handler.
|
@joshuap I think you might have a problem. The error handler isn't called until the error trickles all the way back up the stack to the Processor. By this time your middleware would have already executed and cleared the context. Maybe your error handler should clear the context after calling |
|
I know of people already using 3.0 (aka the master branch) in production but we're not and I still think of master as not production quality yet. That said, I don't know of any problems with it. |
|
Ah, good point. The problem with clearing the context in the error handler block is that it is only invoked when an error occurs, which would not be every job. Ideally the context would be specific to each job, since you wouldn't want data for multiple jobs being sent for a single exception. Can you think of a way to accomplish that? If not, I guess clearing it in the error handler would be better than nothing. |
|
Clear it at the START of a new job. :-) On Wed, Mar 12, 2014 at 8:16 AM, joshuap notifications@github.com wrote:
|
|
Heh. This is why we pay you the big bucks. :) |
Per #63, This prevents the context from being cleared before the error handler can report it (which happens up the stack, and isn't always job-specific).
I'm going to drop native support for error handlers in Sidekiq 3.0, so as to not play favorites and give each error service full control over their own implementation. This includes Honeybadger so you'll need to ship your own Sidekiq 3.0 support.
To be clear, I'll be removing this line:
https://github.com/mperham/sidekiq/blob/master/lib/sidekiq/exception_handler.rb#L13
BugSnag has a sample implementation for their integration here:
https://github.com/bugsnag/bugsnag-ruby/blob/master/lib/bugsnag/sidekiq.rb
3.0 has no set release date so you should have plenty of time.
The text was updated successfully, but these errors were encountered: