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

More information in the context of each exception #37

Open
everton opened this issue Mar 3, 2024 · 5 comments
Open

More information in the context of each exception #37

everton opened this issue Mar 3, 2024 · 5 comments

Comments

@everton
Copy link
Contributor

everton commented Mar 3, 2024

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:

  1. Is there any reason no not use #inspect instead of creating the errors as just the class name?
  2. What do you think about improving the context identification in the gem itself? Would this go beyond the scope you initially imagined?
@everton
Copy link
Contributor Author

everton commented Mar 3, 2024

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?

@everton
Copy link
Contributor Author

everton commented Mar 3, 2024

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.

@fractaledmind
Copy link
Owner

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.

@fractaledmind
Copy link
Owner

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: honeybadger-io/honeybadger-ruby#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: getsentry/sentry-ruby#1643 (comment)

class ApplicationJob < ActiveJob::Base
  before_perform do |job|
    Rails.error.set_context(arguments: job.serialize["arguments"])
  end
end

@fractaledmind
Copy link
Owner

@everton: Would you like to make the PR to patch controllers and jobs to automatically add context?

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