openedx / openedx/openedx-events
Event handler errors are suppressed by default in tests
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 13
- Forks
- 33
- Avg merge
- 14h 37m
- Merged PRs (30d)
- 4
Description
When I try using an event in my openedx-platform tests, I found that throwing an exception in an event handler has no effect, and the exception is silently ignored.
This is because openedx-events uses send_robust by default even in tests, which suppresses exceptions thrown in the event receivers. This seems to go against the original intention of https://github.com/openedx/openedx-events/pull/29 which said:
This PR changes which send method to use when sending an event, so they don't fail during runtime. Instead, they will be allowed to fail in a testing/dev environment.
I found that I can work around this by adding EventsIsolationMixin to my test class, and adding this setup code:
@classmethod
def setUpClass(cls):
"""Test setup"""
super().setUpClass()
# By default, errors thrown in signal handlers get suppressed. We want to see them though!
cls.allow_send_events_failure(*(s.event_type for s in OpenEdxPublicSignal.all_events()))
But I think that this shouldn't be necessary - suppressing exceptions during tests is confusing and unexpected behavior that takes a while to debug. Also, the code shown above "leaks" because it affects any other tests that will run afterward. The events API provides a way to "allow send events failure" but it doesn't provide a way to undo that.
I recommend that you just scrap all of that and add a setting called SUPPRESS_EVENT_EXCEPTIONS or EVENT_SENDING_ROBUST_BY_DEFAULT which is True in envs/production.py and otherwise defaults to False.
Other issues
-
"allow send events failure" is very ambiguous. Does that mean "allow events to fail, by ignoring their exceptions", or does that mean "allow exceptions (failures) to be reported" ? Calling this "suppress exceptions" would be way more clear.
-
I'm not the only one confused. The documentation of
OpenEdxPublicSignal.allow_send_event_failuresays:
But this is the opposite of what it actually does!
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading openedx_events/tooling.py at lines 168-171 and 267-273, then inspect envs/production.py to understand the current event-sending behavior and configuration. Clarify how exceptions should behave in tests versus production, how the setting should be named, and how the misleading allow_send_event_failure documentation should change. Done means the behavior is consistent with the documented configuration and does not leak between tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100