More information in the context of each exception
In order to understand some errors, it is better to have the context that generated them (the params in a controller action, the arguments sent to a job, etc). The code line and stack trace are great, but you need to go after all your registers to understand which record generated that bug, etc.
I totally understand that you are trying to keep the gem as simple as possible, and as is stated in the README:
The goal is to provide a simple, lightweight, and performant solution for tracking exceptions in your Rails application. If you need more features, you should probably use a 3rd party service
I added this information using using the Rails.error.set_context with codes like:
class ApplicationController < ActionController::Base
before_action { Rails.error.set_context(params: params) }
end
and
class ApplicationJob < ActiveJob::Base
before_perform do |job|
Rails.error.set_context(arguments: job.arguments)
end
end
It worked for the controllers, but for the jobs, active record objects were saved in the context as just a String with their class name, which makes them useless.
I saw the "sanitizer" code, and I can see you chose to create this string instead of, for example, calling the Object#inspect. I have two separate question for this issue:
- Is there any reason no not use #inspect instead of creating the errors as just the class name?
- What do you think about improving the context identification in the gem itself? Would this go beyond the scope you initially imagined?
P.S.: for the second question, if you feel it is indeed outside of the scope of this gem to bring the context inside the engine, would you mind if at least we add a how-to in the README orienting how people can add this more obvious information like the params and args?
P.S.2: In the case of the Jobs, if #inspect is adopted to save the context, the arguments will already be there, and there is no actual need to add that before_perform I suggested initially.
What do you think about improving the context identification in the gem itself? Would this go beyond the scope you initially imagined?
I'm open to the PR, yes.
Is there any reason no not use #inspect instead of creating the errors as just the class name?
I dropped the #to_honeybadger case from the sanitizer, but kept the else branch as is.
The PR in Honeybadger that added this code explains more context: https://github.com/honeybadger-io/honeybadger-ruby/pull/239
At a more basic level, #inspect isn't used for probably the same reason it isn't used by ActiveJob—it is noisy and unpredictable. But, I see your point that the current situation is clearly lacking.
Maybe the simplest path forward is to have the hook added by the gem to jobs pushed ActiveJob's serialized data, like this comment from the Sentry repo: https://github.com/getsentry/sentry-ruby/issues/1643#issuecomment-1013094261
class ApplicationJob < ActiveJob::Base
before_perform do |job|
Rails.error.set_context(arguments: job.serialize["arguments"])
end
end
@everton: Would you like to make the PR to patch controllers and jobs to automatically add context?