-
Notifications
You must be signed in to change notification settings - Fork 148
Description
Before Rails 8 was released we used the following to explicitly set the component and action when calling Rails.report
(to which, so far, only Honeybadger is subscribed):
exception = ...
component = ...
action = ...
context = ...
payload = { exception:, component:, action: }
Rails.error.report(payload, context: ..., ...)
This worked perfectly, the component and action were being correctly rendered by the Honeybadger UI. This was especially useful for explicitly setting the component and action for our jobs (we cannot use the Sidekiq plugin for Honeybadger as we handle errors in a different way). Downside of course is that other error reporters, if we were going to use them, would probably choke on the payload not being an exception.
However, once Rails 8 was released this workaround stopped working. Reason being the addition of this line to ActiveSupport::ErrorReporter
:
https://github.com/rails/rails/blob/v8.0.1/activesupport/lib/active_support/error_reporter.rb#L212
This line breaks when the first argument passed in to Rails.error.report
does not respond to the backtrace
method. There are hacky solutions for this, but maybe it's better if it's possible to pass in the component and action through the context instead, and Honeybadger can retrieve it from the context? That would also be more compatible in the situation of other error reporters than Honeybadger subscribing.
So something like:
exception = ...
component = ...
action = ...
context = ...
Rails.error.report(exception, context: context.merge(component:, action:), ...)
What do you think?
Activity
feat: set action and component from context
stympy commentedon Feb 21, 2025
I think this is reasonable, and I threw together a possible implementation in #672.
@joshuap what do you think?
joshuap commentedon Feb 24, 2025
I think we should think about/discuss a bit more as this as a team. My main concern is that
component
andaction
are generic terms, and sincecontext
is user/domain-specific data, these terms could conflict with people's existing and future context data—in which case, it would change the grouping of those errors since we include the component in our fingerprint (as well as displaying them in the UI). We do currently have several "magic context keys" such asuser_id
anduser_email
, but I think this is a bit different sincecomponent
andaction
are really internal Honeybadger properties.Looping in @subzero10 @rabidpraxis — what do you think?
gstokkink commentedon Feb 25, 2025
@joshuap agreed with your concerns regarding namespacing. Perhaps adding a
honeybadger
namespace within thecontext
hash is a good idea?subzero10 commentedon Feb 28, 2025
I don’t have a strong opinion on this (and that’s probably why it took me a few days to reply 😅), but one way to approach this would be to first define what the ideal solution would look like without the constraints of the current implementation. Then, once we’ve settled on that, we can figure out how to bridge the gap between the current behavior and the ideal solution.
If we look at component and action purely from a design perspective:
With that in mind, I think a safe approach could be:
This approach would:
gstokkink commentedon Feb 28, 2025
@subzero10 completely agree. Perhaps using
context: { honeybadger: { component:, action: }, ... }
is more future-proof thancontext: { honeybadger_component:, honeybadger_action:, ... }
when it comes to adding additional Honeybadger-specific contextual settings.joshuap commentedon Feb 28, 2025
If
component
andaction
really are context, I suppose we could make them official special keys, similar touser_id
anduser_email
, and list them in our docs:https://docs.honeybadger.io/guides/errors/#context-data
I just remembered that we also do something similar for
tags
in Ruby, so maybe we've already crossed this Rubicon?https://docs.honeybadger.io/lib/ruby/getting-started/adding-context-to-errors/#special-context-values
Just trying to think through all the implications of this. I think I'm warming to the idea of keeping them more general top-level keys (
component
andaction
) and adding it to the docs. 😅subzero10 commentedon Mar 1, 2025
To clarify, with this you are saying that we could add the to the
context
hash without namespacing them, right?Considering that we already have support for special keys, this just means we are adding to that list, so yeah it makes sense to me.
Another benefit with this solution would be that we can implement this change in the Honeybadger backend instead of having to add code in the client libraries to extract
component
andaction
keys fromcontext
before sending them to Honeybadger (the second point of my suggested approach above).Finally, if I understand correctly, the only concern with this solution is that it could create confusion for customers already using
context.component
andcontext.action
with different values than the top-levelcomponent
andaction
values. This seems unlikely to me, but I wonder if there's an easy way to verify this (i.e. query a sample of fairly recent errors)?subzero10 commentedon Mar 1, 2025
Today I was working on a wordpress-plugin for Honeybadger and I just came across this in our PHP library. It seems that for the PHP library, we support setting
component
andaction
throughcontext
, prefixed withhoneybadger_
, and extracting them just before sending the report to Honeybadger:stympy commentedon Mar 5, 2025
We have updated our pipeline to allow specifying the component and the action in the context by setting the
_component
and_action
keys in the context hash. No gem changes are necessary. :) If provided, the values for those keys will override whatever values would have been used for the component and action.Thanks for the suggestion, @gstokkink, and thanks for the notes on the PHP implementation and the backend suggestion, @subzero10!
Closes #672
subzero10 commentedon Mar 5, 2025
@stympy @joshuap Should we update docs to include
_action
and_component
? Is this the only place we'd update?stympy commentedon Mar 5, 2025
I opened an issue on the docs repo for this.
gstokkink commentedon Mar 13, 2025
@stympy @joshuap can I request the gem also be updated? We noticed that now in the
config.before_notify
callback,notice.action
andnotice.component
do not contain the values set through the context. Now we have to do something like this:action = notice.context.fetch(:_action, notice.action)
. Not very elegant if you ask me 😋5 remaining items