feat(insights): before_event callback #213
Conversation
Also fix config bleeding issue with asgi test
There was a problem hiding this comment.
Pull Request Overview
Adds support for a before_event callback to allow users to mutate or skip events, and fixes configuration bleeding in ASGI tests.
- Introduce
before_eventoption inConfigurationwith default passthrough behavior. - Apply
before_eventlogic inHoneybadger.eventto handle in-place mutation, replacement, or explicit rejection. - Add tests for
before_eventbehavior and reset global config in ASGI tests to avoid bleed.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| honeybadger/config.py | Add before_event to OPTIONS and default. |
| honeybadger/core.py | Integrate before_event callback in event. |
| honeybadger/tests/test_core.py | New tests for mutated, replaced, and skipped events. |
| honeybadger/tests/contrib/test_asgi.py | Reset global config in ASGI event test to avoid bleed. |
| @@ -107,6 +107,17 @@ def event(self, event_type=None, data=None, **kwargs): | |||
| "The first argument must be either a string or a dictionary" | |||
| ) | |||
|
|
|||
| if callable(self.config.before_event): | |||
There was a problem hiding this comment.
The event method docstring should be updated to describe the new before_event callback behavior: how return values are interpreted (False to skip, None for in-place mutation, a dict to replace).
Copilot uses AI. Check for mistakes.
| @@ -22,6 +22,7 @@ class Configuration(object): | |||
| # Insights options | |||
| ("insights_enabled", bool), | |||
| # Events options | |||
| ("before_event", callable), | |||
There was a problem hiding this comment.
[nitpick] Consider using collections.abc.Callable instead of the built-in callable for clearer type intent in the OPTIONS tuple.
Copilot uses AI. Check for mistakes.
This sounds good to me! |
Adds a
before_eventcallback with a similar implementation as thebefore_noticewith one small change: This only rejects an event with an explicitFalsereturn. If the result isNone, which is typically no return, then we assume that the user is mutating the payload and use the one passed in. @roelbondoc, what do you think about this tweak?